1495433 - Bookmarks Menu icon disappears in toolbar after clicking "add Bookmarks Menu to toolbar"

Status: NEW

(Firefox :: Toolbars and Customization, defect, P3)

Description

[u625328]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180920131237

Steps to reproduce:

1. Have bookmarks menu icon in toolbar at top right.
2. Click the bookmarks menu icon, then "bookmarking tools", then "add bookmarks menu to toolbar".
3. Bookmark menu icon disappears.



Actual results:

Bookmarks Menu disappears. 


Expected results:

The option "add bookmarks menu to toolbar" should not be there when Bookmarks Menu icon is already in the toolbar, or the option should not have done anything and gave an error that the icon is already there.

Comment 1

[u625328]

Forgot to mention that it happened once in the toolbar, and the steps can be reproduced on my end all the time when it's in the Overflow Menu.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Hey ngmx,

Is there still a gap for the bookmarks toolbar items in your main toolbar, but the toolbar is empty? Can you show us a screenshot?

Comment 3

[Alice0775 White]

maybe you are misunderstanding this bug.

Steps To Reproduce:
1. Open Customize
2. Place Bookmarks Menu widget on Tab bar
3. Enable Bookmarks toolbar
4. Exit the customize mode

5. Open hamburger menu. 
   And select Library > Bookmarks > Bookmarking Tools > Add Bookmarks Menu to Toolbar

Actual Results:
The Bookmarks Menu widget is gone from browser view. It will be aveilable in the Customize panel.

Expected Results:
The menu "Add Bookmarks Menu to Toolbar" should be disabled.

Or(but, I do not like)
The Bookmarks Menu widget is placed on Bookmarks Toolbar

Comment 4

[:Gijs (he/him)]

The issue is with this code:

https://hg.mozilla.org/mozreview/gecko/rev/5d9441fecb8f99e8b460a8cd8b7a8f2d09f558ea#l1.141

https://searchfox.org/mozilla-central/rev/924e3d96d81a40d2f0eec1db5f74fc6594337128/browser/base/content/browser-places.js#1665-1666

which makes the wording on the item (add vs. remove) be based on whether the item is in the *main nav bar*. So it assumes the item isn't there (and shows the "Add" wording) if it's in the tabstrip, menubar, or bookmarks toolbar, or (manually pinned to) overflow menu.

Then the code that actually runs when clicking this item:

https://searchfox.org/mozilla-central/rev/924e3d96d81a40d2f0eec1db5f74fc6594337128/browser/base/content/browser-places.js#1674-1708

checks if the placement is null or not (ie is it in the palette / customizable tools or "somewhere in the UI"), and adds/removes accordingly.

These two things should be consistent. I don't know what the desired behavior should be, exactly (e.g. move the item to the navbar if it's in another toolbar, or just show "Remove" in that case, which may be confusing if the item is in a location that's not visible, e.g. a collapsed bookmarks toolbar or menubar, or the pinned overflow menu). Aaron, can you come up with something?

Comment 5

[u625328]

Hey Mike,

The bookmarks menu icon (star with a square bracket from below) just disappears after clicking on the "add to toolbar", and doesn't leave a gap. It's as if I removed the bookmarks icon and it hides back in to the customization page.

https://i.imgur.com/y1GXX1d.png

Comment 6

[Aaron Benson]

It looks like this is fixed in Nightly. The menu item reads "Remove Bookmarks Menu From Toolbar" which I think makes sense. Let me know if I'm missing something..

Comment 7

[:Gijs (he/him)]

(In reply to Aaron Benson from comment #6)
> It looks like this is fixed in Nightly. The menu item reads "Remove
> Bookmarks Menu From Toolbar" which I think makes sense. Let me know if I'm
> missing something..

I think you're missing something. :-)

If you follow the following steps:

0. clean profile
1. open customize mode
2. drag+drop the 'bookmarks menu' button to the *tabstrip* (e.g. next to the [+] button and/or "list all tabs" dropdown button)
3. exit customize mode
4. click library button, then 'bookmarks', then 'bookmarking tools'

... then the button says "Add to Toolbar", but clicking it will remove the item.

I outlined a few issues with possible solutions in comment #4 as well. Specifically, in the steps I just gave the button is visible, so saying "Remove from toolbar" makes some sense. But if you put the button the menubar (on windows/linux) or on the bookmarks toolbar or pin it to the overflow menu/panel, then it may not be visible (because the relevant toolbar/menu may be hidden). So having the button say "remove" in those cases might confuse users.

Comment 8

[Aaron Benson]

I think the state of the menu item (Library > Bookmarks > Bookmarking Tools > Add/Remove Bookmark Menu to Toolbar) should reflect whether or not the Bookmark menu button is visibly in the toolbar or not. If the Bookmark menu button is in the tab strip, the action should read "Add Bookmark Menu to Toolbar". Similarly, if it's in the overflow menu (or anywhere else, really), the menu should read "Add Bookmark Menu to Toolbar". The menu item is specifically stating the action of putting the button into the *toolbar* and should be a binary option. 


Should they inadvertently move the Bookmark menu button into, or out of, the toolbar by exploring that menu item, then they can place it back where they prefer it through customize mode.