1674091 - Bookmarks toolbar height shrinks when empty, and is shown on new tab even when empty

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[Chris Morgan]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:84.0) Gecko/20100101 Firefox/84.0

Steps to reproduce:

1. Take an empty profile.
2. Have browser.toolbars.bookmarks.visibility set to its recent new default of “newtab”.
3. Go to a new tab.
4. Delete the “Get Involved” bookmark that’s in the Bookmarks Toolbar, so that the Bookmarks Toolbar folder is empty.

Actual results:

The bookmarks toolbar remains, but starts taking up less vertical space.

Expected results:

The bookmarks toolbar should either retain its original height, or disappear.

I’m inclined to think that disappearing is better, at least in the case of newtab. If it’s always, I dunno. (As I’ve alluded to in bug 1674090, I feel that newtab makes this toolbar less a toolbar and more a part of about:newtab.)

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1674091 - Add a getter for the Bookmarks Toolbar in BookmarkingUI.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1674091 - Show and hide the empty bookmarks message if there are visible children in the toolbar.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1674091 - Don't update the state of the Other Bookmarks shortcut folder if the personal-bookmarks item has been placed in the palette.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2ed373e6fc3
Add a getter for the Bookmarks Toolbar in BookmarkingUI. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/4d49ba20a908
Show and hide the empty bookmarks message if there are visible children in the toolbar. r=Gijs,fluent-reviewers
https://hg.mozilla.org/integration/autoland/rev/b37a366b6bca
Don't update the state of the Other Bookmarks shortcut folder if the personal-bookmarks item has been placed in the palette. r=Gijs

Comment 5

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=b37a366b6bca1d54f3cc150e42d811c7d446e998&searchStr=bc&selectedTaskRun=OqowoRtSSgOg6wmJzUvW6A.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=321636886&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/4ba5594ebb80a612656b1a8714426a245aae0c06

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4db429711269
Add a getter for the Bookmarks Toolbar in BookmarkingUI. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/437053f5228a
Don't update the state of the Other Bookmarks shortcut folder if the personal-bookmarks item has been placed in the palette. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/5c55402f9d41
Show and hide the empty bookmarks message if there are visible children in the toolbar. r=Gijs,fluent-reviewers

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4db429711269
https://hg.mozilla.org/mozilla-central/rev/437053f5228a
https://hg.mozilla.org/mozilla-central/rev/5c55402f9d41

Comment 8

[Sören Hentzschel]

The bug title contains "Bookmarks toolbar height shrinks when empty" and this is still true (at least on macOS, I can't test on other platforms). Is this expected?

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Sören Hentzschel from comment #8)

The bug title contains "Bookmarks toolbar height shrinks when empty" and this is still true (at least on macOS, I can't test on other platforms). Is this expected?

We tried to limit this on Windows but should take a second look on macOS. Can you file a new bug as this one is now closed?

Comment 10

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)

(In reply to Sören Hentzschel from comment #8)

The bug title contains "Bookmarks toolbar height shrinks when empty" and this is still true (at least on macOS, I can't test on other platforms). Is this expected?

We tried to limit this on Windows but should take a second look on macOS. Can you file a new bug as this one is now closed?

We already have a mac bug for this, as it was caught by our test infrastructure - bug 1678666.