Update Intl legacy compromise semantics to match latest spec

RESOLVED FIXED in Firefox 54

Status

()

Core
JavaScript: Internationalization API
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: anba, Assigned: anba)

Tracking

Trunk
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

11 months ago
The spec was updated to fix some minor issues, let's update our implementation accordingly.
(Assignee)

Comment 1

11 months ago
Created attachment 8836669 [details] [diff] [review]
bug1339032.patch

This updates the Intl legacy constructor semantics with the changes from https://github.com/tc39/ecma402/commit/7eb2c2b149ede6c192d9ba683191b7f8d7d05158 and https://github.com/tc39/ecma402/pull/127 (adds [[Description]] to the Intl fallback symbol).
Attachment #8836669 - Flags: review?(jwalden+bmo)
Comment on attachment 8836669 [details] [diff] [review]
bug1339032.patch

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

::: js/src/tests/Intl/DateTimeFormat/unwrapping.js
@@ -109,5 @@
>                  hasInstanceCalled = true;
>                  return true;
>              }, configurable: true
>          });
> -        let isUndefinedOrNull = thisValue !== undefined || thisValue !== null;

Erk.  This condition wasn't written correctly!  This variable would always have been true.

::: js/src/tests/Intl/NumberFormat/unwrapping.js
@@ -111,5 @@
>                  hasInstanceCalled = true;
>                  return true;
>              }, configurable: true
>          });
> -        let isUndefinedOrNull = thisValue !== undefined || thisValue !== null;

This was buggy too.
Attachment #8836669 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 4

11 months ago
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #2)
> > -        let isUndefinedOrNull = thisValue !== undefined || thisValue !== null;
> 
> Erk.  This condition wasn't written correctly!  This variable would always
> have been true.

Psst, I tried to fix this one without attracting a lot of attention. :-)

Comment 5

11 months ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0429b0b495da
Update Intl legacy constructor compromise semantics per the latest spec changes. r=Waldo
Keywords: checkin-needed

Comment 6

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0429b0b495da
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
(In reply to André Bargull from comment #4)
> Psst, I tried to fix this one without attracting a lot of attention. :-)

I am wise to your ways.  :-)  The reviewer who let that in is clearly not to be trusted.
You need to log in before you can comment on or make changes to this bug.