Bug 47825 – Different behaviour with the .sort(callback) method (unlike Firefox & Chrome)

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

Overview: 
Webkit doesn't seem to correctly handle sort's callback that return a boolean. 

Test case:
[{e:1}, {e:0}, {e:1}].sort(function(a,b){return a.e < b.e});

Actual Results: (it does "nothing")
[{e:1}, {e:0}, {e:1}]

Expected Results (tested with Chromium 6 & Firefox 3.6):
[{e:1}, {e:1}, {e:0}]

Additional Information:
Opera does nothing too.

Hum, after reading ECMA-262 "15.4.4.11 Array.prototype.sort (comparefn)"

"If comparefn is not undefined and is not a consistent comparison function for the elements of this array (see
below), the behaviour of sort is implementation-defined."
I don't know what "implementation-defined" mean in this case but if chromium & firefox do sort this way, maybe it would be a good idea to do the same ?

[Alexey Proskuryakov]

What does Internet Explorer do?

I've been told that V8 is expected to be bug-to-bug compatible with JavaScriptCore, so Chromium is supposed to to the same as other WebKit ports. If IE matches Safari and Opera, than it's Firefox that's the odd one out.

[Erik Arvidsson]

Internet Explorer, Chrome, Firefox and Opera all returns:

[{"e":1},{"e":1},{"e":0}]

Given that I think it is safer to change JSC than V8.

[Alexey Proskuryakov]

The original report says that Opera matches Safari, if I understood it correctly.

[Erik Arvidsson]

(In reply to comment #3)
> The original report says that Opera matches Safari, if I understood it correctly.

I just double checked and Opera 10.60 returns

[{"e":1},{"e":1},{"e":0}]

I was testing with Opera 10.10 which return:
[{e:1}, {e:0}, {e:1}]

[David Kilzer (:ddkilzer)]

Also seen here:  <http://kangax.github.io/compat-table/es5/>

[Radar WebKit Bug Importer]

<rdar://problem/28653104>

[Keith Miller]

(In reply to comment #6)
> Also seen here:  <http://kangax.github.io/compat-table/es5/>

This seems to be a different issue from the Kangex table bug. I opened a new bug for that problem: https://bugs.webkit.org/show_bug.cgi?id=163085

[Keith Miller]

Created attachment 308856 [details]
Patch

[Keith Miller]

Created attachment 308858 [details]
Patch

[Keith Miller]

Created attachment 308859 [details]
benchmark results

sorting-benchmark didn't repro on subsequent runs.

[Mark Lam]

Comment on attachment 308858 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308858&action=review

> Source/JavaScriptCore/ChangeLog:11
> +        JQuery sorting of DOM nodes by 30%. The regression was do to the

typo: /do/due/

[Build Bot]

Comment on attachment 308858 [details]
Patch

Attachment 308858 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3661123

New failing tests:
http/tests/inspector/worker/blob-script-with-cross-domain-imported-scripts.html

[Build Bot]

Created attachment 308864 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 308858 [details]
Patch

Attachment 308858 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3661134

New failing tests:
http/tests/inspector/worker/blob-script-with-cross-domain-imported-scripts.html

[Build Bot]

Created attachment 308865 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 308858 [details]
Patch

Attachment 308858 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3661227

New failing tests:
http/tests/inspector/worker/blob-script-with-cross-domain-imported-scripts.html

[Build Bot]

Created attachment 308866 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Keith Miller]

Created attachment 308940 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 308940 [details]
Patch for landing

Clearing flags on attachment: 308940

Committed r216137: <http://trac.webkit.org/changeset/216137>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.