Closed
Bug 1324429
Opened 8 years ago
Closed 8 years ago
Context menu submenu child items should inherit? the contexts from parent by default
Categories
(WebExtensions :: Frontend, defect, P1)
WebExtensions
Frontend
Tracking
(firefox53 fixed)
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: zombie, Assigned: rpl, Mentored)
References
Details
(Keywords: dev-doc-complete, Whiteboard: [tab context] triaged)
Attachments
(1 file)
While working on bug 1316020, Blake noticed that child items of a submenu don't show up if you don't specify the context. According to our current docs (and implementation), the default context if omitted is "page", which Kris thinks we should change.
My current best idea is:
- if the menu item has a parent,
- and it doesn't have an explicit context set,
- but the parent has
then:
- the child menu item should inherit the context.
Not sure how likely this change is to break existing extensions.
Reporter | ||
Updated•8 years ago
|
Mentor: tomica
Keywords: good-first-bug
Updated•8 years ago
|
Assignee: nobody → lgreco
Priority: -- → P1
Whiteboard: [tab context]
Updated•8 years ago
|
Keywords: good-first-bug
Updated•8 years ago
|
Whiteboard: [tab context] → [tab context] triaged
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8820926 -
Flags: review?(kmaglione+bmo)
Assignee | ||
Comment 2•8 years ago
|
||
The attached patch pushed to try:
- https://treeherder.mozilla.org/#/jobs?repo=try&revision=75ae7315acf91a1dfa652824a52013785ee52f8d
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•8 years ago
|
||
Hey Andy, you meant to WONTFIX this in bug 1266455 comment 6. Wanna double-check this?
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(amckay)
Comment 5•8 years ago
|
||
My main concern was breaking the compat. with Chrome for something that could be specified manually. If I read this correctly, if you specify it all manually correctly it all works well. It's just the default case where its not specified that would be good to get right.
I took a bit more time to read this and had a quick play around and it sounds like whilst your suggestion is different from Chrome, it is a perfectly reasonable and sane solution. Perhaps I was too hasty in my comment on bug 1266455. This solution in comment 0 seems reasonable to me.
Flags: needinfo?(amckay)
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8820926 [details]
Bug 1324429 - Context menu items without contexts should inherit it from their parent.
https://reviewboard.mozilla.org/r/100320/#review104296
Attachment #8820926 -
Flags: review?(kmaglione+bmo) → review+
Comment 7•8 years ago
|
||
There shouldn't be any compatibility issues. The current default context is "all", which works the same as this approach for in-page context menus. The only place where it makes a difference is in things like tab and page_action menus.
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4926f3ca5c90
Context menu items without contexts should inherit it from their parent. r=kmag
Keywords: checkin-needed
Comment 9•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Updated•8 years ago
|
Keywords: dev-doc-needed
Comment 10•8 years ago
|
||
I've updated: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/contextMenus/create
Please let me know if this covers it.
Flags: needinfo?(lgreco)
Updated•7 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•