LocaleService has an event that is fired when locales change. Especially since JS Context is initialized very early, we do end up with it having the default locale not reflecting current UI locale. In order to fix that, we should make JS context listen to `intl:app-locales-changed` event and when it happens just do: ``` nsAutoCString appLocaleStr; LocaleService::GetInstance()->GetAppLocaleAsBCP47(appLocaleStr); JS_SetDefaultLocale(cx, appLocaleStr.get()); ``` This is needed for content processes to use correct locale due to initialization order (see bug 1349106 for content side, and bug 1344355 for parent process side).

The locale is currently set in http://searchfox.org/mozilla-central/source/js/xpconnect/src/XPCLocale.cpp#208 I'm not sure if XPCLocaleCallbacks is the right place to set nsObserver and listen to intl:app-locales-changed. Where should it go?

Olli, :shu said I should ask someone from #content. Where this observer should be added in order to fire it on all JS contexts?

There is just one js context, at least currently, so XPCLocale.cpp should be fine, assuming this is main thread only. Other option is somewhere http://searchfox.org/mozilla-central/rev/944f87c575e8a0bcefc1ed8efff10b34cf7a5169/xpcom/base/CycleCollectedJSContext.cpp#556 but remember that code runs in workers too, and prefs can be used in mainthread only.

More feedback from :smaug: > create a small class in XPCLocale.cpp, make it nsIObserver, listen to intl:app-locales-changed, when called, use CycleCollectedJSContext::Get()->Context() and call xpc_setDefaultLocale

bz also says that in order to get it to all workers we need `BROADCAST_ALL_WORKERS`

And we can look at how it works for accepted_languages - http://searchfox.org/mozilla-central/rev/944f87c575e8a0bcefc1ed8efff10b34cf7a5169/dom/workers/RuntimeService.cpp#1268

With some help from :smaug on IRC I got it working. I verified that it does react to the event properly and when combined with patch from bug 1348042 it properly propagates app locale to JS contexts both in parent and content processes which fixes bug 1344355 and bug 1349106!

I left the workers out for now, since they may add some complexity and we don't need it at the moment. I'd like to first get parent/content main threads to work properly to unblock the datetime picker team and fennec work.

Comment on attachment 8858976 [details] Bug 1356066 - JS_SetDefaultLocale should be updated on intl:app-locales-changed. Adding r?shu for the impact on SpiderMonkey.

Comment on attachment 8858976 [details] Bug 1356066 - JS_SetDefaultLocale should be updated on intl:app-locales-changed. https://reviewboard.mozilla.org/r/130978/#review133824 ::: js/xpconnect/src/XPCLocale.cpp:53 (Diff revision 1) > + > + observerService->AddObserver(this, "intl:app-locales-changed", false); > +} > + > +NS_IMETHODIMP > +XPCLocaleObserver::Observe(nsISupports *aSubject, const char *aTopic, const char16_t *aData) Nit, * goes with the type. nsISupports* aSubject, const char* aTopic, const char16_t* aData ::: js/xpconnect/src/XPCLocale.cpp:84 (Diff revision 1) > localeToLowerCase = LocaleToLowerCase; > localeCompare = LocaleCompare; > localeToUnicode = LocaleToUnicode; > + > + // It's going to be retained by the ObserverService. > + XPCLocaleObserver* locObs = new XPCLocaleObserver(); This should be RefPtr<XPCLocaleObserver> locObs = new XPCLocaleObserver(); since per COM rules caller is supposed to keep params alive. Feels bad to pass refcnt==0 object to AddObserver

Comment on attachment 8858976 [details] Bug 1356066 - JS_SetDefaultLocale should be updated on intl:app-locales-changed. https://reviewboard.mozilla.org/r/130978/#review134016 ::: js/xpconnect/src/XPCLocale.cpp:57 (Diff revision 2) > +NS_IMETHODIMP > +XPCLocaleObserver::Observe(nsISupports* aSubject, const char* aTopic, const char16_t* aData) > +{ > + if (!strcmp(aTopic, "intl:app-locales-changed")) { > + JSContext* cx = CycleCollectedJSContext::Get()->Context(); > + xpc_LocalizeContext(cx); xpc_LocalizeContext is fallible, looks like on OOM. Need to handle the error case. xpc_LocalizeContext also calls JS_SetLocaleCallbacks, which requires that no GC or CC is ongoing. I think that's true in this case, but I don't know enough about when NotifyObservers is called to say if this is case. When does the "intl:app-locales-changed" event fire?

Updated to your feedback. The intl:app-locales-changed is fired here http://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#233 in response to change in one of request locales prefs: http://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#436 So basically, when the user changes preferences or new langpack is installed.

Comment on attachment 8858976 [details] Bug 1356066 - JS_SetDefaultLocale should be updated on intl:app-locales-changed. https://reviewboard.mozilla.org/r/130978/#review134024 Going off the reasonable assumption that NotifyObservers cannot be called during CC, r=me.

Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/904cbae6e235 JS_SetDefaultLocale should be updated on intl:app-locales-changed. r=shu,smaug