Opened 6 years ago

Closed 6 years ago

#9771 closed Bug (fixed)

Webkit scrolls to the top after the paste

Reported by: tema Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.0.1
Component: Core : Pasting Version: 4.0.1
Keywords: paste Cc:

Description

How to reproduce:

  1. Open http://ckeditor.com/demo in Chrome
  2. Open javascript console, run
    CKEDITOR.instances.editor1.resize(null, 1500)
    
  3. scroll the window so that toolbar and the bottom of the editor are not visible
  4. paste something anywhere in the visible area of the editor

Now: after the paste editor scrolls to the top

Expected: editor to stay at the pasted text position

Change History (11)

comment:1 Changed 6 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.0.1
Status: newconfirmed

After resizing editor pasting scrolls viewport even if the final position of caret after pasting would be visible. I reporoduced that on Chrome, but not on FF.

comment:2 Changed 6 years ago by Piotrek Koszuliński

To avoid incorrect scrolling during pasting we're trying to place pastebin in safe position. That means:

  1. Its top has to be visibile, because when pasting one letter caret will be placed in the upper part of pastebin and if that place won't be visible, then one of viewports (iframe or window) will be scrolled.
  2. Its bottom has to be visible too, because when pasting more content caret will be placed in the lower part of pastebin and the same will happen if it isn't visible.
  3. Pastebin's height has to be greater than line height of pasted content, because otherwise if after pasting caret will stay in the upper part of pastebin and the pastebin is aligned to the top of viewport, then caret won't fit in the viewport (unchangeable vertical-align:baseline) and the viewport will be scrolled to show entire caret.

So we set pastebin's top 10px from the top of editor's viewport and bottom 10px from bottom (10px are just safety gaps). That works perfectly if both ends of editor are visible in window. But unfortunately fails terribly when editor is high or window is low, because one or two ends of pastebin are outside window viewport. So we need to position pastebin also according to window.

I spotted also more issues that doesn't help in fixing this one.

  • pastebin.offsetParent returns null on Webkits (pastebin is appended directly to editable body) if pastebin is based on body element.
  • Switching to div based pastebin doesn't help, because currently editable's body has 20px margins, so it returns getDocumentPosition().y == 20. However, even though after switching to div based pastebin body is returned as offsetParent, in fact when I set pastebin.top=0 it's displayed outside body - it's 0px related to html element, not body. So why is it body returned as offsetParent?
  • Setting position:relative for body of course helps and that will have to be part of my fix.

comment:3 Changed 6 years ago by tema

Maybe I could help you. We are using older version (3.6.2) and I managed to fix this issue there. To lazy to make a proper patch. In a 3.6.2 version (http://dev.ckeditor.com/browser/CKEditor/tags/3.6.2/_source/plugins/clipboard/plugin.js), right before pastebin.setStyles I did

var top = 0;
if (CKEDITOR.env.webkit) {
    // http://dev.ckeditor.com/ticket/9771
    var win = doc.getWindow(),
        frameElement = win.$.frameElement,
        offset = window.pageYOffset -
        CKEDITOR.dom.element.get(frameElement).getDocumentPosition().y;

    if (offset > 0) {
        top = offset;
    }
} else {
    top = sel.getStartElement().getDocumentPosition().y;
}

Then, using 'top' variable

pastebin.setStyles(
    {
        position : 'absolute',
        // Position the bin exactly at the position of the selected element
        // to avoid any subsequent document scroll.
        top : top + 'px',
        width : '1px',
        height : '1px',
        overflow : 'hidden'
    });

comment:4 Changed 6 years ago by Piotrek Koszuliński

Thanks agentcooper, but unfortunately this solution doesn't work on v4. In fact, it doesn't work for me even on 3.6.6. This positioning has to be even more sophisticated now.

comment:5 Changed 6 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

I pushed first version of the patch git:cb05aa0. It needs final polishing like comments, maybe some refack and definitely a lot of tests, but I think that I achieved what I wanted.

comment:6 Changed 6 years ago by Piotrek Koszuliński

Status: assignedreview

I pushed rebased branch with few more improvements.

My tests revealed that Opera needs the same fix as Webkit. But remembering how long it took me to deal with all Webkit's quirks I decided to work on that in separate ticket (#9858), after 4.0.1.

The rest looks pretty good. I haven't had any strange behaviours on IE and FF (code for these browsers haven't changed, except of decreased safety gaps). On Webkit I completely eliminated from our samples issues mentioned in this ticket. But I know that they may still happen on more complex configurations when editor is placed in overflowing element(s). Calculating which part of editor's iframe is visible sounds for me like too much complications in code. I would rather switch to getting clipboard content from paste event. Especially when we'll have proper filtering system.

comment:7 Changed 6 years ago by Piotrek Koszuliński

I pushed t/9771b with simpler fix. It turned out that we can restore initial window scroll position after paste and on Webkit there won't be any noticeable scroll.

I decided to port the same solution for Opera (fixing #9858), but in this case this solution isn't so perfect - there's a noticeable scroll, but at least final position is correct.

Note - this solution still doesn't cover all cases - we restore scroll only of host window, ignoring all scrollable elements and parent windows. That's because comprehensive fix will be complicated and heavy.

Last edited 6 years ago by Piotrek Koszuliński (previous) (diff)

comment:8 Changed 6 years ago by tema

Did you test Safari? Because when I tried a fix, which involves scrolling after the paste to the saved position, I noticed it jumping in Safari only.

comment:9 Changed 6 years ago by Piotrek Koszuliński

Arghh. Unfortunately you're right. On Safari 5 once per ~3-5 pastes I noticed jumping. On Safari 6 once per >5 times.

That's sad, but I think that taking into consideration how much this patch is simpler than previous one we can accept this little issue. Most importantly final scroll position is correct.

comment:10 Changed 6 years ago by Olek Nowodziński

Status: reviewreview_passed

We can still observe flickering (Opera) and delicate scrolling (Safari 5|6); yet this patch is enough since it works in Chrome and prevents random scroll positioning in other browsers.

Anyway, this scenario is really odd, and for sure, it'll affect only some of end-users. We should go ahead with it and focus on priorities.

comment:11 Changed 6 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed with git:8341c16.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy