1440635 - support onShown notification for the extensions submenu

Status: NEW

(WebExtensions :: Frontend, enhancement, P5)

Description

[winflip]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180223001838

Steps to reproduce:

An extension I work on needs this functionality.
Use cases:
-The extension makes a request to a 3th party website. For privacy reasons, this should only happen when the user expands the extension's sub-menu, not every time the user opens the context menu. (and has text selected)
-To dynamically create sub-menus of sub-menus. Creating them all at once is not performant. Lazy loading is preferred.

Comment 1

[winflip]

Would be handy to get the selected text available as data in that event too, otherwise it's all for naught.

Comment 2

[Shane Caraveo (:mixedpuppy)]

Have you tried onBeforeShow?  Bug 1215376

Comment 3

[winflip]

(In reply to Shane Caraveo (:mixedpuppy) from comment #2)
> Have you tried onBeforeShow?  Bug 1215376

Yes, this triggers every time the context menu is opened, and as such doesn't solve the use cases described above.

Comment 4

[Caitlin Neiman [:caitmuenster]]

Hi winflip, this has been added to the agenda for the WebExtensions APIs triage meeting on March 13, 2018. Would you be able to join us? 

Here’s a quick overview of what to expect at the triage: 

* We normally spend 5 minutes per bug
* The more information in the bug, the better
* The goal of the triage is to give a general thumbs up or thumbs down on a proposal; we won't be going deep into implementation details

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 5

[Shane Caraveo (:mixedpuppy)]

Rob, since you did the onbeforeshow work, do you have any thoughts on this?

In general we wont do an onShow event as that timing can slow down firefox ui.  onbeforeshow might be able to be expanded to provide a hook that will cover the use case (if it does not already).

Comment 6

[Rob Wu [:robwu]]

(In reply to winflip from comment #1)
> Would be handy to get the selected text available as data in that event too,
> otherwise it's all for naught.

If you have the permissions to read data from the page, the selected text is already included in the menus.onShown event when the context type is "selection".


(In reply to Shane Caraveo (:mixedpuppy) from comment #5)
> Rob, since you did the onbeforeshow work, do you have any thoughts on this?

I understand the desire for this feature request, and my main questions are
1) do we want to give extensions the ability to track the users' interaction with the menu?
2) do we want to encourage the use of dynamic and delayed (sub)menu items?

The onShown event was added to offer extension authors the freedom to have dynamic menu items that do not fit in the declarative menu API. The event provides the full context that it necessary to build a menu for the contextual action (the context won't change, unless you view user interaction with the menu as a conscious request to change the context).

If we implement this feature request, then menus.refresh() should be changed to only update the modified menu items (instead of replacing the whole extension menu), or else the menu can disappear during user interaction.

Comment 7

[Caitlin Neiman [:caitmuenster]]

Sorry, all, we're actually going to hold off on discussing this until the 20th. I will circle back with the agenda when it is created!

Comment 8

[Caitlin Neiman [:caitmuenster]]

Hi all, this has been added to the agenda for the WebExtensions APIs triage on May 15, 2018. winflip, would you be able to join us? 

Here’s a quick overview of what to expect at the triage: 

* We normally spend 5 minutes per bug
* The more information in the bug, the better
* The goal of the triage is to give a general thumbs up or thumbs down on a proposal; we won't be going deep into implementation details

Relevant Links: 

* Wiki for the meeting: https://wiki.mozilla.org/WebExtensions/Triage#Next_Meeting
* Meeting agenda: https://docs.google.com/document/d/1Y_oYPldTT_kQOOouyJbC-8y3ASIizScLKFRhQfsDQWI/edit#
* Vision doc for WebExtensions: https://wiki.mozilla.org/WebExtensions/Vision

Comment 9

[winflip]

(In reply to Caitlin Neiman [:caitmuenster] from comment #8)
> Hi all, this has been added to the agenda for the WebExtensions APIs triage
> on May 15, 2018. winflip, would you be able to join us? 

I should be able to be there.

Comment 10

[Shane Caraveo (:mixedpuppy)]

The general idea here is to allow an extension to process context menus only when its submenu is shown, rather than when the context menu itself is shown.  We can extend the onBeforeShown notification to do this.

Use case:

xhr to translation service

Current implementation:

extension has to prepare and make modifications for every context menu shown notification, even if the user never looks at the extensions menu.  In this case that means doing the xhr for every context menu.

Resulting change:

extension only prepares when its own menu is shown, limiting the xhr call to happening only when necessary.

Comment 11

[Shane Caraveo (:mixedpuppy)]

TBH I don't really care if it is a new notification (ie. onShown) that is specific to extension menus.  That may make more sense technically, but I'll let Rob chime in on that when he has a chance.

Comment 12

[Rob Wu [:robwu]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #10)
> We can extend the onBeforeShown notification to do this.

There is no (blocking) menus.onBeforeShown event, only a menus.onShown event (which is fired when the main menu is shown).

I intend to add a new event (menus.onShownMenuItem) to support this feature,
together with the ability to refresh a specific menu item (via menus.refresh).

Comment 13

[Shane Caraveo (:mixedpuppy)]

(In reply to Rob Wu [:robwu] from comment #12)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #10)
> > We can extend the onBeforeShown notification to do this.
> 
> There is no (blocking) menus.onBeforeShown event, only a menus.onShown event
> (which is fired when the main menu is shown).

I don't want it to be blocking.

> I intend to add a new event (menus.onShownMenuItem) to support this feature,
> together with the ability to refresh a specific menu item (via
> menus.refresh).

"menus.onShownMenuItem" please don't make names like that.

Off the top of my head...I think it could work to do something like this:

browser.menus.onShown.addListener(listenerFn, filter)

Where the filter (or options) is a set of items that can be enabled or disabled per that listener.  One option could be "submenu", or "context_submenu".  An extension could use both to specify they want the event for both, or one or the other.  default would of course work as it does now.

I'd like to not finalize the api until it is considered with the full scope of menu changes.

Comment 14

[Rob Wu [:robwu]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #13)
> (In reply to Rob Wu [:robwu] from comment #12)
> > (In reply to Shane Caraveo (:mixedpuppy) from comment #10)
> > > We can extend the onBeforeShown notification to do this.
> > 
> > There is no (blocking) menus.onBeforeShown event, only a menus.onShown event
> > (which is fired when the main menu is shown).
> 
> I don't want it to be blocking.

Me neither. I have updated the summary to more clearly communicate this intent.

> Off the top of my head...I think it could work to do something like this:
> 
> browser.menus.onShown.addListener(listenerFn, filter)
> 
> Where the filter (or options) is a set of items that can be enabled or
> disabled per that listener.  One option could be "submenu", or
> "context_submenu".  An extension could use both to specify they want the
> event for both, or one or the other.  default would of course work as it
> does now.

I was thinking of a filter too, but then using menuItemId or menuItemIds.

> I'd like to not finalize the api until it is considered with the full scope
> of menu changes.

I'll ping you when I am going to work on this.
I have other questions, such as whether we want to provide the filter functionality for other menu events such as onHidden /  onClicked.

Comment 15

[exie]

3 years and still waiting. There are extensions that would profit from this functionality.