Closed Bug 1347002 Opened 3 years ago Closed 3 years ago

Add LocaleService::GetAvailableLocales

Categories

(Core :: Internationalization, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Assignee: nobody → gandalf
Blocks: 1346877
Status: NEW → ASSIGNED
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.
Attachment #8846902 - Flags: feedback?(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?
Attachment #8846902 - Flags: review?(jfkthame) → review+
That makes sense! AvailableLocales should *at least* have the default locale. I hope that build system will guarantee that.
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7cbd72553080
Add LocaleService::GetAvailableLocales. r=jfkthame
https://hg.mozilla.org/mozilla-central/rev/7cbd72553080
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.