Closed Bug 1367160 Opened 8 years ago Closed 6 years ago

Consider removing default context menu items in extension frames

Categories

(WebExtensions :: Frontend, enhancement, P2)

54 Branch
enhancement

Tracking

(firefox64 fixed)

RESOLVED FIXED
mozilla64
Iteration:
64.2 - Sep 28
Tracking Status
firefox64 --- fixed

People

(Reporter: jkt, Assigned: robwu)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [design-decision-approved])

Attachments

(3 files)

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.
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)
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
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.
> 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.
Severity: normal → enhancement
Priority: -- → P5
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)
Product: Toolkit → WebExtensions
Blocks: 1280347
Assignee: nobody → rob
Status: NEW → ASSIGNED
Iteration: --- → 64.2 (Sep 28)
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.
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
Depends on D6622
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+
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
It's probably best if the documentation writing efforts is combined with bug 1280347.
Keywords: dev-doc-needed
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
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!

Added a definition for: overrideContext to describe the ability to show the menu items defined for an extension rather than the default menus.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: