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)
Tracking
(firefox64 fixed)
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.
Updated•8 years ago
|
Component: WebExtensions: General → WebExtensions: Frontend
Whiteboard: [design-decision-needed]
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#
Comment 2•7 years ago
|
||
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.
Updated•7 years ago
|
Flags: needinfo?(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.
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
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.
Flags: needinfo?(amckay)
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.
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.
Updated•7 years ago
|
Severity: normal → enhancement
Priority: -- → P5
Updated•7 years ago
|
Priority: P5 → P2
Updated•7 years ago
|
Flags: needinfo?(mconca)
Updated•7 years ago
|
Flags: needinfo?(mconca)
Updated•7 years ago
|
Whiteboard: [design-decision-needed] → [design-decision-needed] [needs-follow-up]
Comment 9•7 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
Comment 10•7 years ago
|
||
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]
Comment 11•7 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
Flags: needinfo?(mixedpuppy)
Updated•6 years ago
|
Product: Toolkit → WebExtensions
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → rob
Status: NEW → ASSIGNED
Iteration: --- → 64.2 (Sep 28)
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.
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
Assignee | ||
Comment 14•6 years ago
|
||
Depends on D6622
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.
Attachment #9011392 -
Flags: review+
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.
Attachment #9011394 -
Flags: review+
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.
Attachment #9011393 -
Flags: review+
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
Assignee | ||
Comment 19•6 years ago
|
||
It's probably best if the documentation writing efforts is combined with bug 1280347.
Keywords: dev-doc-needed
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
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
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!
Updated•6 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Comment 22•6 years ago
|
||
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.
Description
•