Closed
Bug 1409185
Opened 7 years ago
Closed 7 years ago
Generalize language-matching for date/time patterns in OSPreferences
Categories
(Core :: Internationalization, enhancement, P3)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: zbraniecki, Assigned: zbraniecki)
References
Details
Attachments
(1 file)
Currently, Windows code has a special code that allows us to look into OS settings for a given locale if language part matches between OS regional prefs locale and appLocale. We can generalize this code to make it work on all platforms making Firefox in en-US use en-GB dates on Mac and Linux.
Assignee | ||
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Priority: -- → P3
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8919052 [details] Bug 1409185 - Generalize language-matching for date/time patterns in OSPreferences. https://reviewboard.mozilla.org/r/189966/#review195884 ::: intl/locale/LocaleService.cpp:251 (Diff revision 2) > + // This facilitates scenarios such as Firefox in "en-US" and User sets > + // regional prefs to "en-GB". > + nsAutoCString appLocale; > + AutoTArray<nsCString, 10> regionalPrefsLocales; > + LocaleService::GetInstance()->GetAppLocaleAsBCP47(appLocale); > + OSPreferences::GetInstance()->GetRegionalPrefsLocales(regionalPrefsLocales); OSPreferences::GetRegionalPrefsLocales() can fail and return an empty list, AFAICS; therefore, we need to check its return value and not just assume we can safely look at regionalPrefsLocales[0] below. ::: intl/locale/windows/OSPreferences_win.cpp:99 (Diff revision 2) > { > - nsAutoCString reqLocale; > + UTF8ToUnicodeBuffer(aLocale, (char16_t*)aBuffer); The GetWindowsLocaleFor() function is no longer doing enough to justify its existence. :) Just do the UTF8ToUnicodeBuffer operation directly at the call site.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
Thanks Jonathan! :)
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8919052 [details] Bug 1409185 - Generalize language-matching for date/time patterns in OSPreferences. https://reviewboard.mozilla.org/r/189966/#review196044 ::: intl/locale/windows/OSPreferences_win.cpp:119 (Diff revision 3) > bool > OSPreferences::ReadDateTimePattern(DateTimeFormatStyle aDateStyle, > DateTimeFormatStyle aTimeStyle, > const nsACString& aLocale, nsAString& aRetVal) > { > - WCHAR buffer[LOCALE_NAME_MAX_LENGTH]; > + LPWSTR localeName; No, the buffer still needs to be declared here, otherwise `localeName` is just an uninitialized pointer to....somewhere off in the weeds. Not a good place to write the Unicode data into! So you want something more like WCHAR localeName[LOCALE_NAME_MAX_LENGTH]; UTF8ToUnicodeBuffer(aLocale, (char16_t*)localeName); so that there's actually some storage backing `localeName`. (I'm not entirely sure if the cast is needed there; the compiler can tell you.)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
> (I'm not entirely sure if the cast is needed there; the compiler can tell you.)
the compiler has spoken - it fancies the cast very much.
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8919052 [details] Bug 1409185 - Generalize language-matching for date/time patterns in OSPreferences. https://reviewboard.mozilla.org/r/189966/#review197200 ::: intl/locale/LocaleService.cpp:256 (Diff revision 4) > + > + Two blank lines are excessive, please drop one of them.
Attachment #8919052 -
Flags: review?(jfkthame) → review+
Comment hidden (mozreview-request) |
Comment 11•7 years ago
|
||
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/016ca1f0bbbc Generalize language-matching for date/time patterns in OSPreferences. r=jfkthame
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/016ca1f0bbbc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•