Closed
Bug 1397925
Opened 7 years ago
Closed 7 years ago
Temporarily add a hook to LocaleService to refresh OSPreferences on intl.locale.os change
Categories
(Core :: Internationalization, defect)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: zbraniecki, Assigned: zbraniecki)
References
Details
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
rnewman
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
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.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment 2•7 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•7 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`.
Comment hidden (mozreview-request) |
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cf262990357e Refresh OSPreferences mSystemLocales when intl.locale.os changes. r=rnewman
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cf262990357e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 7•7 years ago
|
||
Should this uplift to beta? If so can you request it so we get it into the next build? Thanks.
status-firefox56:
--- → ?
Flags: needinfo?(gandalf)
Assignee | ||
Comment 8•7 years ago
|
||
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?
Updated•7 years ago
|
tracking-firefox56:
--- → +
Comment 9•7 years ago
|
||
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+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 10•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/aa3740564226
Keywords: checkin-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•