CKeditor 3.2.1 : html content attached makes ckeditor crash the browser FF/IE

[Alex Medina]

Tested in Ckeditor 3.2.1 demo site. I tested mostly with FF but it was also reported with IE.

The content in the file attached causes FF and IE to utilize tons of memory and the browser reports a script not responding alert error ("unresponsive script" in FF). If you click on "Continue" in the browser alert window, the browser crashes.

The attached small_html_ckeditor_bug.html web page causes this error with very little html code.

Notes/observations:

• If you only remove the the center tags, the problem goes away.
• If you only remove the last li tags, the problem goes away.
• if you only remove the inner table, the problem goes away.

[Garry Yao]

Reproduced in fullpage mode.

[Garry Yao]

Ticket Test added:
​run OR ​view source.

[Frederico Caldeira Knabben]

I think the idea behind the patch, as well as the tests are wrong. Leaving those <li>s as is could give unexpected results on the editor operations.

[Tobiasz Cudnik]

After deeper checking, i think that Garry's solution is fine, as all LIs are still fixed by a code executed before the new one.

I'm opting for another review.

[Sa'ar Zac Elias]

I tend to agree with Fred. We should opt to fix this code (gather this items in lists etc.) and not leaving it as is. For example now we can not use the list properties dialog on the list items.

[Paweł Horzela]

If there is <LI> inside <TD> it's not allowed in DTD. We create new element <UL> between <TD> and <LI> to have <TD><UL><LI>.

[Paweł Horzela]

Patch

[Garry Yao]

The idea is good, but why you're moving the children? Test case:

<div>
	text
	<li>item</li>
</div>

produces

<div>
	<ul>
		text
		<li>
			item</li>
	</ul>
</div>

[Garry Yao]

Also, we must also not add the 'ul' directly, the <ul> might not fit in position as well, it should be something like:

parser.onTagOpen( 'ul', {} );

[Paweł Horzela]

Patch

[Garry Yao]

Ticket test updated with [5989].

[Paweł Horzela]

Fixed with [5990].