Last Comment Bug 789393 - String.prototype.localeCompare() with no argument always returns 0
: String.prototype.localeCompare() with no argument always returns 0
Status: RESOLVED FIXED
[js:p3]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla21
Assigned To: Norbert Lindenberg
:
:
Mentors:
Depends on: 839751
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-06 22:52 PDT by Norbert Lindenberg
Modified: 2013-02-23 23:31 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (3.02 KB, patch)
2013-02-07 00:07 PST, Norbert Lindenberg
jwalden+bmo: review+
Details | Diff | Splinter Review
patch (3.07 KB, patch)
2013-02-07 17:23 PST, Norbert Lindenberg
mozillabugs: review+
Details | Diff | Splinter Review

Description Norbert Lindenberg 2012-09-06 22:52:38 PDT
If no argument is passed to String.prototype.localeCompare, the function returns 0 no matter what the string obtained from this is.

According to ES5 clause 15, fourth paragraph, "if a function or constructor described in this clause is given fewer arguments than the function is specified to require, the function or constructor shall behave exactly as if it had been given sufficient additional arguments, each such argument being the undefined value."

ToString(undefined) is "undefined", so the function should treat a missing first argument as that string.

Current versions of Safari, Chrome, and Opera have the same bug; Internet Explorer 9 follows the specification.
Comment 1 Norbert Lindenberg 2013-02-07 00:07:44 PST
Created attachment 711188 [details] [diff] [review]
patch

I'd rather fix this separately than as a side effect of implementing the ECMAScript Internationalization API Specification.

I'm also adding a test case to test262, the ECMA-262 conformance test suite. For more information, see
https://bugs.ecmascript.org/show_bug.cgi?id=1245
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2013-02-07 16:54:41 PST
Comment on attachment 711188 [details] [diff] [review]
patch

Review of attachment 711188 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, this definitely should get pulled out of all that.  Two little things to fix and this is fine.

::: js/src/jit-test/tests/basic/testLocaleCompare.js
@@ +1,1 @@
> +assertEq("a".localeCompare(), "a".localeCompare("undefined"));

Add an:

  assertEq("undefined".localeCompare(), 0);

to better test the actual implemented behavior here.

::: js/src/jsstr.cpp
@@ +783,1 @@
>              return true;

Uh...this should be return false.  I'm not sure how that got there before!
Comment 3 Norbert Lindenberg 2013-02-07 17:23:28 PST
Created attachment 711615 [details] [diff] [review]
patch

Updated per comment 2. Carrying r+jwalden.
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2013-02-07 18:28:18 PST
Comment on attachment 711615 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/e6f017ce103b
Comment 5 Ed Morley [:emorley] 2013-02-08 03:06:08 PST
https://hg.mozilla.org/mozilla-central/rev/e6f017ce103b
Comment 6 Kohei Yoshino [:kohei] 2013-02-23 23:31:32 PST
I've added this bug to the compatibility doc. Please correct the info if I'm wrong.
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_21

Note You need to log in before you can comment on or make changes to this bug.