1346877 - Finalize LocaleService

Status: ASSIGNED

(Core :: Internationalization, enhancement, P3)

Description

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

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.

Comment 1

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

 - look at what functions could can take const.

Comment 2

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

 - should switch to artifical locales in tests to ensure that real test env. locales don't create false positives.

Comment 3

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

: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.

Comment 4

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

(also, probably the same should happen with OSIPreferences.systemLocale).

Comment 5

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

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.

Comment 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

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.

Comment 7

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

Jonathan, what's your take on splitting LocaleService into LocaleServiceBase, LocaleServiceClient and LocaleServiceParent?

Comment 8

[Jonathan Kew [:jfkthame]]

(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.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 10

[Suhaib Mujahid [:suhaib]]

Sorry, there was a problem with the detection of inactive users. I'm reverting the change.