Styles aren't maintained at end of line

[Josh Nisly]

Under certain conditions, basic styles (bold, italic, etc) aren't maintained at the end of lines.

Steps to reproduce:

• Open the replace by code example.
• Press the Bold button, then type text.
• Press the end button (or click after the end of the line.)
• Type some more text.

The newly typed text should be bold, but is not.

This also happens under the following conditions:

• Open the replace by code example.
• Type some text.
• Press Home, then Shift+End to select the whole line.
• Press the Bold button.
• Press the end button (or click after the end of the line.)
• Type some more text.

The problem is that Firefox is appending a <br/> at the end, but it is not being included in the <strong> element. The solution is, when applying a style, to include that BR element.

A patched is attached which fixes this problem. The problem can be reproduced in the latest nightly samples.

[Josh Nisly]

It turns out that there are multiple issues involved. This is a more comprehensive patch to address those issues:

• The enterkey plugin needs to be changed to appendBogus() to any newly-created inline style elements, instead of to the block element.
• fixBlock() in range.js needs to handle situations where an inline element is the only element on the page. Without this, an <p> elements created when pressing enter are created within the inline style element, which makes it impossible to remove the style.

Just to clarify, the aim of this patch is to make sure that the ending, bogus BR is always contained in any relevant inline style elements, so that when pressing End and typing text, the newly created text maintains the previous text's style.

[Josh Nisly]

Oops, the last patch didn't include the changes in the first one.

[Garry Yao]

The thinking is reasonable and the fixing is mostly valid, but I don't understand why the changes arround L1299 of _source/core/dom/range.js, could you explain what doesn 'not necessary' in the comments mean here?

[Josh Nisly]

The issue is that if the document consists of a single style element (e.g. "<strong>Bold</strong>", the range will be entirely inside the strong, so when the range is split into a <p> element, it will be contained inside the <strong> ("<strong><p>Bold</p></strong>"), which makes it impossible to unbold. With the bogus appended, the range extends past the strong, which makes the strong be moved inside of the p element.

If the strong element is not the only element, the range would extend past the strong even without the bogus. That is why it is sometimes unnecessary.

[Josh Nisly]

Actually, it turns out that the range.js change was the right idea, but not quite the right implementation. Here is a better patch.

[Josh Nisly]

3509_4.patch

[Garry Yao]

Replying to highjinx_53:

The issue is that if the document consists of a single style element (e.g. "<strong>Bold</strong>", the range will be entirely inside the strong, so when the range is split into a <p> element, it will be contained inside the <strong> ("<strong><p>Bold</p></strong>"), which makes it impossible to unbold. With the bogus appended, the range extends past the strong, which makes the strong be moved inside of the p element.

This is not true, I assume you're question the result of range::fixBlock function here, and I'll show you how it actually works well with this TC:

1. Open the 'replacebyclass' example with 'enterMode = br' with the following content:

   <b>bold</b>
   

2. Put the cursor at the beginning of document, then use several 'Shift + Arrow Right' to select the whole word, the selection range after this step is as following:

   ^<b>bold</b>^
   <br />
   

3. Execute the following lines of code globally:

     CKEDITOR.instances.editor3.getSelection().getRanges()[0].fixBlock(false, 'p')
   

• Expected Result :

  <p>
  	<strong>bold</strong><br />
  </p>
  

• Actual Result : Same as expected.

[Garry Yao]

Replying to garry.yao:

The thinking is reasonable and the fixing is mostly valid...

Now I have some other concern above your fixing highjinx, actually, on output data, we would like to strip those line-height maintaining br ( aka. padding br ) on #2886, and the problem is that your fixing would have impact on the [principle ​http://dev.fckeditor.net/ticket/2886#comment:3] of judging padding br.
And actually, here the bug is caused by End Key, which might be more properly been fixed by keystroke handler, as you could imagine, we can intercept the key press, 'redirect' it to the correct position, say before end of <b> element here.
So, as stated above and actually we've identified several other places where keystroke redirecting is needed (e.g. #3375), I would now ask you to please hold the current fixing on this ticket and wait to see if we can have a better fix later maybe also from you. I also see your forum post regard this, so again sorry for slow responding, and thanks for your contribution on this and several other tickets, it will be greate to see more of your activities on v3.

[Josh Nisly]

Replying to garry.yao:

Replying to garry.yao:

The thinking is reasonable and the fixing is mostly valid...

And actually, here the bug is caused by End Key, which might be more properly been fixed by keystroke handler, as you could imagine, we can intercept the key press, 'redirect' it to the correct position, say before end of <b> element here.

Actually, this isn't just an End key issue. If you click after the end of the line, the same problem happens, so it's a general selection bug. Without manipulating the DOM, I don't know of any way to avoid the click-after-line problem. I'm curious if you have any ideas on fixing that?

So, as stated above and actually we've identified several other places where keystroke redirecting is needed (e.g. #3375), I would now ask you to please hold the current fixing on this ticket and wait to see if we can have a better fix later maybe also from you.

This continues to be an issue for our users, so we need to have a solution. We'll continue to use this patch (which works well for us); perhaps once another solution is provided we can switch to it.

[Josh Nisly]

Replying to garry.yao:

Replying to highjinx_53:

The issue is that if the document consists of a single style element (e.g. "<strong>Bold</strong>", the range will be entirely inside the strong, so when the range is split into a <p> element, it will be contained inside the <strong> ("<strong><p>Bold</p></strong>"), which makes it impossible to unbold. With the bogus appended, the range extends past the strong, which makes the strong be moved inside of the p element.

This is not true, I assume you're question the result of range::fixBlock function here, and I'll show you how it actually works well with this TC:

Ah, but you missed the point of this patch. :-) This patch moves the <br/> inside the strong, which breaks the TC.

[Josh Nisly]

Replying to garry.yao:

Replying to garry.yao:

The thinking is reasonable and the fixing is mostly valid...

And actually, here the bug is caused by End Key, which might be more properly been fixed by keystroke handler, as you could imagine, we can intercept the key press, 'redirect' it to the correct position, say before end of <b> element here.

I've attached another patch that works as you suggest. There's one big problem, though: there's no key event that we can tie into to do what we need to do. If we use keydown, the selection hasn't moved to the end of line yet, and if we use keyup, other keystrokes can sneak in. For example, if a user types the end key and then a letter key in rapid succession, the letter will "beat" our handler, and therefore will use the default font. I don't see a way around this.

[Garry Yao]

We'll have to leave these keystroke handler feature to 3.2.

[Garry Yao]

Defer this bug since at least users could still use arrow key to correct the result.

[Frederico Caldeira Knabben]

Milestone CKEditor 3.x deleted

[Jakub Ś]

Can be reproduced in CKEditor 4.x (v4)

[Jakub Ś]

#8195 was marked as duplicate.

[Jakub Ś]

It seems we have whole buch of issues basic styles and selection problem across all browsers:

• FF: #3509, #8958, #7830, #11346, #10872
• Blink: #11657
• IE11: #11346

[Jakub Ś]

#12329 was marked as duplicate.

[Jakub Ś]

Possible duplicate was reported here: #12649

[Jakub Ś]

#13137 was marked as duplicate.

[kkrzton]

Tried to reproduce on FF 43.0 and CKEditor 4.5.6 without success.