1403318 - Expose Intl.PluralRules

Status: RESOLVED FIXED

(Core :: JavaScript: Internationalization API, enhancement)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

Intl.PluralRules reached Stage 4 at today's TC39 meeting.

We can now expose it on the public Intl object!

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1403318 - Expose Intl.PluralRules.

Review commit: https://reviewboard.mozilla.org/r/184114/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184114/

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1403318 - Remove mozIntl.PluralRules.

Review commit: https://reviewboard.mozilla.org/r/184116/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184116/

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

[André Bargull [:anba]]

(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 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8912806 [details]
Bug 1403318 - Expose Intl.PluralRules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184114/diff/1-2/

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8912807 [details]
Bug 1403318 - Remove mozIntl.PluralRules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184116/diff/1-2/

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8912806 [details]
Bug 1403318 - Expose Intl.PluralRules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184114/diff/2-3/

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8912807 [details]
Bug 1403318 - Remove mozIntl.PluralRules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184116/diff/2-3/

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

 thank you :)
try looks green!

Comment 10

[André Bargull [:anba]]

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

Comment 11

[André Bargull [:anba]]

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 :-)

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8912806 [details]
Bug 1403318 - Expose Intl.PluralRules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184114/diff/3-4/

Comment 13

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8912807 [details]
Bug 1403318 - Remove mozIntl.PluralRules.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184116/diff/3-4/

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

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.

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

Anba: I went ahead and incorporated all three of your feedback suggestions into the patch. Hope it's ok with you :)

Comment 16

[André Bargull [:anba]]

(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

[Jonathan Kew [:jfkthame]]

Comment on attachment 8912807 [details]
Bug 1403318 - Remove mozIntl.PluralRules.

Seems fine to me; sorry for the delay.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
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

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0b2271bd6e50
https://hg.mozilla.org/mozilla-central/rev/72af2f1919fe

Comment 20

[Florian Scholz (Open Web Docs)]

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