Closed Bug 1318791 Opened 8 years ago Closed 8 years ago

Open popups (browser/page/sidebar action) from a context menu action

Categories

(WebExtensions :: General, defect)

Product:
WebExtensions
Component:
General
Version:
50 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: contact, Assigned: mixedpuppy)

References

Details

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

Attachments

(1 file)

Bug 1318791 support internal commands to open actions from context menus,
59 bytes, text/x-review-board-request
kmag
: review+
Details 
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161104212021

Steps to reproduce:

I'm working on an extension and I was looking for a way to fire the same popup as on a browser action with a click on a context menu action.


Actual results:

The API doesn't provide such an interaction.


Expected results:

We should be able to mimic the same action than on a browser action from a context menu one.
browser.browserAction.openPopup() seems to be the solution (cf. https://bugzilla.mozilla.org/show_bug.cgi?id=1278180).
Blocks: webext-port-save-to-pocket
Whiteboard: [design-decision-needed]
/me investigating
Assignee: nobody → mixedpuppy
Comment on attachment 8838983 [details]
Bug 1318791 support internal commands to open actions from context menus,

Looking for direction feedback.

This proposal adds an optional command for contextMenus.create:

chrome.contextMenus.create({
  id: "open_browser_action",
  title: "Open Browser Action",
  contexts: ["all"],
  command: "_execute_browser_action",
});

It also allows the onclicked listener to be additionally called which is useful for collecting stuff like selectedtext, which could then be messaged to the action.

command could be expanded to support any commands added by the addon, perhaps just calling doCommand on the provided command (and skipping the onclicked listener).
Attachment #8838983 - Flags: feedback?(kmaglione+bmo)
Attachment #8838983 - Flags: feedback?(aswan)
Attachment #8838983 - Flags: feedback?(amckay)
Wouldn't it be more useful to make the browser.browserAction.openPopup method available instead of adding a "command" key, and then require an explicit user gesture to have occurred in order to open the popup?

User gestures include:
- Contextmenu event
- Shortcut
- Click on notification

This allows for more flexibility, e.g. setting the popup page based on the option before opening it:
browser.browserAction.setPopup( ... ).then( () => ... browser.browserAction.openPopup());


And possibly with a new permission, to make it easier for add-on reviewers to see whether the add-on makes use of such a potentially annoying API.

There has been lengthy discussions about making chrome.browserAction.openPopup a public API in Chrome, see https://crbug.com/436489.
I like Robs proposal. I'm fine with it being a permission, but I don't think its one that the end user needs prompting for unless we see it being abused a lot.
That is a lengthy discussion, over 3 years in the making, last chrome person to comment was almost 2 years ago.

In the case of providing an API like that, if nsIDOMWindowUtils.isHandlingUserInput can be used somehow, there is no need for a permission.

I'd prefer the approach I've taken over new APIs (in this particular case) because; I think there will be more issues and edge cases in the API approach.  The command option builds on top of the same implementation used for commands.  It does not preclude adding the APIs later.  The command approach is probably land-able this week (given a test and reviews).
You are right that both yours and Robs suggestions can be implemented independently of each other. I'm just a little hesitant about adding in a new Firefox only parameter onto the commands API. I don't feel there's a huge rush for this in 54.
If you are mainly concerned about ease of implementation, skip the "user gesture requirement" part for now, and add a permission. This permission makes it easier to audit the add-on by reviewers at AMO, which is even more effective at preventing abuse than what an arbitrary "user gesture" requirement can do.

I suggest to make the browserAction.openPopup, pageAction.openPopup (and sidebarAction.showPanel?) available as an API (not guarded on any permission except for the existence of the *_action key in manifest.json), and reject the promise if the add-on is now allowed to open the popup. Initially, this is just a test whether the permission is set, later on we can add more sophisticated conditions if necessary.
It's not primarily ease of implementation.  Reading through that conversation, there are a number of issues that we have to decide on (have we had this in our team?).  Given that the chromium implementation hasn't happened and the conversation died out, we'd also have to be fine with possibly being incompatible if they pick it up again.  I haven't made my way through it all yet, and I'd rather not drag through all the issues in this bug, a google doc may be better.
Summary: Open popup from the browser action on a context menu action → Open popups (browser/page/sidebar action) from a context menu action
See Also: → 1341126
Comment on attachment 8838983 [details]
Bug 1318791 support internal commands to open actions from context menus,

Fine with me. We should probably also consider adding an API to do this when triggered by user interaction, though.
Attachment #8838983 - Flags: feedback?(kmaglione+bmo) → feedback+
Comment on attachment 8838983 [details]
Bug 1318791 support internal commands to open actions from context menus,

https://reviewboard.mozilla.org/r/113742/#review123702

::: browser/components/extensions/ext-contextMenus.js:247
(Diff revision 2)
> +      if (name == "_execute_page_action") {
> +        action = pageActionFor(item.extension);
> +      } else if (name == "_execute_browser_action") {
> +        action = browserActionFor(item.extension);
> +      } else if (name == "_execute_sidebar_action") {
> +        action = sidebarActionFor(item.extension);
> +      }

Can we just make this an object lookup?

    {
      _execute_page_action: pageActionFor,
      _execute_browser_action: browserActionFor,
      _execute_sidebar_action: sidebarActionFor,
    }
Attachment #8838983 - Flags: review?(kmaglione+bmo) → review+
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f3fa2acf658b
support internal commands to open actions from context menus, r=kmag
https://hg.mozilla.org/mozilla-central/rev/f3fa2acf658b
Status: UNCONFIRMED → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Keywords: dev-doc-needed
Whiteboard: [design-decision-needed] → [design-decision-approved]
Hey,
Can I play with that on my Nightly 55? Any hint to use it is welcome.
draft docs: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/contextMenus/create

Does this cover it?
Flags: needinfo?(mixedpuppy)
looks good.
Flags: needinfo?(mixedpuppy)
Depends on: 1380269
Is there a way to distinguish a call from the command action and the one "simulated" from the context menu?
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: