[chrome] SCAYT toolbar options are in reversed order

[Tobiasz Cudnik]

SCAYT toolbar options are in reversed order. Whats interesting, other menus aren't. On Safari everything works fine.

Screenshot attached.

[Tobiasz Cudnik]

As it turns out Chrome differently interprets Array#sort "no-change" result (which is supposed to be 0). Changing it to -1 resolves the problem.

It should work for all other browsers, but i've limited this change to Chrome-only, which required new property in CKEDITOR.env.js.

[Tobiasz Cudnik]

There are still issues with newest Chrome 3 and Safari 3. I will post next patch after more extensive tests.

[Tobiasz Cudnik]

Affected are:

• Chrome 3
• Chrome 2
• Safari 3
• FF 2 (in other way)

Attaching screenshots of all.

[Tobiasz Cudnik]

I've decided to implement sorting manually. Thanks to that it works same way in all browsers. I've used bubble sort, because of low amount of data which going to be sorted.

This solution doesn't needed new CKEDITOR.env property.

[Alfonso Martínez de Lizarrondo]

The reason of this problem is that for Chrome the sort function is not stable. That means that when two elements compare as equal in a custom sort, as it does happen in this situation, the behavior is undefined. It doesn't have to keep the original order.

They explain it briefly in ​http://code.google.com/p/v8/issues/detail?id=90 which points to ​https://bugzilla.mozilla.org/show_bug.cgi?id=224128 that was checked for Firefox 3, so that explains also the difference with Firefox 2.

So that means that it's not a browser bug, just a difference of opinions; it can't be detected, as the behavior is undefined, the test might say that it has been sorted correctly, but it was just a lucky situation; and the solutions are to add a third key to provide the default sort or use a custom function like patch 2.

A simple script to test it:

function dump(data)
{
	document.write("<p>Contents of the array: ")
	for(var i=0; i<data.length; i++)
	{
		document.write(data[i].name);
		if (i<(data.length-1)) document.write(", ");
	}
	document.write("</p>");
}
var simpleArray = [ {name:'enable (1)'}, {name:'options (2)'}, {name:'languages (3)'}, {name:'about (4)'}]
	dump(simpleArray);
		simpleArray.sort( function( itemA, itemB )
			{
				return 0; // No sort
			});
	dump(simpleArray);

[Garry Yao]

It seems for me the menu item array is immutable, if that's true, we could easily alter the CKEDITOR.menu::add function on L107 to build a second column for used as the secondary comparison key:

add : function( item )
{
	// Later we may sort the items, but Array#sort is not stable in
	// some browsers, here we're forcing the original sequence with
	// 'order' attribute if it hasn't been assigned.
	if ( !item.order )
		item.order = this.items.length;
	this.items.push( item );
},

[Tobiasz Cudnik]

Garry's solution is best way in my opinion, so i've made a patch with it, after checking in all browsers.

[Tobiasz Cudnik]

Fixed with [3824].