789393 - String.prototype.localeCompare() with no argument always returns 0

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Norbert Lindenberg]

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]

Attachment: 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]]

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]

Attachment: patch

Updated per comment 2. Carrying r+jwalden.

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 711615 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/e6f017ce103b

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e6f017ce103b

Comment 6

[Kohei Yoshino]

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