1869827 - Links in the "Language and Appearance" sub-section of about:preferences don't work as expected

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Hanna Jones [:hjones]]

Attachment: unexpected link behaviour

The "Manage colors" and "Extensions & Themes" links in the "Language and Appearance" section of about:preferences#general either no longer work or link to the wrong place. The "Manage colors" link doesn't link to anything, and the "Extensions & Themes" link goes to about:addons, but doesn't link directly to the "Themes" section. It also always opens a new tab when previously it would just switch the tab if about:addons was already open.

Digging into this a bit, it looks like these listeners are getting removed when Fluent runs, which would explain the unexpected behaviour and make sense in the context of the regressing bug. Before that change, we were waiting for Fluent to run on the relevant fragment before adding the event listeners. The issue can be fixed by changing the code to something like this:

// make the AppearanceChooser init async and wait for pending Fluent mutations before adding listeners
async init() {
  ...
  if (document.hasPendingL10nMutations) {
    await new Promise(r =>
      document.addEventListener("L10nMutationsFinished", r, { once: true })
    );
  }
  // Add event listeners here
  ...
}

It looks like adding event listeners to DOM elements nested in translations is a known limitation of Fluent, so I'm not sure if there's a different way we might want to address this (like by moving the listeners off of the nested elements).

Comment 1

[:Gijs (he/him)]

(In reply to Hanna Jones [:hjones] from comment #0)

Nice find and thanks for filing!

It looks like adding event listeners to DOM elements nested in translations is a known limitation of Fluent, so I'm not sure if there's a different way we might want to address this (like by moving the listeners off of the nested elements).

Yes, that last thing. Otherwise, if the browser language changes at runtime, the listeners also stop working. In principle, the calling code should not need to care about the timing of the l10n bits, and accept that it cannot attach listeners directly to these elements - and/or we should fix this as a bug in fluent-DOM (ie make it keep the existing DOM references instead of replacing them, which is why the listeners are going away). I'm fairly sure it's on file but I cannot find it right now. :-(

Comment 2

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

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

Comment 3

[Hanna Jones [:hjones]]

Attachment: Bug 1869827 - Fix click listeners for Language and Appearance links in about:preferences r=#preferences-reviewers

This patch moves the event click event listeners off of the nested DOM elements to a common ancestor.

I was looking around but didn't see related tests - happy to add coverage here or as part of a follow up bug.

Comment 4

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

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c4182d7a809
Fix click listeners for Language and Appearance links in about:preferences r=settings-reviewers,Gijs

Comment 6

[Natalia Csoregi [:nataliaCs]]

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

Comment 7

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

The patch landed in nightly and beta is affected.
:hjones, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Comment 8

[:Gijs (he/him)]

Attachment: Bug 1869827 - Fix click listeners for Language and Appearance links in about:preferences r=#preferences-reviewers

This patch moves the event click event listeners off of the nested DOM elements to a common ancestor.

I was looking around but didn't see related tests - happy to add coverage here or as part of a follow up bug.

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

Comment 9

[Phabricator Automation]

Uplift Approval Request

• Code covered by automated testing: no
• Risk associated with taking this patch: Low
• Needs manual QE test: yes
• String changes made/needed: No
• User impact if declined: Broken links in the Firefox settings
• Fix verified in Nightly: no
• Explanation of risk level: Very small patch that can only affect the settings page, JS only,
• Steps to reproduce for manual QE testing: See comment 0
• Is Android affected?: no

Comment 10

[Bogdan Maris, Desktop Test Engineering]

Reproduced the initial issue described in comment 0 using an old affected Nightly build from (2023-12-12), verified that using the latest Nightly build from today (2023-12-28), both "Manage colors" and "Extensions & Themes" links point to the correct place, manage colors menu and about:addons (Themes section).

Comment 11

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/4031612cf0c8

Comment 12

[Bogdan Maris, Desktop Test Engineering]

Also verified as fixed using latest Beta build 122.0b6 across platforms (Windows 11, macOS 13 and Ubuntu 22.04).