1889697 - Translations don't work across private/non-private windows

Status: VERIFIED FIXED

(Firefox :: Translations, defect)

Description

[Marco Castelluccio [:marco]]

After the first time I translate in a non-private window, translations don't work anymore in private windows.
Same happens in the other direction.

Comment 1

[Marco Castelluccio [:marco]]

Mozregression says:
96:48.62 INFO: Last good revision: ad3e0a193f6695c9133975ca3d272236bf4fa59f
96:48.62 INFO: First bad revision: c21d8e0ee576012d309a4cbd267e9e54b660e605
96:48.62 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ad3e0a193f6695c9133975ca3d272236bf4fa59f&tochange=c21d8e0ee576012d309a4cbd267e9e54b660e605

It could be either bug 1870327 or bug 1870300.

Comment 2

[Erik Nordin [:nordzilla]]

I dug into this and found that the regression is here:

Bug 1870327
https://phabricator.services.mozilla.com/D201282

---

This is where I refactored the way that the language lists are built to be in the shared class here.

The new code stores the initialized state based on the panel's element ID. So, when full-page-translations-panel gets its language lists initialized, that gets put into the map as "initialized".

This appears to break when someone opens up a new window after translating, not just a private window.

I looked into adding the PID to the map as a differentiator, but that's not aggressive enough: all the windows still have the same parent process.

I then looked into adding the innerWindowId as a differentiator, but that is too aggressive: even new tabs get a different innerWindowId.

It's not really clear to me at the moment why the FullPageTranslationsPanel class, which is a singleton class, seems to be losing information when the TranslationsPanelShared class, which is just a class with static members and functions, is retaining information within the same process.

I'm going to keep looking into this.

Comment 3

[Erik Nordin [:nordzilla]]

Attachment: Bug 1889697 - Consider windowId for TranslationsPanel Initialization r=#translations-reviewers!

Fixes an issue where the cached state for the Translations
Panel's lanuage list initializtion was too aggressive across
new window instances. Now considers the inner window id
when caching the state.

Comment 4

[Erik Nordin [:nordzilla]]

This is slightly wrong. I think in my haste to debug this I somehow logged the innerWindowId incorrectly.

The innerWindowId does seem to be what we want. I'm uploading a patch.

Comment 5

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

Set release status flags based on info from the regressing bug 1870327

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b9e06dfd0cd
Consider windowId for TranslationsPanel Initialization r=translations-reviewers,gregtatum

Comment 7

[Erik Nordin [:nordzilla]]

Attachment: Bug 1889697 - Consider windowId for TranslationsPanel Initialization r=#translations-reviewers!

Fixes an issue where the cached state for the Translations
Panel's lanuage list initializtion was too aggressive across
new window instances. Now considers the inner window id
when caching the state.

Original Revision: https://phabricator.services.mozilla.com/D206703

Comment 8

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: The Full-Page Translations panel will fail to populate the language lists if users attempt to translate with two Firefox windows open at the same time.
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: Open more than one Firefox window at the same time and try to invoke Full-Page Translations in both of them
• Risk associated with taking this patch: Low risk
• Explanation of risk level: This is a few-line change that concatenates an extra identifier onto a string key to a map.
• String changes made/needed: None
• Is Android affected?: no

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1b9e06dfd0cd

Comment 10

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/5d44351380f3

Comment 11

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Reproduced the issue with Firefox 126.0a1 (2024-04-04) on Windows 10x64. After opening a new normal or private window the translation panel will be empty for the Translate from and Translate to dropdowns.
The issue is verified fixed with Firefox 125.0RC1 and 126.0a1 (2024-04-08) on Windows 10x64, macOS 13 and Ubuntu 23.10. The Translate from and Translate to dropdowns are successfully populated with languages after following steps from comment 8. Note that bug 1890494 was encountered during testing.