1819980 - 110.0.1 misplaced icons in toolbars with no "Bookmark toolbar items"

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[08xjcec48]

Steps to reproduce:

Users who had removed the Bookmark toolbar items element from their toolbars are reporting that their custom icons are misplaced after updating to Firefox 110.0.1:

https://old.reddit.com/r/firefox/comments/11evixz
https://old.reddit.com/r/firefox/comments/11fgnlc
https://old.reddit.com/r/firefox/comments/11g98xd

Comment 1

[jscher2000]

This appears to have been caused by changing a rule from width: 0; (which overrides the natural width of the text content of the element) to min-width: 0; for #personal-toolbar-empty-description in bug 1812636:

<hbox id="personal-toolbar-empty" skipintoolbarset="true" removable="false" nowidth="">
    <description id="personal-toolbar-empty-description" data-l10n-id="bookmarks-toolbar-empty-message" 
        onclick="BookmarkingUI.openLibraryIfLinkClicked(event);" 
        onkeydown="BookmarkingUI.openLibraryIfLinkClicked(event);">
            For quick access, place your bookmarks here on the bookmarks toolbar. 
            <html:a xmlns:html="http://www.w3.org/1999/xhtml" data-l10n-name="manage-bookmarks" 
            class="text-link" tabindex="0">Manage bookmarks…</html:a>
    </description>
</hbox>

https://searchfox.org/mozilla-release/source/browser/themes/shared/browser-shared.css#222 et seq.

#PersonalToolbar[collapsed=false] > #personal-toolbar-empty[nowidth] > #personal-toolbar-empty-description {
  margin-inline: 0;
  min-width: 0; /*** previously this was width: 0; ***/
  white-space: nowrap;
  position: relative;
  z-index: 1;
}

Comment 2

[Itiel]

This can be seen also during Firefox's startup, even if there are bookmarks there.

Comment 3

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

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

For more information, please visit auto_nag documentation.

Comment 4

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

The bug has a release status flag that shows some version of Firefox is affected, thus it will be considered confirmed.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I'm confused, I can't repro this on a clean profile? What are the conditions on which this happens?

Comment 6

[:Gijs (he/him)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

I'm confused, I can't repro this on a clean profile? What are the conditions on which this happens?

STR that work for me:

1. new profile
2. open customize mode
3. remove import button + bookmarks toolbar items from the toolbar
4. put any other toolbar button there by dragging it from the palette
5. exit customize mode
6. open new window (or restart Firefox, presumably)

Does that help?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Ah yeah, I guess I missed (6). Thanks!

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1819980 - Tweak empty bookmarks toolbar message visibility logic. r=Gijs

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cee1f3bbec04
Tweak empty bookmarks toolbar message visibility logic. r=Gijs

Comment 10

[Emilio Cobos Álvarez (:emilio)]

It's probably worth backing out the offending patch from beta/next dot release.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

ni? me to add a test.

Comment 12

[jscher2000]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #10)

It's probably worth backing out the offending patch from beta/next dot release.

I only have the end user perspective, but wouldn't it be simpler to add back this width rule that was changed to a min-width rule?

#PersonalToolbar[collapsed=false] > #personal-toolbar-empty[nowidth] > #personal-toolbar-empty-description {
  width: 0; 
}

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Well that's effectively what backing out would do right?

Comment 14

[jscher2000]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #13)

Well that's effectively what backing out would do right?

You're right. I thought there were other changes, but it's just a comment.

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/cee1f3bbec04

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1819980 - Add test for this issue. r=Gijs

Comment 17

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

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 18

[Donal Meehan [:dmeehan]]

Setting 111 to Fixed, Bug 1812636 was backed out of beta

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0bc587101fe
Add test for this issue. r=Gijs

Comment 20

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/a0bc587101fe

Comment 21

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our latest Nightly build 112.0a1 (2023-03-12) as well as our latest RC 111.0 .