Open Bug 1615680 Opened 6 years ago Updated 2 years ago

i18n API not returning the correct value after OS language changes

Categories

(WebExtensions :: General, enhancement, P3)

Product:
WebExtensions
Component:
General
Type:
enhancement

Tracking

(Not tracked)

People

(Reporter: amejia, Unassigned)

References

Details

The UBlock extension relies on i18n API for translations, on AC we are updating GeckoView every time the language changes using runtime.settings.locales, but if we change the language GeckoView still returns the old value to the extension until the app gets killed and re-open again. The same behaviour can be seen in the Geckoview sample app.

Rob, Luca, do you have an idea where this might be going wrong?

Flags: needinfo?(rob)
Flags: needinfo?(lgreco)

(In reply to :Agi | ⏰ PST | he/him from comment #1)

Rob, Luca, do you have an idea where this might be going wrong?

I haven't tried yet to reproduce the issue locally but, based on an initial quick look on searchfox, my guess is that it could be related to the AC code linked in comment 0 which isn't calling runtime.settings.setLocales(...) as the unit test LocaleTest.kt seems to suggest is the way to update the locale at runtime and dispatch the new preferred locales to the GeckoRuntime currently running.

If I'm not reading it wrong, locales does set the requested locales:

but it doesn't dispatch the requested locales choice to Gecko, as setLocales seems to be doing:

setLocales (or, better, commitLocales) should be dispatching a "GeckoView:SetLocale" event which should make the new locales to be reflected at a Gecko toolkit level, and at that point the i18n API should already become aware that the locale has changed.

Flags: needinfo?(lgreco)

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #2)

(In reply to :Agi | ⏰ PST | he/him from comment #1)

Rob, Luca, do you have an idea where this might be going wrong?

I haven't tried yet to reproduce the issue locally but, based on an initial quick look on searchfox, my guess is that it could be related to the AC code linked in comment 0 which isn't calling runtime.settings.setLocales(...) as the unit test LocaleTest.kt seems to suggest is the way to update the locale at runtime and dispatch the new preferred locales to the GeckoRuntime currently running.

If I'm not reading it wrong, locales does set the requested locales:

but it doesn't dispatch the requested locales choice to Gecko, as setLocales seems to be doing:

setLocales (or, better, commitLocales) should be dispatching a "GeckoView:SetLocale" event which should make the new locales to be reflected at a Gecko toolkit level, and at that point the i18n API should already become aware that the locale has changed.

Just to confirm, on AC code linked in comment 0 we are calling setLocales, runtime.settings.locales is just Kotlin syntactic sugar, but it is calling setLocales.

Flags: needinfo?(lgreco)

(In reply to Arturo Mejia from comment #3)

Just to confirm, on AC code linked in comment 0 we are calling setLocales, runtime.settings.locales is just Kotlin syntactic sugar, but it is calling setLocales.

Eh, then it looks that I was reading it wrong :-)
I was kind of suspecting that could have been the kotlin syntax to call the setter, but then I saw the test case LocaleTest.kt also written in kotlin and calling setLocales explicitly and that put me off track.

I just reproduce the issue locally and I verified that Gecko is actually receiving the "GeckoView:SetLocale" event as expected and with the languages strings in the expected order.

I'm going to add my needinfo back to look more deeply into it.

Flags: needinfo?(lgreco)
Flags: needinfo?(lgreco)

No worries, thanks for all the support :)

Luca is already looking into this, so I'll remove ni? from me. I have debugged localization issues in the past, so if you think that I can help, feel free to ni me again.

Flags: needinfo?(rob)

Ok, I took a deeper look and the issue is definitely related to a limitation in how we handle (and cache) the selected locales internally.

At the moment, besides reloading the entire application, another way to make the extension to immediately switch to the newly selected locale is to reload the extension (e.g. the extension can even reload itself by calling browser.runtime.reload(), once the extension has been reloaded the localized messages returned by the i18n API are the expected ones, and re-opening the popup reflects that).

Follows some additional detail related about the reasons behind this issue:

In particular, even if browser.i18n.getUILanguage() (as well as browser.i18n.getMessage("@@ui_language")) always returns the locale currently selected on Gecko as expected (and Gecko has received the newly requested locale to set from Fenix as I verified and mentioned in comment 4), the localized messages are being cached the first time they have been used (in particular they are going to be cached the first time the LocaleData.prototype.availableLocales lazy getter is accessed).

Changing the LocaleData's selectedLocale property (e.g. by updating its value on the extension instance, extension.localeData.selectedLocale) would not be enough, e.g. because changing it would not make LocaleData.prototype.availableLocales lazy getter to reflect that change.

Also, changing the locale at an arbitrary time, while the extension is running, may potentially lead to some undefined behavior (e.g. an extension page may already be rendered in a previously selected locale and new localized messages may be in the new selected one, resulting in a mix of languages), and the i18n doesn't provide any API event to make the extension aware that the locale has been changed at runtime.

I'm also moving this issue into the WebExtensions::General bugzilla component (as it is technically a general issue of the WebExtensions internals) and changing the type of this issue from defect to enhancement (becasue this is currently the expected behavior and we may need to also enhance the API itself, e.g. by introducing an API event to let the extension know when the selected language has been changed, to fully cover this scenario).

Type: defect → enhancement
Component: Extensions → General
Flags: needinfo?(lgreco)
Product: GeckoView → WebExtensions

From a product perspective we will not be prioritizing this bug at the moment. We recommend that the GeckoView/Fenix team reloads add-ons when the language is changed as a workaround. Thank you everyone for investigating!

Priority: -- → P3

(In reply to Philipp Kewisch [:Fallen] [:📆][:🧩] from comment #8)

From a product perspective we will not be prioritizing this bug at the moment. We recommend that the GeckoView/Fenix team reloads add-ons when the language is changed as a workaround. Thank you everyone for investigating!

From AC/Fenix we don't have APIs for reloading the add-ons, the only think that we can do is to fully reload the app every time the language changes, that it is not going to be a nice user experience :(

(In reply to Arturo Mejia from comment #9)

(In reply to Philipp Kewisch [:Fallen] [:📆][:🧩] from comment #8)

From a product perspective we will not be prioritizing this bug at the moment. We recommend that the GeckoView/Fenix team reloads add-ons when the language is changed as a workaround. Thank you everyone for investigating!

From AC/Fenix we don't have APIs for reloading the add-ons, the only think that we can do is to fully reload the app every time the language changes, that it is not going to be a nice user experience :(

Technically reloading an add-ons doesn't differ too much from disabling and re-enabling it, and so that could be a workaround for that missing GeckoView API (but I think that adding a reload method to the GeckoView API should be simple enough, and so we may opt to just implement it if we don't have actual reasons for not introducing that method at all), or it could be the extension itself to detect that the UI language has been changed and reload itself (or ask the user first) by calling browser.runtime.reload().

I was also wondering: how common is for android users to switch the language at runtime more than once?
(I'm pretty sure that I did it for the first time on my phone to investigate this issue :-P)

If it isn't that common, it may be reasonable to don't workaround this limitation at all for now (especially if the workaround is "reloading the entire app", which I totally agree with Arturo that it would be pretty bad from a user experience perspective).

I see this problem on uBlock (switched language to Italian -> uBlock UI is still in English). And enabling/disabling does not work. Is this expected? see also https://github.com/mozilla-mobile/fenix/issues/12530

Note that installing uBlock on a clean profile works as expected (Italian localization).

Flags: needinfo?(lgreco)

(In reply to Agi Sferro | :agi | ⏰ PST | he/him from comment #11)

I see this problem on uBlock (switched language to Italian -> uBlock UI is still in English). And enabling/disabling does not work. Is this expected? see also https://github.com/mozilla-mobile/fenix/issues/12530

Note that installing uBlock on a clean profile works as expected (Italian localization).

Hey Agi, I gave this another look today (by connecting to both the Fenix main process and the ublock extension using the remote debugging) and it seems that something has changed from what I described in my comment 7, in particular:

it looks that both browser.i18n.getUILanguage() / browser.i18n.getMessage("@@ui_locale) from the extension side and Services.locale.appLocaleAsBCP47 on the chrome privileged js code side are still returning "en-US" after I set the italian language as the primary android language, and that would be preventing the extension from switching to the italian locale even after it has been restarted (or enabled/disabled).

I'm not sure yet what did actually change in the meantime (e.g. I'm wondering if the issue may have been made worst because of some changes we may have introduces on the Services.locale side).

do you know if it is possible for QA to easily bisect a regression on Fenix (or on the GeckoViewExample)?

Otherwise I may try to dig deeper and see if I can figure out exactly in which layer this more recent issue has been introduced (or look if I can bisect it locally using hg bisect).

Flags: needinfo?(lgreco) → needinfo?(agi)

FYI you can bisect with mozregression -n gve. I also see this on web content too so it's probably not extension specific.

Flags: needinfo?(agi)

Do we need another issue for tracking the bug? I noticed the issue is not only related to add-ons, it looks like the web content is not updating neither. As the user described in on the fenix issue the search box text in about:config is not getting translated, I was able to confirm the pref intl.accept_languages was getting updated in both beta GV:79.0-20200717001501 and nightly GV: 80.0a1-20200727095125. But in beta the search box text about:config is getting translated but not in nightly.

Flags: needinfo?(lgreco)
Flags: needinfo?(agi)

(In reply to Arturo Mejia from comment #14)

Do we need another issue for tracking the bug?

yeah I think that it would be good to file a separate issue and needinfo someone that works on those internals.

Based on what I saw (as I did mention in comment 12) I was already suspecting that there is now a separate non-extension specific issue that has been introduced in the meantime.

Agi has also confirmed in comment 13 that it was seeing this on web content too, but I don't know if he has already filed another issue and/or tried to bisect it.

Flags: needinfo?(lgreco)

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #15)

(In reply to Arturo Mejia from comment #14)

Do we need another issue for tracking the bug?

yeah I think that it would be good to file a separate issue and needinfo someone that works on those internals.

Based on what I saw (as I did mention in comment 12) I was already suspecting that there is now a separate non-extension specific issue that has been introduced in the meantime.

Agi has also confirmed in comment 13 that it was seeing this on web content too, but I don't know if he has already filed another issue and/or tried to bisect it.

Luca I filed #1656515, I cc both of you if you could add needinfo for the right person It would be pretty helpful thanks in advance!

(In reply to Arturo Mejia from comment #16)

Luca I filed #1656515, I cc both of you if you could add needinfo for the right person It would be pretty helpful thanks in advance!

Thanks Arturo, I just needinfo-ed zibi on Bug 1656515, another idea to get to a regression range would be to try to run mozregression -n gve as Agi did mention in comment 13 and see if we can bisect it.

Flags: needinfo?(agi)
Severity: normal → S3
See Also: → 1812835
You need to log in before you can comment on or make changes to this bug.