[Webkit][IE8] Browser hangs when user set Number list & then insert table

[Satya Minnekanti]

Test data:

 <div>aaaaaaaaaaaaaaaa</div>
  <div class="bbbbbbbbbbbbb"><br></div>

To reproduce the defect:

1. Open nightly build, clear all editor content. set above data as editor content.

2. Ctrl+A to select all the content.

3. Click on "Insert/remove number list" to convert 2 lines into number list.

4. List should be selected.

5. Click "Table" icon to open Table Properties dialog & click OK to insert table with default values.

Issue: Browser hangs

[Jakub Ś]

Problem can be reproduced from CKEditor 4.4.4 in Blink and Webkit (Mac Safari) browsers.

It has started occurring when inserting a list has started creating two elements. In version 4.4.3 and below only one list item was created in step 3 and no hanging occurred.

[Tade0]

If the class is removed from the second div, the error does not occur. Similarly substituting class for e.g. style also produces no error.

[Tade0]

There's an infinite loop here: ​https://github.com/ckeditor/ckeditor-dev/blob/846f2a1a8f45db9b2c59538b59cbbfb1080b84ed/core/editable.js#L433 that keeps splitting the block ad infinitum.

How is this connected with having a class is unclear at the moment.

[Tade0]

One important difference is that in this line: ​https://github.com/ckeditor/ckeditor-dev/blob/846f2a1a8f45db9b2c59538b59cbbfb1080b84ed/core/editable.js#L419 the contents don't get deleted the same way when the second element has a class, and this causes the previously mentioned infinite loop.

[Tade0]

The error is produced as follows:

Right before its occurrence the selection looks like this:

<ol>
  <li>[aaaaaaaaaaaaaaaa</li>
  <li class="bbbbbbbbbbbbb">]<br></li>
</ol>

After range.deleteContents( 1 ); is called the selection looks like this:

<ol>
  <li></li>[]
  <li class="bbbbbbbbbbbbb"></li>
</ol>

Both the range startContainer and endContainer are the <ol> element. Such a selection is not expected, and therefore causes the while loop that follows to get into a state of adding a paragraph/div infinitely.

[Marek Lewandowski]

Pushed tests to ​t/14755 .

[Marek Lewandowski]

Pushed to ​t/14755.

[kkrzton]

LGTM!

Rechecked exactly and this issue occurs on WebKit and Blink browsers and also IE8 browser. There is no need to specify additional if's only for those browsers as the issue happens for the specific selection only present on this browsers for this particular case. The fix operates on this selection so it is well-tailored for this issue.

Just one thing to discuss: There is a small inconsistency, between native behavior and the fix. On browsers without the issue, inserted table replaces the whole list. The fix works in a way that table is inserted inside first list item. While this is an edge case I would say that behavior is fine. However, I would like to ask @m.lewandowski if it was done on purpose or maybe you just have not considered different approach? Is it acceptable from you perspective or we should be 100% consistent with native behaviour?

[Marek Lewandowski]

So here the scope was to prevent the browser from hanging. Also the fix had to be consistent with current CKEditor behavior, when you do the same operation on bare li element, without class attribute - and that's achieved.

The end-user behavior is a matter for other issue, as this is a broader subject. Because of doubts I added a note in the manual test.

[kkrzton]

Thanks for the explanation @m.lewandowski. Fair enough.

[kkrzton]

Merged to master with ​70e047f.

[Marek Lewandowski]

Updated the issue title to match recent findings.