#12621 closed Bug (fixed)
Can't remove Bold/Italic/Underline in empty line.
Reported by: | Jakub Ś | Owned by: | Piotrek Koszuliński |
---|---|---|---|
Priority: | Must have (possibly next milestone) | Milestone: | CKEditor 4.4.6 |
Component: | Core : Styles | Version: | 4.4.5 |
Keywords: | Blink Webkit Support | Cc: |
Description (last modified by )
This is happening mainly in Mac but some variation can be observed in Windows.
- Open replacebycode sample
- Put cursor at the end of first paragraph
July 21 at 02:56 UTC.^
and press enter. - Press Command+I or Ctrl+I
- 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.
Change History (12)
comment:1 Changed 10 years ago by
Status: | new → confirmed |
---|
comment:3 Changed 10 years ago by
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.
comment:4 Changed 10 years ago by
Component: | General → Core : Styles |
---|
comment:5 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 10 years ago by
Priority: | Normal → High |
---|
comment:7 Changed 10 years ago by
I have some more details about this issue (examples for removing selection from <p>foo<strong>\u200b{}</strong></p>
):
- Result of
style.removeFromRange
is correct. The range is placed in:<p>foo<strong>\u200b</strong>[]</p>
. - When
selection.selectRanges
is executed it checks whether the selection needs a filling char. - 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. - For some odd reason the algorithm uses the
walker.invisible()
method. Apparently it did returntrue
for the<strong>\u200b</strong>
element. But it does not do this anymore.
Plan:
- We need tests for
walker.invisible()
to verify that behaviour of this method really changed for the above scenario. The expected result isfalse
, so if the method returns this value, then it should not be touched. - We need to fix the
rangeRequiresFix
function which should not be based on previous quirky behaviour of thewalker.invisible()
method.
comment:8 Changed 10 years ago by
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()
.
comment:9 Changed 10 years ago by
Owner: | set to Piotrek Koszuliński |
---|---|
Status: | confirmed → review |
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?
comment:11 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | review → closed |
Fixed on master with git:457224e.
comment:12 Changed 10 years ago by
Milestone: | → CKEditor 4.4.6 |
---|
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.