1898512 - Icon / buttons in sidebar launcher are wonky / duplicated with sidebar.revamp=true

Status: VERIFIED FIXED

(Firefox :: Sidebar, defect, P1)

Description

[:Gijs (he/him)]

Attachment: Screenshot

Fairly sure this broke in the last 24-48h.

There are also errors in the browser console:

Uncaught TypeError: can't access property "assignedNodes", this.slotEl is null
    checkForLabelText chrome://global/content/elements/moz-button.mjs:89
    handleEvent chrome://global/content/vendor/lit.all.mjs:2031
2 moz-button.mjs:89:5
    checkForLabelText chrome://global/content/elements/moz-button.mjs:89
    handleEvent chrome://global/content/vendor/lit.all.mjs:2031

(this may be a reusable components change that broke this...)

Comment 1

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

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

:hjones, since you are the author of the regressor, bug 1898076, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Hanna Jones [:hjones]]

the tl;dr here is this will be fixed when the patch for Bug 1892429 lands, but just to dig into it a bit...

That console error was definitely an oversight in the patch for the linked bug (I'll put up a fix ASAP), but it's not actually responsible for this display issue. We added this line to moz-button's connectedCallback to avoid having to add data-l10n-attrs on all instances:

this.dataset.l10nAttrs = "label";

but it turns out the FTL IDs being used for the sidebar at the moment already have label attrs because they are probably already being used on XUL elements. The combination of the text showing unexpectedly while the buttons are still using a background-image is causing the wonkiness.

In any case, this should all be fixed when the in-review patch for Bug 1892429 lands so I don't know if we need/want to take action on this issue directly.

Comment 3

[Hanna Jones [:hjones]]

Attachment: Bug 1898512 - revert and adjust some recent changes to moz-button to fix sidebar display issues r=mstriemer

Comment 4

[Hanna Jones [:hjones]]

I said we might not want to action this directly, but the fix was straightforward enough that I put up a patch. I'll file a follow up bug to re-implement that moz-button enhancement after Bug 1892429 lands

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ecf57ff373e
revert and adjust some recent changes to moz-button to fix sidebar display issues r=mstriemer,reusable-components-reviewers

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/6ecf57ff373e

Comment 7

[Sarah Clements [:sclements]]

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

I said we might not want to action this directly, but the fix was straightforward enough that I put up a patch. I'll file a follow up bug to re-implement that moz-button enhancement after Bug 1892429 lands

Thanks Hanna!

Comment 8

[Bogdan Maris, Desktop Test Engineering]

Reproduced the issue using an old/affected Nightly build from 2024-05-23, verified that using latest Nightly build 129.0a1 and latest Beta 128.0b9 the icons are displayed correctly for the new revamped sidebar (sidebar.revamp to true) across platforms MacOS 13, Ubuntu 22.04 and Windows 11.