1347002 - Add LocaleService::GetAvailableLocales

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

Final part of the language negotiation is to be able to retrieve the available locales.

Initially, the list of available locales will match the list of available locales in ChromeRegistry, but soon we'll be cross-checking it with available locales in L10nRegistry and maybe in the future we'll also narrow it down to locales we have Intl data for.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1347002 - Add LocaleService::GetAvailableLocales.

Review commit: https://reviewboard.mozilla.org/r/119892/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119892/

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8846902 [details]
Bug 1347002 - Add LocaleService::GetAvailableLocales.

I swear this the end is near :)

This API is simple at the moment as it basically retrieves locales for global package from ChromeRegistry.

Once I land L10nRegistry, I'll start returning only locales present in both.

I'll add caching in bug 1345527 once we finalize this API and migrate the callsites.

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8846902 [details]
Bug 1347002 - Add LocaleService::GetAvailableLocales.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119892/diff/1-2/

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 8846902 [details]
Bug 1347002 - Add LocaleService::GetAvailableLocales.

https://reviewboard.mozilla.org/r/119892/#review124856

One question re the best/safest way to handle the (should-not-occur) case where no locales are available? Might be worth adjusting that.

::: intl/locale/LocaleService.h:130
(Diff revision 2)
> +   * Returns a boolean indicating if the attempt to retrieve the
> +   * list was successful.

Just wondering about an edge case: what should happen if there's no actual error, but the enumerator just doesn't return any locales (i.e. localesEnum->HasMore returns false the first time)? Should we consider an empty result to be an error and throw something like NS_ERROR_UNAVAILABLE, rather than just returning "success" but an empty list, which callers may not expect?

::: intl/locale/LocaleService.cpp:199
(Diff revision 2)
> +      return false;
> +    }
> +
> +    aRetVal.AppendElement(localeStr);
> +  }
> +  return true;

See above, I wonder if doing

    return !aRetVal.IsEmpty();

would be good here, so that "success" always indicates at least one locale code is available in the result?

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

That makes sense! AvailableLocales should *at least* have the default locale. I hope that build system will guarantee that.

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8846902 [details]
Bug 1347002 - Add LocaleService::GetAvailableLocales.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119892/diff/2-3/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7cbd72553080
Add LocaleService::GetAvailableLocales. r=jfkthame

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7cbd72553080