Can't remove Bold/Italic/Underline in empty line.

[Jakub Ś]

This is happening mainly in Mac but some variation can be observed in Windows.

1. Open replacebycode sample
2. Put cursor at the end of first paragraph July 21 at 02:56 UTC.^ and press enter.
3. Press Command+I or Ctrl+I
4. Press Command+I or Ctrl+I again and type few letters.

Results: On Mac it is impossible to remove simple style. On Windows button is still highlighted, elements path shows 'em' but when you start typing you get normal text.

Problem can be reproduced in Blink and Webkit browsers from CKEditor 4.4.5.

This issue causes #12631.

[Piotrek Koszuliński]

It might look like a regression, but actually it's not. It's an older bug in the styles system (I might have reported it some time ago).

On 4.4.4 (and earlier versions), when you do that TC, there is an empty inline element left after you press the style keystroke for the 2nd time - <p><strong></strong><br></p>. It should be removed by the styles system when removing the style, but it's not.

In 4.4.5 in #12423 we changed a behaviour of a method important for the styles and selection systems (so it's hard to say what it could affect). And I think that this change might actually be good, because it aligned the method behaviour on Webkit/Blink to Firefox. And on Firefox this problems isn't reproducible. This may mean that there's some other part of code wrong on Chrome/Blink (e.g. the code that affects the getStartElement() method).

Anyway, if we fix the issue in the styles system, then this issue (reported here) won't be reproducible. However, the getStartElement() behaviour must be checked too.

[Piotrek Koszuliński]

It might look like a regression, but actually it's not. It's an older bug in the styles system (I might have reported it some time ago).

Nope, I haven't reported it.

[Piotrek Koszuliński]

I have some more details about this issue (examples for removing selection from <p>foo<strong>\u200b{}</strong></p>):

1. Result of style.removeFromRange is correct. The range is placed in: <p>foo<strong>\u200b</strong>[]</p>.
2. When selection.selectRanges is executed it checks whether the selection needs a filling char. Note: by "correct" I mean that it has not changed recently, although it is a bug that the removeFromRange() method does not remove the empty inline element.
3. In this case the algorithm (implemented by the rangeRequiresFix function) should find that the range is placed right after <strong>\u200b</strong> and return true - the range requires the filling char.
4. For some odd reason the algorithm uses the walker.invisible() method. Apparently it did return false for the <strong>\u200b</strong> element. But it does not do this anymore.

Plan:

1. We need tests for walker.invisible() to verify that behaviour of this method really changed for the above scenario. The expected result is true, so if the method returns this value, then it should not be touched.
2. We need to fix the rangeRequiresFix function which should not be based on previous quirky behaviour of the walker.invisible() method.

[Piotrek Koszuliński]

Edit: I updated the comment:7 because I messed up walker.invisible()'s return values.

In ​git:d31d2cf6 I added a test that verifies the case described in comment:7. Before #12423 it was returning false and now it returns true. Therefore, the behaviour of rangeRequiresFix must be fixed, because it cannot rely on walker.invisble().

[Piotrek Koszuliński]

Pushed ​branch:t/12621.

I needed to ignore the new tests on IE8, because it doesn't pass them. Am I right that it's impossible to set selection at that position on IE8?

[Piotrek Koszuliński]

The patch fixes also #12631.

[Artur Delura]

Fixed on master with ​git:457224e.