Opened 4 years ago

Last modified 4 years ago

#14395 confirmed Bug

[Firefox] backspacing into a list with an empty list in between causes content to be removed

Reported by: Alex T Owned by:
Priority: Normal Milestone:
Component: General Version: 3.0
Keywords: Cc:

Description

Steps to reproduce

  1. go to the ckeditor demo page
  2. put the following html into the demo page
<ul>
    <li>dsadas</li>
    <li>dsa</li>
</ul>

<ol>
</ol>

<p>this will disappear</p>
  1. Place the cursor at the start of the paragraph tag and press backspace

Expected result

Under chrome - "this will disappear" will be merged into the list item "dsa". The empty ol in between will be removed.

Actual result

Under Firefox 44 - paragraph tag disappears, empty ol remains.

Other details (browser, OS, CKEditor version, installed plugins)

Firefox 44 and 44.0.1 on OSX.

Attachments (1)

IE11.swf (595.8 KB) - added by Jakub Ś 4 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 4 years ago by Jakub Ś

Status: newconfirmed
Version: 4.5.73.0

In Firefox this problem can be reproduced from CKEditor 3.0. Exactly as it has been described.

If Blink and Webkit it works as expected.

In IE in general the text is merged into empty ol, which will be merged into <li>dsa</li> element if you keep backspacing. At some stage, the new nested list will be left and you will start deleting dsa. If you then click at the beginning of this will disappear and press backspace, the text will disappear.

Please see attached video.

Changed 4 years ago by Jakub Ś

Attachment: IE11.swf added

comment:2 Changed 4 years ago by Alex T

I've started looking at this. I'm pretty new to the CKEditor codebase so it's not a perfect fix yet and I need to figure out a few other things still. But maybe my initial findings can help someone, or trigger something to point me in the right direction. :)

Part of the issue is at https://github.com/ckeditor/ckeditor-dev/blob/master/plugins/list/plugin.js#L904

This if block ends up executing if you're deleting into an empty list, and the value of walker.previous() is null, so the handler soon crashes out a few lines later.

Fixing it so that it keeps previous as the empty list causes "this will disappear" to correctly get merged up, but at least under Firefox, the invisible empty list in between is kept, which isn't consistent with what Chrome does. I suspect this might be a ContentEditable thing but I'm not sure yet.

comment:3 Changed 4 years ago by Alex T

The problem I mentioned actually occurs in Chrome as well. It appears the original behaviour of removing the empty <ol> was happening because the default backspace event was not prevented, since the backspace handler was erroring out.

The cursor is actually placed inside the <ol>, but it's not considered an editable element (since the dtd for lists only allows list items, per spec) and so the "closest block element" ends up being the list item in the non-empty list.

I've moved on to figuring out how to insert a <li> into the invisible list when this case occurs - although jarring if different bullet type appears out of nowhere, it at least should allow the user to remove the empty list without using a source dialog.

comment:4 Changed 4 years ago by Alex T

I think I have something working. I need to do more testing coverage and I'll throw up a PR for this.

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