1507536 - Redundant right click items on the menu bar

Status: VERIFIED FIXED

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

Description

[Itiel]

STR:
1. Right click the tab bar, enable Menu Bar
2. Right click the menu bar

AR:
Redundant right click items (Manage Extension, Remove Extension, Auto-Hide in Toolbar), and the items 'Pin to Overflow Menu' and 'Remove from Toolbar' are not disabled

ER:
The same as the attached "Before bug 1356920"

This is a regression from bug 1356920.

Comment 1

[Itiel]

Attachment: Before bug 1356920

Comment 2

[Itiel]

Attachment: After bug 1356920

Comment 3

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

This is a bit confusing and sloppy, but as far as I can tell, these context menu items don't actually do anything when clicked. Marking P3, but we should clean this up.

Comment 4

[Itiel]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #3)
> but as far as I can tell, these context
> menu items don't actually do anything when clicked.

Some of them actually do something:
'Manage Extension' goes to about:addons in the Get Addons tab, with an infinite 'Loading' text showing up
'Auto-Hide in Toolbar' hides the Downloads button from the toolbar

Comment 5

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

Attachment: Bug 1507536 - Remove spacer in toolbar-menubar, and just make the menubar items flex instead. r?dao

This is necessary to avoid an exception being thrown from CustomizableUI.isWidgetRemovable,
which expects all direct descendants of toolbars to either have an ID, be one of a special
set of node types, or have skipintoolbarset="true" set on it.

Comment 6

[Dão Gottwald [:dao]]

(In reply to Itiel from comment #4)
> (In reply to Mike Conley (:mconley) (:⚙️) from comment #3)
> > but as far as I can tell, these context
> > menu items don't actually do anything when clicked.
> 
> Some of them actually do something:
> 'Manage Extension' goes to about:addons in the Get Addons tab, with an
> infinite 'Loading' text showing up
> 'Auto-Hide in Toolbar' hides the Downloads button from the toolbar

Indeed, this seems quite broken.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd00df2e0aba
Remove spacer in toolbar-menubar, and just make the menubar items flex instead. r=dao

Comment 9

[Stefan Hindli [:stefan_hindli]]

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

Comment 10

[Itiel]

LGTM. Thanks!

Comment 11

[Cosmin Sabou [:CosminS]]

Backed out changeset fd00df2e0aba (bug 1507536) for causing bug 1509711 as mconley requested.
https://hg.mozilla.org/mozilla-central/rev/204cda7581188cfc8c8ef11dce4680dadf2b43bb

Comment 12

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

Attachment: Bug 1507536 - Set skipintoolbarset="true" on some titlebar items to avoid redundant context menu items. r?dao

This adds a skipintoolbarset to the toolbar-menubar spacer and the titlebar-buttonbox-container
hbox to avoid a case where an exception can be thrown from CustomizableUI.isWidgetRemovable,
which expects all direct descendants of toolbars to either have an ID, be one of a special
set of node types, or have skipintoolbarset="true" set on it.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d476f054342f
Set skipintoolbarset="true" on some titlebar items to avoid redundant context menu items. r=dao

Comment 14

[Cosmin Sabou [:CosminS]]

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

Comment 15

[Mohammad Maruf Rahman]

I have reproduced this bug with Nightly 65.0a1 (2018-11-15) on Windows 7, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID 	20181203093109
User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0