The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla21

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Norbert Lindenberg, Assigned: Norbert Lindenberg)

Tracking

unspecified
mozilla21
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:p3])

Attachments

(1 attachment, 1 obsolete attachment)

3.07 KB, patch
Norbert Lindenberg
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
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.
Whiteboard: [js:p3]
(Assignee)

Comment 1

4 years ago
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
Assignee: general → mozillabugs
Status: NEW → ASSIGNED
Attachment #711188 - Flags: review?(jwalden+bmo)
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!
Attachment #711188 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 3

4 years ago
Created attachment 711615 [details] [diff] [review]
patch

Updated per comment 2. Carrying r+jwalden.
Attachment #711188 - Attachment is obsolete: true
Attachment #711615 - Flags: review+
Attachment #711615 - Flags: checkin?(jwalden+bmo)
Comment on attachment 711615 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/e6f017ce103b
Attachment #711615 - Flags: checkin?(jwalden+bmo)
https://hg.mozilla.org/mozilla-central/rev/e6f017ce103b
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21

Updated

4 years ago
Depends on: 839751
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
You need to log in before you can comment on or make changes to this bug.