Closed Bug 1339032 Opened 7 years ago Closed 7 years ago

Update Intl legacy compromise semantics to match latest spec

Categories

(Core :: JavaScript: Internationalization API, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: anba, Assigned: anba)

Details

Attachments

(1 file)

The spec was updated to fix some minor issues, let's update our implementation accordingly.
Attached patch bug1339032.patchSplinter Review
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+
(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. :-)
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
https://hg.mozilla.org/mozilla-central/rev/0429b0b495da
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
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.

Attachment

General

Created:
Updated:
Size: