Opened 12 years ago
Closed 12 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:
- Open http://ckeditor.com/demo in Chrome
- Open javascript console, run
CKEDITOR.instances.editor1.resize(null, 1500)
- scroll the window so that toolbar and the bottom of the editor are not visible
- 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 12 years ago by
Milestone: | → CKEditor 4.0.1 |
---|---|
Status: | new → confirmed |
comment:2 Changed 12 years ago by
To avoid incorrect scrolling during pasting we're trying to place pastebin in safe position. That means:
- 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.
- 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.
- 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
returnsnull
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 12 years ago by
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 12 years ago by
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 12 years ago by
Owner: | set to Piotrek Koszuliński |
---|---|
Status: | confirmed → assigned |
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 12 years ago by
Status: | assigned → review |
---|
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 12 years ago by
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.
comment:8 Changed 12 years ago by
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 12 years ago by
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 12 years ago by
Status: | review → review_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 12 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Fixed with git:8341c16.
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.