There's a shrinking number of changes that we need to add to LocaleService before it can be considered stable. I'd like to use this tracking bug to track those and once they're all resolved, I'd like to clean up docs and tests.
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
- look at what functions could can take const.
- should switch to artifical locales in tests to ensure that real test env. locales don't create false positives.
:jfkthame brought up a point in bug 1346616 that we should return `undefined` for missing value in singular calls like getRequestedLocale, getAppLocaleAsLangTag, getAppLocaleAsBCP47. While my hope is that we'll move away from the singular forms overall, it would be good to make sure that we don't treat the edge case scenarios (when request locale is not set) the same way as if it's set to an empty string. I'll try to do this in this bug, add tests and also test extensively if any use case of the API breaks.
(also, probably the same should happen with OSIPreferences.systemLocale).
As :qDot pointed out in bug 1348042 comment 20, we may want to look into splitting LocaleService into Client/Server files and classes that inherit from a base class. I'd prefer to do that as part of this bug so that it's not combined with functional changes.
While you're splitting out functionality in LocaleService, could you also throw main thread checks in on GetInstance and all public service functions? There's a ton of code in there that will crash anyways if anything happens off the main thread (preferences, clearonshutdown, etc), but it's just nice to fail early if we can.
Jonathan, what's your take on splitting LocaleService into LocaleServiceBase, LocaleServiceClient and LocaleServiceParent?
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #7) > Jonathan, what's your take on splitting LocaleService into > LocaleServiceBase, LocaleServiceClient and LocaleServiceParent? I don't feel strongly about it, but I do think it'd probably be good for long-term readability and maintenance.
Summary: Finalize LanguageService → Finalize LocaleService
You need to log in before you can comment on or make changes to this bug.