Opened 6 years ago

Closed 6 years ago

#10907 closed Bug (fixed)

[IE11] Selection needs <br> filler in empty blocks

Reported by: Piotrek Koszuliński Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.3
Component: General Version:
Keywords: IE11 Cc:

Description (last modified by Piotrek Koszuliński)

Broken TCs:

  1. Load data: <p>&nbsp;</p><p>&nbsp;</p>. Paragraphs are not selectable, because they are invisible.
  1. Click new page button - paragraph has 0 height (visible with show blocks on).
  1. In some cases paragraphs created by enter key are not filled with <br>. For example when leaving list by double enter.
  1. Data retrieved from focused editor contains both - nbsp filler and bogus br: <p><br />&nbsp;</p>.

Related: #10992.

Change History (7)

comment:1 Changed 6 years ago by Piotrek Koszuliński

Status: newconfirmed

comment:2 Changed 6 years ago by Piotrek Koszuliński

In my opinion we should extract flags like 'needsFillerBr' and 'needsFillerNbsp' to CKEDITOR.env, or somewhere else, instead of checking in 1000000x places CKEDITOR.env.ie.

Version 0, edited 6 years ago by Piotrek Koszuliński (next)

comment:3 Changed 6 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedassigned

comment:4 Changed 6 years ago by Piotrek Koszuliński

Description: modified (diff)

comment:5 Changed 6 years ago by Piotrek Koszuliński

Status: assignedreview

Uh...

What's done in phase 1:

  1. Introduced env.needsBrFiller and env.needsNbspFiller.
  2. Used those flags in various places across all code base. I was using mainly needsBrFiller, but if the meaning of code was "if we need nbsp here for some reason" I was using the second flag.

Those two points actually fix most of the issues. However, few IE specific remained, so:

  1. Editor should fill empty block with bogus <br> on selection change - thanks to that padding is not lost when applying block styles or switching on&off lists. Note - sometimes IE places selection after appended bogus, so we need to fix this.
  2. It is possible to remove bogus <br> from blockless editable, so on keyup we're fixing this. I was hesitating between preventing this and fixing after it happened, but I chose the second version to KISS.
  3. Bogus should not be appended if we've got a fake selection.
  4. There was a failing test in http://ckeditor4.t/dt/plugins/list/list_enterBr.html because list plugin wasn't removing bogus <br>s when removing list. It was looking for elements with type=_moz so this issue was reproducible also on FF in specific cases. Now we handle also other browsers there. Also, there was a bug in the second condition there (have fun with finding it :D), so I fixed it.

That's it...

comment:6 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

I've made a small commit to fix still one issue.

These changes will require us to have more monkey testing happening in the editor this time, to ensure that basic editing features are not impacted. I'm talking about other browsers, other than IE11.

comment:7 Changed 6 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on major with git:ac1285f on dev and 99a987c on tests.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy