Closed Bug 1403318 Opened 6 years ago Closed 6 years ago
Intl.PluralRules reached Stage 4 at today's TC39 meeting. We can now expose it on the public Intl object!
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
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.
(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!)
thank you :) try looks green!
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 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 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)
Anba: I went ahead and incorporated all three of your feedback suggestions into the patch. Hope it's ok with you :)
(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 on attachment 8912807 [details] Bug 1403318 - Remove mozIntl.PluralRules. Seems fine to me; sorry for the delay.
Attachment #8912807 - Flags: review?(jfkthame) → review+
Pushed by email@example.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
You need to log in before you can comment on or make changes to this bug.