Closed Bug 1409158 Opened 7 years ago Closed 7 years ago

Retrieve the correct OSPreferences::GetRegonalPrefsLocales on Unix using LC_TIME

Categories

(Core :: Internationalization, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

Attachments

(1 file)

As per analysis in bug 1337069, the low hanging fruit here is to retrieve the LC_TIME as the operating system regionalprefslocale.
Assignee: nobody → gandalf
Blocks: 1337069
Status: NEW → ASSIGNED
Comment on attachment 8919051 [details] Bug 1409158 - Use LC_TIME to retrieve OSPreferences::GetRegionalPrefsLocales on Unix. https://reviewboard.mozilla.org/r/189942/#review195108 C/C++ static analysis found 1 defect in this patch. You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp` If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: intl/locale/gtk/OSPreferences_gtk.cpp:35 (Diff revision 1) > { > - // For now we're just taking System Locales since we don't know of any better > - // API for regional prefs. > - return ReadSystemLocales(aLocaleList); > + MOZ_ASSERT(aLocaleList.IsEmpty()); > + > + // For now we're just taking the LC_TIME from POSIX environment for all > + // regional preferences. > + const char* posixID = setlocale(LC_TIME, NULL); Warning: Use nullptr [clang-tidy: modernize-use-nullptr] const char* posixID = setlocale(LC_TIME, NULL); ^ nullptr
Priority: -- → P3
Comment on attachment 8919051 [details] Bug 1409158 - Use LC_TIME to retrieve OSPreferences::GetRegionalPrefsLocales on Unix. https://reviewboard.mozilla.org/r/189942/#review195852 Seems reasonable enough. I'm not sure what we should do if LC_NUMERIC, for example, were set to something different - but let's not worry about that for now. ::: intl/locale/gtk/OSPreferences_gtk.cpp:35 (Diff revision 2) > + const char* posixID = setlocale(LC_TIME, nullptr); > + nsAutoCString localeStr; > + localeStr.Assign(posixID); It'd be more concise to just write this as nsAutoCString localeStr(setlocale(LC_TIME, nullptr)); and should be directly equivalent.
Attachment #8919051 - Flags: review?(jfkthame) → review+
> I'm not sure what we should do if LC_NUMERIC, for example, were set to something different - but let's not worry about that for now. I'd like to get to the point where we'll allow you to do `GetRegionalPreferences(LocaleService::Type::Numeric)` or sth like that, but that's a bit longer term. :)
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4577afc2a13d Use LC_TIME to retrieve OSPreferences::GetRegionalPrefsLocales on Unix. r=jfkthame
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Depends on: 1410266
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: