Consider removing default context menu items in extension frames

RESOLVED FIXED in Firefox 64

Status

enhancement
P2
normal
RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: jkt, Assigned: robwu)

Tracking

(Blocks 1 bug, {dev-doc-needed})

54 Branch
mozilla64
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox64 fixed)

Details

(Whiteboard: [design-decision-approved])

Attachments

(3 attachments)

(Reporter)

Description

2 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.
Component: WebExtensions: General → WebExtensions: Frontend
Whiteboard: [design-decision-needed]
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#
This could help tab center redux implement native context menus.

Comment 3

2 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.
Flags: needinfo?(mixedpuppy)
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.
No longer depends on: 1332447
Flags: needinfo?(mixedpuppy) → needinfo?(amckay)
See Also: → 1215376
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

Comment 6

2 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.
Flags: needinfo?(amckay)
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.

Comment 8

2 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.

Updated

2 years ago
Severity: normal → enhancement
Priority: -- → P5
See Also: → 1416839
Priority: P5 → P2
Flags: needinfo?(mconca)
Flags: needinfo?(mconca)
Whiteboard: [design-decision-needed] → [design-decision-needed] [needs-follow-up]
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
This has been approved with a few constraints. Shane to follow up with some comments about what those constraints are. :)
Flags: needinfo?(mixedpuppy)
Whiteboard: [design-decision-needed] [needs-follow-up] → [design-decision-approved]
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
Flags: needinfo?(mixedpuppy)
Blocks: 1466876

Updated

10 months ago
Product: Toolkit → WebExtensions
(Assignee)

Updated

7 months ago
Blocks: 1280347
(Assignee)

Updated

7 months ago
Assignee: nobody → rob
Status: NEW → ASSIGNED
Iteration: --- → 64.2 (Sep 28)
(Assignee)

Comment 12

7 months 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.
(Assignee)

Comment 13

7 months 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
(Assignee)

Comment 14

7 months ago
Depends on D6622
(Assignee)

Updated

7 months ago
See Also: → 1493639
Comment on attachment 9011392 [details]
Bug 1367160 - Refactor to support multiple menu items in the root menu

Shane Caraveo (:mixedpuppy) has approved the revision.
Attachment #9011392 - Flags: review+
Comment on attachment 9011394 [details]
Bug 1367160 - Add tests for overriding extension menus

Shane Caraveo (:mixedpuppy) has approved the revision.
Attachment #9011394 - Flags: review+
Comment on attachment 9011393 [details]
Bug 1367160 - Allow extensions to hide default menu items

Shane Caraveo (:mixedpuppy) has approved the revision.
Attachment #9011393 - Flags: review+

Comment 18

7 months 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
(Assignee)

Comment 19

7 months ago
It's probably best if the documentation writing efforts is combined with bug 1280347.
Keywords: dev-doc-needed

Comment 20

7 months 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
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64

Updated

7 months ago
Flags: qe-verify-
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!
You need to log in before you can comment on or make changes to this bug.