Closed
Bug 1403318
Opened 6 years ago
Closed 6 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•6 years ago
|
Keywords: dev-doc-needed
|
Assignee | |
Updated•6 years ago
|
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Updated•6 years ago
|
Attachment #8912806 -
Flags: review?(andrebargull)
Attachment #8912807 -
Flags: review?(andrebargull)
|
|
Assignee | |
Comment 3•6 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•6 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•6 years ago
|
||
thank you :) try looks green!
|
|
||
Comment 10•6 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•6 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•6 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•6 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•6 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•6 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•6 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•6 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/0b2271bd6e50 https://hg.mozilla.org/mozilla-central/rev/72af2f1919fe
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
|
|
||
Comment 20•6 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
•