[IE] Focusing widget will reset editor scrollbar position

[Marek Lewandowski]

since: 4.3 until major

If you will scroll your content all the way down, blur editor, and click directly to widget (to focus it) - editor scrollbar will jump to topmost position. It happens regardless of widget type.

1. open image2 sample
2. scroll editor to its very bottom
3. blur editor (i.e. click on sample title)
4. click directly on any image widget

Expected result:
Clicked widget should simply be focused.

Current result:
Scrollbar is moved to top, in addition to that widget is not being focused, because caret goes to begining of the document.

[Marek Lewandowski]

I've just checked IE9 and it turns out that it occurs there as well, so it's present at least in IE10, IE9.

[Jakub Ś]

I was able to reproduce this issue from CKEditor 4.3 beta in IE9 and IE10 (IE8 seems to work fine)

[Marek Lewandowski]

change milestone to 4.3.1 it would be good if we can make this issue into release since it may be really annoying for end user

[Marek Lewandowski]

Source of the problem is focusing editor itself in:
​https://github.com/ckeditor/ckeditor-dev/blob/master/plugins/widget/plugin.js#L1040

However skipping that line will cause no focus on widget. For the time being we will skip this issue since it's time consuming.

---

Also i found another TC, might (but doesn't have to) be related to that ticked.

(using IE10 @ Win8)

1. open sample with image2 plugin (/samples/plugins/image2/image2.html)
2. scroll down editor content all the way down
3. click at white space between editor and browsers scrollbar

As a result in addition also window will be scrolled.

[Marek Lewandowski]

Back to confirmed, since i got sick, and did not investigate that much at friday.

[Piotrek Koszuliński]

This was a nightmare and I found only a clean solution, not a perfect one. I also won't be able to explain all my findings in a understandable way, so just a couple of facts:

1. On IE something weird happens when you click widget wrapper or its child directly - after mouseup control selection or something that resembles it is created. I couldn't understand if that's CKE's fault or purely IE thing. It's very fragile and depends on used elements, attributes etc.
2. Preventing mousedown does not help for what happens after mouseup, although for the time between mousedown and mouseup the control selection sometimes (also depends on case and IE version) is not created, so UX is better.
3. To fix this we need to wait after mouseup and focus widget then.
4. Unfortunately this creates a weird situation in which mousedown happened, mouseup happened, but we blocked all that so selection wasn't unlocked.
5. I checked controlselect and beforeeditfocus events, because I thought that I'll be able to avoid preventing default action on mousedown, what would made the situation more normal, but they are terribly implemented and not worth a penny.
6. So, we are in a situation after mouseup, with selection still locked, because we prevented mousedown and we want to focus widget.
7. First we fake selection on widget wrapper. We cannot unlock selection first because of ​git:f0c7b424. I didn't want to have code branching there, so the order has to be: set selection, focus.
8. So there's lockedSel#fake -> lockedSel#reset -> hideSelection.
9. hideSelection has to move selection to the hidden selection container. We cannot skip that even if selection is locked, because we'll not avoid issue explained in ​git:f0c7b424 which actually occurs also on IEs (vide this issue).
10. So hideSelection calls editor.getSelection().selectRanges( [ hiddenSelConRange ] ). Since editor returns locked selection, sel#getRanges falls into the special condition for sel#isLocked. This code is a hack, but the only possible hack allowing to play with locked selection and assuring that all other properties of it will be updated accordingly. Unfortunately it is far from perfect and when trying to handle focus it actually fires many focus/blur events what leads locking selection again and... nvmnd :D. Horror.
11. But what's also wrong is that to move selection to hidden container hideSelection uses locked selection when it should use the real one. This is proposed in ​t/11104.
12. Hooooowever, the special condition in selectRanges assures that focus gets back to the place where it was originally. Since this code was the actual source of issue, we cannot use it and, since we don't have unlimited time, I want to leave this broken. No one should play with locked selection and I'm for removing the special condition from selectRanges in the future. But for now - we can just ignore this.

[Piotrek Koszuliński]

Ok, enough clicking - time for review. I didn't include any tests because I couldn't write them. Since we don't have a nice logging system (#9786) the only easily checkable thing is hidden. Moreover, I saw that in my TC the "Wrong selection instance resets fake selection." is still logged when it's not when manually testing this.

Pushed ​branch:t/11104. Note: this patch has to be thoroughly tested on every browser.

Known issues (not regressions):

• [FF] widget has to be clicked twice to focus it when starting from blurred inline editor.

Fixed issues:

• [IE] no more scrolling.
• [IE] no "Wrong selection instance resets fake selection." when focusing editor by clicking on widget if widget was focused before editor was blurred.
• [IE] no "Wrong selection instance resets fake selection." when doubleclicking widget.

[Olek Nowodziński]

Issues specific for t/11104 found so far:

• [IE11]
  1. go to plugins/image2/dev/image2.html
  2. select the last widget of "Inline sample"
  3. clicking outside of the editor doesn't blur the widget (repeat 2. and 3. until reproducible)

[Piotrek Koszuliński]

Replying to a.nowodzinski:

Issues specific for t/11104 found so far:

• [IE11]
  1. go to plugins/image2/dev/image2.html
  2. select the last widget of "Inline sample"
  3. clicking outside of the editor doesn't blur the widget (repeat 2. and 3. until reproducible)

I could not reproduce this. I repeated 2-3 many times.

[Olek Nowodziński]

Another TC:

• [IE8]
  1. plugins/image2/dev/image2.html
  2. select any widget in any editor
  3. De-caption it and set align to none (make it truly inline)
  4. Editable is scrolled to top

[Olek Nowodziński]

Replying to Reinmar:

Replying to a.nowodzinski:

Issues specific for t/11104 found so far:

• [IE11]
  1. go to plugins/image2/dev/image2.html
  2. select the last widget of "Inline sample"
  3. clicking outside of the editor doesn't blur the widget (repeat 2. and 3. until reproducible)

I could not reproduce this. I repeated 2-3 many times.

Check this out attachment:11104_ie11_unable_to_blur.mp4​

[Piotrek Koszuliński]

Replying to a.nowodzinski:

Replying to Reinmar:

Replying to a.nowodzinski:

Issues specific for t/11104 found so far:

• [IE11]
  1. go to plugins/image2/dev/image2.html
  2. select the last widget of "Inline sample"
  3. clicking outside of the editor doesn't blur the widget (repeat 2. and 3. until reproducible)

I could not reproduce this. I repeated 2-3 many times.

Check this out attachment:11104_ie11_unable_to_blur.mp4​

Trying 100x I was able to reproduce this once. Totally not important because it's even hard to understand if it's t/11104 specific.

[Piotrek Koszuliński]

Replying to a.nowodzinski:

Another TC:

• [IE8]
  1. plugins/image2/dev/image2.html
  2. select any widget in any editor
  3. De-caption it and set align to none (make it truly inline)
  4. Editable is scrolled to top

I was able to reproduce it on t/11104 on IE8, but not on IE11. What's more - I was able to reproduce it on master on IE11, but not on IE8.

As long as t/11104 fixes more than breaks it's better than nothing.

[Olek Nowodziński]

Replying to Reinmar:

• [FF] widget has to be clicked twice to focus it when starting from blurred inline editor.

The right, reproducible TC is as follows:

1. Open plugins/image2/dev/image2.html
2. Focus framed editor (click somewhere in contents).
3. Scroll to inline editor.
4. Focus a widget in inline editor (it's focusing).
5. Focus framed editor (see: 2.).
6. Try re-focusing the same widget in inline editor (now it requires double-click).

[Piotrek Koszuliński]

I checked that TC mentioned in comment:17 is IE8 specific. Little bit more important than comment:15, but still not a blocker.

[Olek Nowodziński]

Though not perfect, the solution in ​branch:t/11104 fixes more than it breaks. I'm OK with that — still this is a step forward. Tested:

• IE[8...11]
• FF
• Chrome (Linux, OSX)
• Safari

[Piotrek Koszuliński]

Fixed on master with ​git:bb0ed1c.