1911342 - FullPageTranslationsPanel Translate button no longer correctly reacts to language changes

Status: VERIFIED FIXED

(Firefox :: Translations, defect)

Description

[Erik Nordin [:nordzilla]]

Description

Bug 1891792 landed some code that changed the way events are handled in the FullPageTranslationsPanel.

Unfortunately, our automated test coverage was missing a specific case that would fail if the Translate button did not correctly become re-enabled when selecting a valid language pair if an invalid language pair was previously selected.

---

Steps to reproduce #1

Note: The following STR should be tested both by selecting the new language value using the mouse, and by selecting the new language value by focusing the menulist with TAB, then using the arrow keys.

• Open a translatable web page, e.g. https://es.wikipedia.org/wiki/Mozilla_Firefox
• Open the Full-Page Translations Panel.
• Verify that valid languages are selected and that the Translate button is enabled.
• Change the selected from-menu-list value to the "Choose a language" option.

Expected behavior
The translate button becomes disabled.

Actual behavior
The translate button does not change.

• Change the selected from-menu-list value back to a language that is not the same as the selected to-menu-list value.

Expected behavior
The translate button becomes re-enabled.

Actual behavior
The translate button does not change.

• Change the selected from-menu-list value back to the same language as the selected to-menu-list value.

Expected behavior
The translate button becomes disabled.

Actual behavior
The translate button does not change.

Steps to reproduce #2

Note: The following STR should be tested both by selecting the new language value using the mouse, and by selecting the new language value by focusing the menulist with TAB, then using the arrow keys.

Repeat the process of STR #1, but change the to-menu-list values instead of changing the from-menu-list values.

---

Tests to implement

• We should add new test cases that ensure this behavior has coverage.
• These tests should ensure that the telemetry is correct as well.

Comment 1

[Erik Nordin [:nordzilla]]

Attachment: Bug 1911342 - Update FPT menulist test infrastructure r=#translations-reviewers!

Updates the way that new languages are selected in the
Full-Page Translations tests to open the menu list and
select the item from the dropdown.

Comment 2

[Erik Nordin [:nordzilla]]

Attachment: Bug 1911342 - Correctly handle FPT menulist event handlers r=#translations-reviewers!

Ensures that the new JS event handlers correctly handle
language-change events in the FullPageTranslationsPanel.

Adds more test cases to the Full-Page Translations
panel test suite that change the selected languages
and trigger new translations from the selections.

Depends on D218422

Comment 3

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

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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a11b47e1a629
Update FPT menulist test infrastructure r=translations-reviewers,gregtatum
https://hg.mozilla.org/integration/autoland/rev/b30da5f6bc1c
Correctly handle FPT menulist event handlers r=translations-reviewers,gregtatum

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/a11b47e1a629
https://hg.mozilla.org/mozilla-central/rev/b30da5f6bc1c

Comment 6

[Erik Nordin [:nordzilla]]

Attachment: Bug 1911342 - Update FPT menulist test infrastructure

Updates the way that new languages are selected in the
Full-Page Translations tests to open the menu list and
select the item from the dropdown.

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

Comment 7

[Erik Nordin [:nordzilla]]

Attachment: Bug 1911342 - Correctly handle FPT menulist event handlers

Ensures that the new JS event handlers correctly handle
language-change events in the FullPageTranslationsPanel.

Adds more test cases to the Full-Page Translations
panel test suite that change the selected languages
and trigger new translations from the selections.

Original Revision: https://phabricator.services.mozilla.com/D218423
Depends on D218422

Depends on D219491

Comment 8

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: The FullPageTranslationsPanel's translate button will not update its enabled-state based on the selected languages changing.
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: STR are in the bug description.
• Risk associated with taking this patch: Low
• Explanation of risk level: These are straightforward changes that affect only the UI code that regressed, and are now tested in automation.
• String changes made/needed: None
• Is Android affected?: no

Comment 9

[Erik Nordin [:nordzilla]]

This is a bit of an odd situation because the regressing bug (Bug 1891792) has three patches in its stack, two of which were landed in 130 with leave-open.

The third patch landed in 131, which closed the bug.

However, something within the first two patches caused a regression, which is fixed in this bug (Bug 1911342), which I've just requested beta uplift for.

However, since the regressing bug was closed in 131, it looks like the regression and the fix both happened in 131, but the reality is that the regression is in 130 and the fix is in 131.

So I'm marking 130 as affected, even though it looks as though the regressor only landed in 131.

Please let me know if I should update the flags on the regressor too.

Comment 10

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/b595ca08f0fd
https://hg.mozilla.org/releases/mozilla-beta/rev/dea6f75d2890

Comment 11

[Peter Magyari (Desktop QA)]

Verified fixed on Windows 10, MacOS 14 and Ubuntu 24.04 using Firefox Beta 130.0b8 (20240821091813) and Firefox Nightly 131.0a1 (20240821160637)