1325459 - Expose PluralRules on mozIntl

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

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

With bug 1270146 fixed, we should now expose PluralRules on mozIntl for internal use until we get it exposed on Intl for web content.

Comment 1

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

Ugh, sorry to bother you :waldo, but I have no idea how to approach this. So far we've only exposed functions on mozIntl, never objects, and the way `AddIntlExtras` handled them was via `JS_SELF_HOSTED_FN`, which won't work here I believe.

Instead, we need to get sth like:

RootedObject pluralRulesProto(cx, CreatePluralRulesPrototype(cx, intl, global));
global->setReservedSlot(PLURAL_RULES_PROTO, ObjectValue(*pluralRulesProto));

but in AddIntlExtras we don't have global, and we also don't have `CreatePluralRulesPrototype` or PLURAL_RULES_PROTO in js.cpp.

How should I approach it?

Comment 2

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

Attachment: Bug 1325459 - Expose PluralRules via addIntlExtras.

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

Comment 3

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

Comment on attachment 8821462 [details]
Bug 1325459 - Expose PluralRules via addIntlExtras.

I hacked and slashed my way through the code to get it in the jsshell.cpp.

I still don't know if you're ok with leaving it exposed (we don't have any plans to change the API and we do intend to push it to stage4 in January), or would you prefer to move it to behind addIntlExtras and mozIntl.

If the latter, I'll also need a bit of help in getting the CreatePluralRulesPrototype and PLURAL_RULES_PROTO in http://searchfox.org/mozilla-central/source/toolkit/components/mozintl/MozIntl.cpp - and I don't know how to get it.

Comment 4

[Jeff Walden [:Waldo]]

Attachment: Somewhat-different tack

The existing patch mostly gets the job done, but I'm not happy about exposing global object slots, nor about the awkward interface of requiring callers to manually fill in a slot after PluralRules creation.  And there's the mozIntl thing as well.  This should cover all the bases.

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 8821462 [details]
Bug 1325459 - Expose PluralRules via addIntlExtras.

https://reviewboard.mozilla.org/r/100758/#review101408

It's a good start, but I want slightly different interface boundaries -- we should expose PluralRules-adding functionality by a function, not by a function to create it all and the implicit step of setting the reserved slot.  I took the patch and massaged things into the right form -- one that also handles mozIntl pretty easily, too.

Comment 6

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

Comment on attachment 8821462 [details]
Bug 1325459 - Expose PluralRules via addIntlExtras.

sweet, thanks a lot Waldo!

Comment 7

[André Bargull [:anba]]

Comment on attachment 8821649 [details] [diff] [review]
Somewhat-different tack

Review of attachment 8821649 [details] [diff] [review]:
-----------------------------------------------------------------

Works for me as long as it's not necessary to install PluralRules to two objects (mozIntl and Intl). 

To get Intl.PluralRules to stage 4, we need two shipping implementations [1], but with this change we only make PluralRules available internally. Is this intended?

[1] https://tc39.github.io/process-document/

Comment 8

[Jeff Walden [:Waldo]]

The separate installation modes are for the shell, and separately for browser chrome code that wants to do plural stuff.  Shouldn't be any overlap of uses.  And the mozIntl idea is "you give me an object, I create and put the PluralRules constructor on it", so it'd only go on that one object.

This is maybe fuzzy, but I think we interpret "shipping" to mean "the functionality can be invoked in some way and tested for correctness to the extent possible".  Back in the day, a prefixed implementation would have counted, even though it wasn't exactly the same signature in how you accessed it.  Intl.js the polyfill counts as an implementation, even tho (obviously) using it requires an extra step that ESmumble doesn't have.  Making someone call |addIntlExtras(Intl)| before doing stuff is in the same vein, IMO.

Comment 9

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

Yeah, I'm pretty sure that MozIntl works as a mean to stage 4. The goal of having two implementation is to test if the algo can be implemented and catch any gotchas in the process. We've done it.
I'll recommend exposing it on Intl once we have stage 4.

Comment 10

[André Bargull [:anba]]

Ah okay, I see. My understanding of "Significant in-the-field experience with shipping implementations [...]" was that a proposed feature is implemented and also exposed to web-content (or available when a about:config flag is set).

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4dd4f25303c
Expose the PluralRules constructor only by explicit action,, for now.  r=anba

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b4dd4f25303c