Tests failing after update to Chrome 35

[Olek Nowodziński]

Two tests fail after upgrade to 35.0.1916.114 while they pass in 34.0.1847.131:

/dt/plugins/elementspath/click.html#test clicking typical strong element
/dt/plugins/enter/enterkey.html#test enterkey after invisible element

We must know the scope and possible impact of changes in v35 behind this failure. It may affect other aspects of the editor which are not tested or impossible to test.

[Olek Nowodziński]

My research revealed that the difference between 34 and 35 is that window.getSelection().getRangeAt() works slightly different, which quite likely has been introduced in ​169100.

Use attached sample to see that if clicked "Select natively" in Opera (Chrome/34.0.1847.132) the range is attached in the text node "foo", while in Chrome (Chrome/35.0.1916.114) it is <strong>. What happens next is just a snowball effect – algorithms enter wrong branches.

As for

/dt/plugins/elementspath/click.html#test clicking typical strong element

I suppose that elementspath plugin is safe because it is a matter of getHtmlWithSelection, which returns a "wrong" range in the assertion. In fact elementspath does not use selection.getRanges() (getRangeAt) at all.

However

/dt/plugins/enter/enterkey.html#test enterkey after invisible element

might be a problem because the algorithm relies on the output of selection.getRanges(). We got to find out how (if?) to fix it. In fact what worries me a lot is that we got 2 different behaviours for CKEDITOR.env.webkit, which means that Webkit and Blink go into different directions. We need to know how to deal with it.

[Piotrek Koszuliński]

cc

[Piotrek Koszuliński]

It seems that we were hit by ​https://code.google.com/p/chromium/issues/detail?id=346613 which is actually fixing one of the biggest problems with selection in Blink (hopefully Webkit will have this patch too). It's not fully fixed yet, so we still need the filling char, but I think that we'll be able to remove it soon :).

The biggest change right now was that getRangeAt() returns exactly the range set by addRange(), without moving it to nearest text node. It's perfectly visible in the sample for ​https://code.google.com/p/chromium/issues/detail?id=351819.

We need to start updating our own code too. Hopefully, these are only tests. Olek, could you handle this? We need to learn whether we don't have some logic depending on the old, incorrect behaviour of Blink. If so, we need to straighten it ASAP. Note that we already have env.safari and env.chrome.

[Olek Nowodziński]

I pushed fixed tests to t/11999. Note that several assertions will fail in Opera unti gets updated to latest Blink.

[Piotrek Koszuliński]

So these were only tests? What about getRanges() method? Does it seem to work ok? I think that you can verify this by controlling env.webkit checks in it.

[Olek Nowodziński]

Replying to Reinmar:

So these were only tests? What about getRanges() method? Does it seem to work ok? I think that you can verify this by controlling env.webkit checks in it.

Working on it. That was only the 1st stage.

[Olek Nowodziński]

I checked the code of seleciton.getRanges() and tested the editor manually. I see no alarming behaviour other than what is fixed by t/11999 (tests). Still I insist that we keep an eye on the whole matter during test phase because there might be something that I missed.

[Piotrek Koszuliński]

Merged to master with f612802. I made some manual tests and the affected cases seem to work.