Closed Bug 1324429 Opened 7 years ago Closed 7 years ago

Context menu submenu child items should inherit? the contexts from parent by default

Categories

(WebExtensions :: Frontend, defect, P1)

defect

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.
Mentor: tomica
Keywords: good-first-bug
Assignee: nobody → lgreco
Priority: -- → P1
Whiteboard: [tab context]
Keywords: good-first-bug
Whiteboard: [tab context] → [tab context] triaged
Attachment #8820926 - Flags: review?(kmaglione+bmo)
Hey Andy, you meant to WONTFIX this in bug 1266455 comment 6.  Wanna double-check this?
Flags: needinfo?(amckay)
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 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+
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.
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
https://hg.mozilla.org/mozilla-central/rev/4926f3ca5c90
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
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)
Looks great, thanks Will.
Flags: needinfo?(lgreco)
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: