1902342 - Bookmarks Toolbar briefly shows placeholder message ("For quick access, place your bookmarks here...") on every new Firefox window, sometimes alongside my first bookmark

Status: NEW

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

Description

[Daniel Holbert [:dholbert]]

Attachment: screenshot of bug (placeholder message briefly displayed in new window, alongside an actual bookmark)

STR:

1. (optional but encouraged) Start some screen recording software, to play back later in slow-motion, to increase the likelihood of seeing the very-brief issue.
2. Configure Firefox to show the bookmarks toolbar, and place some bookmarks on it. (I have 5 bookmarks on it, the first of which is Spotify, and in my case I have it set to only-show-on-new-tab.)
3. Open a new Firefox window, and watch its bookmarks-toolbar very closely.

ACTUAL RESULTS:

• The new Firefox Window's bookmarks-toolbar briefly renders with this placeholder message:

For quick access, place your bookmarks here on the bookmarks toolbar.  _Manage bookmarks..._

• In my case, this actually rendered alongside my first bookmark, which seems bizarre & like something that should never happen.

EXPECTED RESULTS:

• The placeholder message should never be displayed simultaneously with an actual bookmark.
• If we have bookmarks on the bookmarks toolbar, we should never display the placeholder message inviting the user to place bookmarks there (since they already have done that). (Even if we're stalled discovering what-the-actual-bookmarks-are, it seems like we could track the fact that they exist in some sort of lightweight quick-to-discover sort of way, and query that when deciding whether to show this message). If we display it, it's pretty disconcerting because it's a lot of text that we flash for just a moment, which makes it feel to the user like they just missed a message of some sort

Comment 1

[Daniel Holbert [:dholbert]]

I can reproduce this bug in a fresh profile, though I think you need to remove the "Import bookmarks" button from the bookmarks-toolbar (the leftmost thing on it) before this reproduces. At least, I wasn't able to see this placeholder message appearing on new windows until I removed that button.

If it matters: I'm testing with Nightly 129.0a1 (2024-06-13) (64-bit) on Linux (though I don't currently have any reason to believe that this would be platform-depenent).

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screencast where the bug reproduces in a fresh profile (extremely briefly, you might need to watch in slow-motion or drag scrubber while video is paused to see it)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: screenshot of a moment in the screencast where the bug can be seen

Here's a screenshot from my just-attached screencast where the bug can be seen, for the second new-window that I open in the screencast. (The first new-window in the screencast seems to be fine, though I suspect that's just luck / raciness.)

Comment 4

[Daniel Holbert [:dholbert]]

Looks like this placeholder message was added in bug 1674091, in part as a way to "prop up" the height of the toolbar when it's empty.

One possible hackaround here is that we could make the message be initially visibility:hidden or something, so that it takes up vertical space but doesn't paint (to avoid the perception of "wait, what was the big message that Firefox just flashed at me for a moment").

That's still not really ideal/complete, though, because that wouldn't fix the problem where an actual bookmark (e.g. "Get Involved" in my just-attached attachment 1902342) is being drawn at the completely-wrong-location (way over on the right side) for a few frames as the window appears.

Comment 5

[Marco Bonardo [:mak]]

(In reply to Daniel Holbert [:dholbert] from comment #4)

One possible hackaround here is that we could make the message be initially visibility:hidden or something, so that it takes up vertical space but doesn't paint (to avoid the perception of "wait, what was the big message that Firefox just flashed at me for a moment").

Yep, I think that'd be a reasonable approach.
To avoid the bookmark shifting we could maybe temporarily set the width of the message box to 0

I doubt we may have time to investigate this shortly, but we'd surely evaluate patches.

Comment 6

[:Gijs (he/him)]

(In reply to Daniel Holbert [:dholbert] from comment #4)

Looks like this placeholder message was added in bug 1674091, in part as a way to "prop up" the height of the toolbar when it's empty.

One possible hackaround here is that we could make the message be initially visibility:hidden or something, so that it takes up vertical space but doesn't paint (to avoid the perception of "wait, what was the big message that Firefox just flashed at me for a moment").

That's still not really ideal/complete, though, because that wouldn't fix the problem where an actual bookmark (e.g. "Get Involved" in my just-attached attachment 1902342) is being drawn at the completely-wrong-location (way over on the right side) for a few frames as the window appears.

Yeah, IIRC at some point we used width: 0 and visible overflow to help with that but that also didn't work because then the link wasn't clickable. ( bug 1812636). Maybe it'd be OK as a transient state? There's been a lot of iteration on exactly how this message works - basically the async-ness here makes it difficult. It's gotten to a point where I feel like rewriting the implementation may be easier than trying to bolt yet another thing on top (as Emilio's comment on that patch says, it's not clear why we need this much logic / CSS for what would appear to be a simple enough message).

That said, although this might be difficult to avoid for the initial window, avoiding it for the Nth window case should be easier as we could have a sync cache of whether there are bookmarks there?

Anyway, yeah, the requirements for this message ideally are:

• take up the same height as a single bookmark before deciding whether to show it, as the bookmarks toolbar is visible and shouldn't flicker or lead to web content being resized when it populates
• allow drag/drop of bookmarks (links) to the entire bookmark bar when visible
• the link should be clickable/functional
• the message should be announced to screen reader users the first time it's visible ( bug 1886807 )
• not be visible / take up width until/unless we know there aren't bookmarks
• shouldn't synchronously load / block on bookmarks being read (esp. for the first window / startup)

hopefully that's it and I've not forgotten anything...

Comment 7

[Marco Bonardo [:mak]]

(In reply to :Gijs (he/him) from comment #6)

It's gotten to a point where I feel like rewriting the implementation may be easier than trying to bolt yet another thing on top

It should probably be implemented as a toolbar placeholder message, and managed by the toolbar widget itself. Though it seems hard to justify time (and risk) spent on another rewrite of it.
The code has already been simplified a bit by Emilio and me.

That said, although this might be difficult to avoid for the initial window, avoiding it for the Nth window case should be easier

Sure, we could pass the information when opening new windows, if the current window has already completed toolbar init.
And use that info somewhere around here, I guess https://searchfox.org/mozilla-central/rev/d31f750ad9e5b6c8f7b62d387dbea3f804044350/browser/base/content/browser-places.js#1523,1567
Though that same code could also manage to hide the text until we know whether it's necessary to show it, and that should just work for any window without the need to pass info around.