remove or replace PluralForm.jsm

RESOLVED FIXED in Firefox 51

Status

enhancement
P1
normal
RESOLVED FIXED
3 years ago
11 months ago

People

(Reporter: tromey, Assigned: jdescottes)

Tracking

(Depends on 2 bugs)

unspecified
Firefox 51
Dependency tree / graph
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox51 fixed)

Details

(Whiteboard: [devtools-html])

Attachments

(2 attachments)

Reporter

Description

3 years ago
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.
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.
Reporter

Comment 2

3 years ago
(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.
Depends on: 1266075
Flags: qe-verify-
Priority: -- → P2
No longer blocks: devtools-html-3
Priority: P2 → --
Whiteboard: [devtools-html] → [devtools-html] [triage]
Whiteboard: [devtools-html] [triage] → [devtools-html]
Priority: -- → P2
Assignee

Updated

3 years ago
Blocks: 1290947
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
Assignee

Updated

3 years ago
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: P2 → P1
Iteration: --- → 51.2 - Aug 29
Assignee

Comment 5

3 years ago
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.
Attachment #8783971 - Flags: feedback?(ttromey)
Reporter

Comment 6

3 years ago
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.
Attachment #8783971 - Flags: feedback?(ttromey) → feedback+
Assignee

Comment 7

3 years ago
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.
Depends on: 1298012
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Reporter

Comment 11

3 years ago
mozreview-review
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.
Attachment #8783971 - Flags: review?(ttromey) → review+
Reporter

Comment 12

3 years ago
mozreview-review
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.
Attachment #8785910 - Flags: review?(ttromey) → review+
Thanks for the reviews Tom! Green try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e8bdccbfcaf, landing.

Comment 14

3 years ago
Pushed by jdescottes@mozilla.com:
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

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cb74e60bc40a
https://hg.mozilla.org/mozilla-central/rev/2c763524c0b6
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51

Updated

3 years ago
Depends on: 1301933

Updated

11 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.