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
https://hg.mozilla.org/mozilla-central/rev/4577afc2a13d
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: