List is not merged

[Garry Yao]

Reproducing Procedures

1. Open the replace by class example page with 'enterMode = br' in FF;
2. Make the content and selection same as below:

   <ul><li>item1</li></ul><ol><li>ite^m2</li></ol>
   

3. Click on 'Bullet List' command to change the list type.
   • Expected Result:

     <ul><li>item1</li><li>item2</li></ul>
     

   • Actual Result: The previous list is not merged.

     <ul><li>item1</li></ul><ul><li>ite^m2</li></ol>
     

[Garry Yao]

The patch is including the fix at #3617 for easy reviewing.

[Martin Kou]

It's not working with the same test case you provided.

After the patch, item 2 is seen to jump on top of item 1:

<ul>
	<li>
		ite^m2</li>
	<li>
		item1</li>
</ul>

[Garry Yao]

Replying to martinkou:

It's not working with the same test case you provided...

L496 of list/plugin.js is modified for passing Martin's catch.

[Martin Kou]

Ok, almost there, but the code can be simplified. :-)

1. The stopFlag - this is actually my fault, you simply copied it over from my old code. But it's not necessary. It can be replaced by a break.
2. mergeSibling is wrapped by CKEDITOR.tools.bind - but the only reference to "this" inside is just this.type. So the CKEDITOR.tools.bind() is actually quite wasteful. Instead, something like this would work with less complexity:

   var type = this.type;
   function mergeSibling( rtl )
   {
    ...
   }
   mergeSibling();
   mergeSibling( true );
   

[Garry Yao]

Replying to martinkou:

Ok, almost there, but the code can be simplified...

Pretty high-quality review ;), I'm updating the empty spaces skipping with new param of CKEDITOR.dom.node::getNext/getPrevious.

[Garry Yao]

Fixed with [3605].