Closed
Bug 1403318
Opened 7 years ago
Closed 7 years ago
Expose Intl.PluralRules
Categories
(Core :: JavaScript: Internationalization API, enhancement)
Core
JavaScript: Internationalization API
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: zbraniecki, Assigned: zbraniecki)
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
Intl.PluralRules reached Stage 4 at today's TC39 meeting.
We can now expose it on the public Intl object!
Updated•7 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8912806 -
Flags: review?(andrebargull)
Attachment #8912807 -
Flags: review?(andrebargull)
Assignee | ||
Comment 3•7 years ago
|
||
Andre, can you take a look at this?
I'd also appreciate any tips on why would this be failing at the moment on test262 tests despite the fact that I did expose it and the tests should now be passing unconditionally.
Comment 4•7 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #3)
> I'd also appreciate any tips on why would this be failing at the moment on
> test262 tests despite the fact that I did expose it and the tests should now
> be passing unconditionally.
The test262 importer adds code to manually install PluralRules into the global Intl object, and I guess with the current patch it probably just replaces Intl.PluralRules with the `undefined` value. I think the following changes should make test262 work again:
- Delete js/src/tests/test262-intl-extras.js
- Remove these two lines http://searchfox.org/mozilla-central/rev/f54c1723befe6bcc7229f005217d5c681128fcad/js/src/tests/test262-update.py#324-325
- Remove all lines in js/src/tests/test262/intl402/PluralRules/shell.js (but don't delete the file itself!)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•7 years ago
|
||
thank you :)
try looks green!
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8912806 [details]
Bug 1403318 - Expose Intl.PluralRules.
https://reviewboard.mozilla.org/r/184114/#review190086
LGTM!
The `addIntlExtras` guard and call should also be removed from this file: http://searchfox.org/mozilla-central/rev/20e41d4a61a8f5e34c9cf357304b78b3e9bced8a/js/src/tests/Intl/PluralRules/supportedLocalesOf.js#1,365
And the `"Calling this function more than once in a realm/global will throw."` warning for addIntlExtras's help message can now be removed, too. http://searchfox.org/mozilla-central/rev/20e41d4a61a8f5e34c9cf357304b78b3e9bced8a/js/src/shell/js.cpp#6695-6696
Not directly related to this change, but it looks like the `PluralRulesSelect` entry in vm/CommonPropertyNames.h is no longer needed: http://searchfox.org/mozilla-central/rev/20e41d4a61a8f5e34c9cf357304b78b3e9bced8a/js/src/vm/CommonPropertyNames.h#287
Attachment #8912806 -
Flags: review?(andrebargull) → review+
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8912807 [details]
Bug 1403318 - Remove mozIntl.PluralRules.
https://reviewboard.mozilla.org/r/184116/#review190090
The changes look reasonable to me, but since I don't really know how idl-files are processed, this is more like a rs=me than a proper r=me :-)
Attachment #8912807 -
Flags: review?(andrebargull) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•7 years ago
|
||
Comment on attachment 8912807 [details]
Bug 1403318 - Remove mozIntl.PluralRules.
Jonathan, can you support anba's review with yours? It's a pretty simple change to remove the special treatment of mozIntl.PluralRules since we now can expose it as Intl.PluralRules.
Attachment #8912807 -
Flags: review?(jfkthame)
Assignee | ||
Comment 15•7 years ago
|
||
Anba: I went ahead and incorporated all three of your feedback suggestions into the patch. Hope it's ok with you :)
Comment 16•7 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #15)
> Anba: I went ahead and incorporated all three of your feedback suggestions
> into the patch. Hope it's ok with you :)
Cool, thanks! :-)
Comment 17•7 years ago
|
||
Comment on attachment 8912807 [details]
Bug 1403318 - Remove mozIntl.PluralRules.
Seems fine to me; sorry for the delay.
Attachment #8912807 -
Flags: review?(jfkthame) → review+
Comment 18•7 years ago
|
||
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0b2271bd6e50
Expose Intl.PluralRules. r=anba
https://hg.mozilla.org/integration/autoland/rev/72af2f1919fe
Remove mozIntl.PluralRules. r=anba
Comment 19•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0b2271bd6e50
https://hg.mozilla.org/mozilla-central/rev/72af2f1919fe
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment 20•7 years ago
|
||
Developer release notes:
https://developer.mozilla.org/en-US/Firefox/Releases/58#JavaScript
Reference pages:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/PluralRules
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/PluralRules/prototype
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/PluralRules/resolvedOptions
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/PluralRules/select
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/PluralRules/supportedLocalesOf
Compat data update:
https://github.com/mdn/browser-compat-data/pull/669
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•