Jonathan Kingston [:jkt] he/him Reporter
	Description • 7 years ago

Bug 1280347 suggested implementing a binding of HTML elements to tabs themselves but instead I would like just allow extensions to prevent context menu default items.

Custom context menus can be added to extension sidebars however they can't override the existing ones, this can cause confusion for some of the items in a menu. The same can be true for browser/page actions too however less of an issue.

The wording also sometimes could be wrong too "Link" instead "Tab" for example.

My original proposal was to use HTML for changing these context items, however I am solution agnostic on changing/removing and or modifying these items.

	Caitlin Neiman [:caitmuenster]
	Comment 1 • 7 years ago

Hi Jonathan, this has been added to the agenda for the July 25 WebExtensions APIs triage meeting. Hope to see you there!

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Next_Meeting

Agenda: https://docs.google.com/document/d/1BBIZhiHG1zlQiu6744jiAYyWJLa-B0iRu9vzWypkvF4/edit#

	Edouard Oger [:eoger]
	Comment 2 • 7 years ago

This could help tab center redux implement native context menus.

	Croydon
	Comment 3 • 7 years ago

It would also be really helpful for my Vertical Tabs Reloaded add-on. With the default context menus it doesn't look and fell like native UI elements and as mentioned above it is causing confusing.

	Shane Caraveo (:mixedpuppy)
	Comment 4 • 7 years ago

notes from the discussion:

We had a previous discussion on removing context menus but my memory on it is flaky.  It is probably ok for an addon to remove default context menus on its own panels, but not on other addons or web content.   This would help some addons, likely tab manager type sidebars.

I'd want to see the current default context menus reviewed to identify what is fine (or not) is allow an addon to disable/remove.  

I think this is on hold for another opinion, maybe andy.

	Andrew Sutherland [:asuth] (he/him)
	Comment 5 • 7 years ago

In the event the default items[1] are deemed worth providing in some cases, it's worth noting that the <video> tag provides some precedent here.  If you go to YouTube and right-click on a video, you get the YouTube provided menu.  If you right-click a second time, you get Firefox's video context menu.  If you shift-right-click the first time (on linux), you get Firefox's video context menu immediately.

1: On linux nightly for me on an effectively blank sidebar page, I'm seeing:
- Save Page As...
- (spacer)
- Send Page To Device > [submenu that's empty for me]
- (spacer)
- View Background Image
- Select All
- (spacer)
- View Page Source
- View Page Info

	Andy McKay
	Comment 6 • 7 years ago

Why does tab center redux (comment 2) need native menus though?

It seems that the main argument is that the native context menu is better and does some things differently from the contextmenu API. I'd rather we spent the time on one or the other menu system as opposed to supporting both.

	Kazumasa Hasegawa (Kazz)
	Comment 7 • 7 years ago

I also create add-on that display tabs in the sidebar.
Although there may be some points that are out of the scope of this bug, these are what I noticed.

* The contextMenus API in WebExtensions can not be used in the sidebar.
  Clicking the menuitem will cause a TypeError.
* Adding menus to native context menus, HTML `menu` element and `menuitem` element are removed from the spec, so they will be unsupported in Firefox soon.
  Bug 1372276
* Also, as handled in this bug, in the native context menu, menuitems for the sidebar and menuitems for the web page are mixed.
* If add-on implement context menu independently, there are restrictions on the expansion of submenus etc. in the narrow area of ​​the sidebar.

I feel it is necessary to add an contextMenus API for sidebar.

	Jan Niklas Hasse
	Comment 8 • 7 years ago

> Why does tab center redux (comment 2) need native menus though?

Because context menus behave and look different on different platforms and people want that to be consistent with their OS.

One example of different behaviour is that on Linux you can right-click, hold, release to open the context menu *and* select an item in it.

	Caitlin Neiman [:caitmuenster]
	Comment 9 • 6 years ago

We'll be revisiting this at the March 13, 2018 WebExtensions APIs triage. All who are interested in this bug are welcome to attend. 

Relevant Links: 

* Wiki for the meeting: https://wiki.mozilla.org/WebExtensions/Triage#Next_Meeting
* Meeting agenda: https://docs.google.com/document/d/1b4r8z964_Est_mbSYUx9jtRt-HTtXgu-EAzM_3yr7ww/edit
* Vision doc for WebExtensions: https://wiki.mozilla.org/WebExtensions/Vision

	Caitlin Neiman [:caitmuenster]
	Comment 10 • 6 years ago

This has been approved with a few constraints. Shane to follow up with some comments about what those constraints are. :)

	Shane Caraveo (:mixedpuppy)
	Comment 11 • 6 years ago

