IE: List creation by merging problem

[Tobiasz Cudnik]

IE: List creation by merging problem.

Steps to reproduce

1. Type 3 lines
2. Select all
3. Apply the Bulleted List command
4. Place a selection in the middle line
5. Apply the Bulleted List command again, to remove this line from the list
6. Select all

Result: A JS error is thrown.

Expected: Lists are merged into one containing 3 lines.

Reproduced in the IE8, strict. Most probably because the step 5 leaves bookmarks in the 1st and the 3rd line.

[Tobiasz Cudnik]

The fault was inside the list splitting logic, which was cloning an UL element along with the _cke_expando ID. This expando was missleading next operations afterwards.

[Garry Yao]

Problem is resolved, but now list ID always lost during transformation.

[Tobiasz Cudnik]

Although cloning IDs is not valid in the context of a document, i've reverted such behavior by extending the dom.node#clone method with ability to clone IDs without the cke_expando.

[Garry Yao]

Are we able to fix this without touching the "clone" method? I mean is the custom data still needed inside arrayToList?

[Tobiasz Cudnik]

We can fix it removing cke_expando manually, although there won't be any difference in the code size. Most important point here is that cloning expando is plain wrong.

There's no customData usage inside arrayToList, although it's needed in other places when used. If we would want to loose it, then we would need to reinvent whole logic of the list processing.

[Tobiasz Cudnik]

Third patch removes the _cke_expando always when cloning a node (right now it was cloned only in IE, thus this issue exists).

There can be more issues because of this, as nodes could became non-unique for such things as customData and event listeners.

[Garry Yao]

1. function 'removeIds' could be further optimized based on 'includeChildren';
2. Just a guess, but as IE treated expando as attributes, copyAttributes should be fixed also;

[Tobiasz Cudnik]

Regarding point 2, following TC confirms that copyAttributes doesn't copy the expando, which isn't actually weird, as copied are only children of an "attributes" property, which expando is not a child of.

var l = console.log;
var a = new CKEDITOR.dom.element( 'p' );
var b = new CKEDITOR.dom.element( 'p' );
a.setCustomData( 'foo', 'bar' );
b.copyAttributes( a );
l( a.$._cke_expando );
l( b.$._cke_expando );

[Tobiasz Cudnik]

It's confirmed using IE8, IE8 in IE7 mode and IE6.

[Tobiasz Cudnik]

Fixed with [6089].