Bug 47825 - Different behaviour with the .sort(callback) method (unlike Firefox & Chrome)
Summary: Different behaviour with the .sort(callback) method (unlike Firefox & Chrome)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel All
: P2 Minor
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-10-18 08:55 PDT by webkit
Modified: 2017-05-17 13:49 PDT (History)
13 users (show)

See Also:


Attachments
Patch (2.95 KB, patch)
2017-05-02 15:21 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (4.38 KB, patch)
2017-05-02 15:22 PDT, Keith Miller
no flags Details | Formatted Diff | Diff
benchmark results (87.26 KB, text/plain)
2017-05-02 15:24 PDT, Keith Miller
no flags Details
Archive of layout-test-results from ews100 for mac-elcapitan (1.03 MB, application/zip)
2017-05-02 16:09 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.10 MB, application/zip)
2017-05-02 16:15 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-elcapitan (1.72 MB, application/zip)
2017-05-02 16:44 PDT, Build Bot
no flags Details
Patch for landing (5.97 KB, patch)
2017-05-03 12:50 PDT, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description webkit 2010-10-18 08:55:57 PDT
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 ?
Comment 1 Alexey Proskuryakov 2010-10-18 15:34:12 PDT
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.
Comment 2 Erik Arvidsson 2010-10-18 15:50:57 PDT
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.
Comment 3 Alexey Proskuryakov 2010-10-18 16:17:24 PDT
The original report says that Opera matches Safari, if I understood it correctly.
Comment 4 Erik Arvidsson 2010-10-18 16:25:37 PDT
(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}]
Comment 5 webkit 2010-10-19 00:41:28 PDT
I was testing with Opera 10.10 which return:
[{e:1}, {e:0}, {e:1}]
Comment 6 David Kilzer (:ddkilzer) 2016-10-06 09:34:45 PDT
Also seen here:  <http://kangax.github.io/compat-table/es5/>
Comment 7 Radar WebKit Bug Importer 2016-10-06 09:35:28 PDT
<rdar://problem/28653104>
Comment 8 Keith Miller 2016-10-06 14:20:01 PDT
(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
Comment 9 Keith Miller 2017-05-02 15:21:01 PDT
Created attachment 308856 [details]
Patch
Comment 10 Keith Miller 2017-05-02 15:22:50 PDT
Created attachment 308858 [details]
Patch
Comment 11 Keith Miller 2017-05-02 15:24:39 PDT
Created attachment 308859 [details]
benchmark results

sorting-benchmark didn't repro on subsequent runs.
Comment 12 Mark Lam 2017-05-02 15:27:58 PDT
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/
Comment 13 Build Bot 2017-05-02 16:09:36 PDT
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
Comment 14 Build Bot 2017-05-02 16:09:38 PDT
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
Comment 15 Build Bot 2017-05-02 16:15:50 PDT
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
Comment 16 Build Bot 2017-05-02 16:15:51 PDT
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
Comment 17 Build Bot 2017-05-02 16:44:19 PDT
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
Comment 18 Build Bot 2017-05-02 16:44:20 PDT
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
Comment 19 Keith Miller 2017-05-03 12:50:56 PDT
Created attachment 308940 [details]
Patch for landing
Comment 20 WebKit Commit Bot 2017-05-03 13:33:05 PDT
Comment on attachment 308940 [details]
Patch for landing

Clearing flags on attachment: 308940

Committed r216137: <http://trac.webkit.org/changeset/216137>
Comment 21 WebKit Commit Bot 2017-05-03 13:33:07 PDT
All reviewed patches have been landed.  Closing bug.