1265887 - remove or replace PluralForm.jsm

Status: RESOLVED FIXED

(DevTools :: Framework, enhancement, P1)

Description

[Tom Tromey :tromey]

PluralForm.jsm is loaded in the inspector but perhaps not used.
It should either be removed or replaced for the devtools
de-chrome-ification project.

Comment 1

[Alexandre Poirot [:ochameau]]

Do we have a bug for Services.Strings.createBundle ?
PluralForm is part of the localization as well as createBundle. I imagine we need a coherent migration for all l10n APIs.

Comment 2

[Tom Tromey :tromey]

(In reply to Alexandre Poirot [:ochameau] from comment #1)
> Do we have a bug for Services.Strings.createBundle ?

I just made one.
I think maybe this will be handled by some other track, not sure though.

Comment 3

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

We're working on Intl.PluralRules API, but it'll be initially chrome-only.

You could consider using a polyfill - https://github.com/eemeli/IntlPluralRules

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1265887 - migrate devtools/ from PluralForm.jsm to plural-form.js;

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

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8783971 [details]
Bug 1265887 - migrate devtools/ from PluralForm.jsm to plural-form.js;

Tom, can you let me know what you think about this approach? I simply took the existing PluralForm jsm, removed the Chrome privileged APIs. 

The only thing that is a bit fishy IMO is that we are relying on a localized string outside of the devtools scope to know the plural rules that should be used. This will work fine when running in chrome, but when running in webpack/content, we won't have access to the file, so for now I decided to default to english.

Comment 6

[Tom Tromey :tromey]

Comment on attachment 8783971 [details]
Bug 1265887 - migrate devtools/ from PluralForm.jsm to plural-form.js;

I think depending on that is ok.  IIRC there are some other dependencies
on non-devtools property files as well.
It seems to me that we can either bundle these files or replace them with
our own shims without too much effort.

Comment 7

[Julian Descottes [:jdescottes]]

Thanks for the feedback ! FYI, I opened a bug to take a closer look at non-devtools dependencies: Bug 1297733

Currently blocking this on Bug 1298012. We need to move l10n.js to devtools/shared so that plural-form.js can also be moved there and be used by files in devtools/shared.

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: Bug 1265887 - Port PluralForm.jsm to plural-form.js (no chrome-privileged APIs);

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

Comment 9

[Julian Descottes [:jdescottes]]

Comment on attachment 8783971 [details]
Bug 1265887 - migrate devtools/ from PluralForm.jsm to plural-form.js;

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

Comment 10

[Julian Descottes [:jdescottes]]

Comment on attachment 8783971 [details]
Bug 1265887 - migrate devtools/ from PluralForm.jsm to plural-form.js;

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

Comment 11

[Tom Tromey :tromey]

Comment on attachment 8783971 [details]
Bug 1265887 - migrate devtools/ from PluralForm.jsm to plural-form.js;

https://reviewboard.mozilla.org/r/73588/#review72822

Thank you.  This looks good.

Comment 12

[Tom Tromey :tromey]

Comment on attachment 8785910 [details]
Bug 1265887 - Port PluralForm.jsm to plural-form.js (no chrome-privileged APIs);

https://reviewboard.mozilla.org/r/74756/#review72832

Thank you.  This is fine.

Comment 13

[Julian Descottes [:jdescottes]]

Thanks for the reviews Tom! Green try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e8bdccbfcaf, landing.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/cb74e60bc40a
port PluralForm.jsm to plural-form.js without chrome APIs;r=tromey
https://hg.mozilla.org/integration/fx-team/rev/2c763524c0b6
migrate devtools/ from PluralForm.jsm to plural-form.js;r=tromey

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cb74e60bc40a
https://hg.mozilla.org/mozilla-central/rev/2c763524c0b6