Opened 11 years ago
Closed 11 years ago
#10865 closed Bug (fixed)
Widgets: copybin is not hidden
Reported by: | Olek Nowodziński | Owned by: | Piotrek Koszuliński |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.3 |
Component: | Core : Pasting | Version: | 4.3 Beta |
Keywords: | Cc: |
Description
Because of that, some flickering can be observed while copying widgets. For example:
- In IE9 open samples/plugins/image2/image2.html
- Select a widget.
- C-c.
- You've just been teleported to the bottom of editable.
This may depend on the size of selected element etc.
Change History (13)
comment:1 Changed 11 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 11 years ago by
Owner: | set to Piotrek Koszuliński |
---|---|
Status: | confirmed → assigned |
comment:3 Changed 11 years ago by
Summary: | Widgets: pastebin is not hidden → Widgets: copybin is not hidden |
---|
comment:4 Changed 11 years ago by
Owner: | Piotrek Koszuliński deleted |
---|---|
Status: | assigned → confirmed |
comment:5 Changed 11 years ago by
Owner: | set to Marek Lewandowski |
---|---|
Status: | confirmed → assigned |
comment:6 Changed 11 years ago by
Status: | assigned → review |
---|
the best test for this copying is image2 plugin test
i've created 2 solutions:
1st is more diffrentiated
+ imho gives better ux
- is more complicated
2nd is more cross-browser
+ straight forward code
- relies on manual scrollTop reset, after very small timeout
1st solution t/10865:
make use of the fact that ie9 and higher allows to copy content of hidden elements, whereas other browsers don't
pros: it inserts hidden element to dom so _theoretically_ there should be no reflow, and no jumping anywhere is visible
if the browser does not support hidden markup copying it will apply absolute positioning
2nd solution t/10865b:
uses the same absolute positioning for all browsers, but ies below 11th does hiccup on that
pros: clearer code
please take a look on both solutions and put your thoughts on this subject
notes how solution 2 behaves on IEs:
IE8: adjusts its position by few pixels before first paste, it jumps when pressing ctrl+c (it does not after some pastes though)
errors: on paste undo, throws error: call .blur() on null/undefined
IE9: scrollbar jumps from time to time (not always) rarely content jump can be seen
IE10: scrollbar + content jump from time to time (approx 1/4 cases)
IE11: everything looks fine
general issue with this approach is that from times to times they only copy inner content of widget (that is caption for image2), but it's quite boundary case. I have no idea what's the case at this moment.
In meanwhile i will do some extra research on that topic, but meanwhile we can decide which conception do we want to follow.
comment:7 Changed 11 years ago by
Status: | review → assigned |
---|
Let's go with absolute position everywhere for now. I find this a more straightforward way and we need to close this issue faster.
Later on we can do more research, especially considering that the issue is specially annoying in IE10.
comment:8 Changed 11 years ago by
Status: | assigned → review |
---|
In this situation we're going to progress with branch t/10865b.
Note that regex was changed in the way that it got duplicated (?:\s*style\=\"[^"]+")?
, that's because IE9 moves style attribute as the first attribute, while other versions place it after id attr.
I've also moved regex to pasteReplaceRegex
variable (which was not joined to var statement @ line 14, since it's messy, and is only important for setupDataProcessing() function ). That way we will avoid regex recompilation everytime setupDataProcessing() will be called.
During that ticket i also found out that:
- there quite often there is issue (cross browser) that only caption text of image2 will be copied instead of whole wrapper
- undo on ie8 causes exception
These issues will be reported in separate ticket.
comment:9 Changed 11 years ago by
Owner: | changed from Marek Lewandowski to Piotrek Koszuliński |
---|
I pushed t/10865b on dev and tests.
Additional fixes:
- Longer timeout is used so Chrome on Mac will be able to grab the content.
- Double spans on IEs are used to avoid expanding editable's height.
- Inverted order of focusing widget and removing copybin to avoid scroll jump on IEs. Previously copybin was removed first, so for a short period selection was reset to the beginning of editable.
- Since the timeout is now pretty long (100ms), copying is aborted if previous is still going on. It prevents from error being thrown and we've got the same stopper in clipboard.
comment:10 Changed 11 years ago by
Current solution works very well with non ie8 browsers, and there is no longer any jumping, here are results of my testing:
ie9 - fine
ie10 - fine
ie11 - fine
opera - fine
firefox - fine
chrome - fine
ie8 - browser always scrolls to the bottom
Seems that only ie8 left to fix. Quickfix which WFM is setting editor.document.$.documentElement.scrollTop
after range.select()
to its initial value, stored during function initialization.
comment:11 Changed 11 years ago by
Thanks, I pushed a fix for IE8 to t/10865b and rebased it on latest major. Waiting for code review now.
comment:13 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Fixed on major with git:cc0310f on dev and 0c6414a on tests.