Some Constraints:

- extensions can hide items only on its own panels/pages
- we provide an enum in schema of those menus that are hideable by extensions.
  - we may choose to disallow certain context menus being disabled (e.g. viewsource, page info/security info)
  - should we have some privacy/security review around what is hideable?
- we should consider a special key combo to show the default menus that would normally be shown

	Rob Wu [:robwu] Assignee
	Comment 12 • 6 years ago

This refactor is mainly meant to support bug 1367160, but it also eases
the implementation of bug 1294429, bug 1325758 or bug 1370735 if we ever
decide to fix those bugs.

Previously, the menu was constructed by creating one root menu item
and moving the submenu to the root if there was only one item.

The refactored code constructs the menu items by generating a list of
(potentially more than one) top-level menu items, and moving excess menu
items to a submenu (as before). Besides the ability to support an
arbitrary number of top-level menu items, this new implementation also
makes it easier to insert menu items and optionally separators at
arbitrary locations in the menu.

The refactored code obsoletes some separate code paths for browserAction
and pageAction menus, which improves the maintainability of the code.

There are two user-visible functional changes in this commit:
- Excess action menu items (more than ACTION_MENU_TOP_LEVEL_LIMIT = 6)
  are not silently discarded, but shown in a submenu. See updated test.
- menus.onShown/onHidden are now always fired when the action menu is
  shown, even if the extension has not created any action menu items.

	Rob Wu [:robwu] Assignee
	Comment 13 • 6 years ago

The new method allows extensions to modify menu items in their own
moz-extension:-pages, with the following features:

- All matching extension items are shown in the root menu (instead of
  being moved into a submenu), above other menu items, if any.
- The icons for these menu items are customizable.
- Optionally, the default menu items (including those from other
  extensions) can be hidden.

Depends on D6621

	Rob Wu [:robwu] Assignee
	Comment 14 • 6 years ago

Depends on D6622

	Shane Caraveo (:mixedpuppy)
	Comment 15 • 6 years ago

Comment on attachment 9011392 [details]
Bug 1367160 - Refactor to support multiple menu items in the root menu

Shane Caraveo (:mixedpuppy) has approved the revision.

	Shane Caraveo (:mixedpuppy)
	Comment 16 • 6 years ago

Comment on attachment 9011394 [details]
Bug 1367160 - Add tests for overriding extension menus

Shane Caraveo (:mixedpuppy) has approved the revision.

	Shane Caraveo (:mixedpuppy)
	Comment 17 • 6 years ago

Comment on attachment 9011393 [details]
Bug 1367160 - Allow extensions to hide default menu items

Shane Caraveo (:mixedpuppy) has approved the revision.

	Pulsebot
	Comment 18 • 6 years ago

Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/04b6a658785b
Refactor to support multiple menu items in the root menu r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/83c3c64b73ba
Allow extensions to hide default menu items r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/0918353c86a1
Add tests for overriding extension menus r=mixedpuppy

	Rob Wu [:robwu] Assignee
	Comment 19 • 6 years ago

It's probably best if the documentation writing efforts is combined with bug 1280347.

	Cristina Coroiu [:ccoroiu]
	Comment 20 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/04b6a658785b
https://hg.mozilla.org/mozilla-central/rev/83c3c64b73ba
https://hg.mozilla.org/mozilla-central/rev/0918353c86a1

	Chris Mills [:cmills]
	Comment 21 • 6 years ago

Note to docs team:

I added a note to the Fx64 rel notes covering this:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/64#Changes_for_add-on_developers

But I'm really not sure I got this right. When you come to properly document this, check it and update as needed. Thanks!