Temporarily add a hook to LocaleService to refresh OSPreferences on intl.locale.os change

RESOLVED FIXED in Firefox 56

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: zbraniecki, Assigned: zbraniecki)

Tracking

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56+ fixed, firefox57 fixed)

Details

Attachments

(1 attachment)

LocaleService and OSPreferences design assumes that OSPreferences is the only place responsible for maintaining the data about OS locales.

Android is the only platform that has hooks for updating when OS locale is changing, and it uses a pref `intl.locale.os` for that.

In bug 1337078 we wanted to switch away from the pref to a FFI binding, so we waited for it and also for bug 1352343 to establish proper event flow.

That unfortunately means that until those two bugs are fixed we don't really react to OS locale changes on fly.

One way to fix it (more within the design) would be to turn OSPreferences into an nsIObserver and observe `intl.locale.os` there. Then land bug 1352343.

The other, simpler fix is to place an OSPreferences::Refresh into LocaleService observer.

Richard is looking into bug 1337078 today, and if he agrees with it being ok to land in 58, I'd prefer to land a simple dirty hack for now to avoid having to switch OSPreferences into an nsIObserver.
Then in 58 I'll lang the Android FFI patch, bug 1352343 and remove the hack.

If Richard will decide that intl.locale.os should stay as a permanent way of communicating between Java code and LocaleService, I'll write a bigger patch to transition OSPreferences.
Assignee: nobody → gandalf
Status: NEW → ASSIGNED

Comment 2

2 years ago
mozreview-review
Comment on attachment 8905689 [details]
Bug 1397925 - Refresh OSPreferences mSystemLocales when intl.locale.os changes.

https://reviewboard.mozilla.org/r/177488/#review182802
Attachment #8905689 - Flags: review?(rnewman) → review+

Comment 3

2 years ago
mozreview-review
Comment on attachment 8905689 [details]
Bug 1397925 - Refresh OSPreferences mSystemLocales when intl.locale.os changes.

https://reviewboard.mozilla.org/r/177488/#review182804

r+ with a nit -- there's a typo. You've used `prefs`, not `pref`.
See Also: → 1398209
Comment hidden (mozreview-request)

Comment 5

2 years ago
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cf262990357e
Refresh OSPreferences mSystemLocales when intl.locale.os changes. r=rnewman
https://hg.mozilla.org/mozilla-central/rev/cf262990357e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Should this uplift to beta? If so can you request it so we get it into the next build? Thanks.
Flags: needinfo?(gandalf)
Comment on attachment 8905689 [details]
Bug 1397925 - Refresh OSPreferences mSystemLocales when intl.locale.os changes.

Approval Request Comment
[Feature/Bug causing the regression]: Not a regression
[User impact if declined]: Gecko strings not updated when Android OS locale changes.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: Bug 1378501
[Is the change risky?]: no
[Why is the change risky/not risky?]: it's a minor fix that makes us refresh OS locales when a pref changes.
[String changes made/needed]: none
Flags: needinfo?(gandalf)
Attachment #8905689 - Flags: approval-mozilla-beta?
Comment on attachment 8905689 [details]
Bug 1397925 - Refresh OSPreferences mSystemLocales when intl.locale.os changes.

Fix verified in bug 1378501. Let's uplift for beta 12/13
Attachment #8905689 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.