add open API to browser/page/sidebarAction

NEW
Assigned to

Status

()

Toolkit
WebExtensions: Frontend
3 months ago
12 days ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

({dev-doc-needed})

49 Branch
dev-doc-needed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [design-decision-approved] triaged, URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

3 months ago
Chrome has a private api, browserAction.openPopup, which has been in discussion for making available to extensions.  This bug is to define the issues and requirements for the same or similar API in Firefox.

An initial read through the chromium discussion I've pulled out the following issues.  Proposed solutions (from that conversation) are marked with *.

Issues:

- multiple extensions calling openPopup
  * openPopup fails if a popup is open
- extensions calling openPopup while firefox UI is open (e.g. geoloction prompt)
  * openPopup fails if primary ui is open
- identification of extension when buttons are located in overflow/hamburger menu
  * Dont allow openPopup when button is not visible in toolbar
- attributing the opening of a panel to an action
  * require user action to call openPopup
- abusive practices
  - e.g. opening a panel on every page load
  - looking like trusted ui
  * require user action to call openPopup
- intrusive practices
  - e.g. stealing focus from a form while user is typing
  * require user action to call openPopup
- popups get activeTab permission
  * require user action to call openPopup
- chrome conversation died out, we risk incompatible implementations of the same api
(Assignee)

Comment 1

3 months ago
I'm actually having a hard time figuring out why requiring user action wouldn't solve all the issues listed in the chromium bug.
For what it's worth, this is one of the APIs SnoozeTabs would like to implement some missing functionality.

Updated

3 months ago
Whiteboard: [design-decision-needed]
This has been added to the March 7 WebExtensions Triage mtg. agenda. 

Agenda: https://docs.google.com/document/d/1zzfedbTKAHUmm4UctxSOIJV3iKayXQ8TuXPodXW8wC0/edit#
(Assignee)

Updated

3 months ago
See Also: → bug 1343817
(Assignee)

Comment 4

3 months ago
We should also understand what will happen with the pocket addon, which could bump priority on this.
Whiteboard: [design-decision-needed] → [design-decision-approved]
Comment hidden (mozreview-request)
(Assignee)

Comment 6

3 months ago
Comment on attachment 8845285 [details]
Bug 1341126 implement open for browser/page/sidebar actions

Looking for general feedback on this approach.
Attachment #8845285 - Flags: feedback?(kmaglione+bmo)
Attachment #8845285 - Flags: feedback?(aswan)

Comment 7

3 months ago
mozreview-review
Comment on attachment 8845285 [details]
Bug 1341126 implement open for browser/page/sidebar actions

https://reviewboard.mozilla.org/r/118454/#review121790

::: browser/components/extensions/ext-browserAction.js:567
(Diff revision 1)
> +        let popup = BrowserAction.for(extension).getProperty(null, "popup");
> +        if (!popup) {
> +          return;
> +        }
> +        let window = windowTracker.topWindow;
> +        BrowserAction.for(extension).triggerAction(window);

This is going to wind up granting the activeTab permission if it's not already granted. Please add a test for this.

Also, if we only want to trigger the popup variant here, let's separate that out into a separate method rather than using triggerAction.

::: browser/components/extensions/ext-c-browserAction.js:12
(Diff revision 1)
> +        let window = Services.wm.getMostRecentWindow(null);
> +        let dwu = window.QueryInterface(Ci.nsIInterfaceRequestor)
> +                         .getInterface(Ci.nsIDOMWindowUtils);
> +        if (!dwu.isHandlingUserInput) {

Let's just transmit this information to the parent process from the binding layer, and either store it on the context, or set the internal flag while the method call is being dispatched.
Attachment #8845285 - Flags: feedback?(kmaglione+bmo) → feedback+

Comment 8

2 months ago
Comment on attachment 8845285 [details]
Bug 1341126 implement open for browser/page/sidebar actions

This is really Kris's domain, clearing the f? since he already responded
Attachment #8845285 - Flags: feedback?(aswan)
(Assignee)

Updated

2 months ago
Assignee: nobody → mixedpuppy
Component: WebExtensions: Untriaged → WebExtensions: Frontend
Whiteboard: [design-decision-approved] → [design-decision-approved] triaged
(Assignee)

Updated

2 months ago
webextensions: --- → ?

Updated

2 months ago
webextensions: ? → ---
Keywords: dev-doc-needed

Comment 9

2 months ago
If I understand the patch correctly it only allows to toggle the sidebar. In order to be sure if the sidebar is being opened or being closed there needs to be at least a way to read the current state of the sidebar in the current window (closed, opened). And as far as I know there is no such way currently without workarounds.
(Assignee)

Comment 10

2 months ago
(In reply to Croydon from comment #9)
> If I understand the patch correctly it only allows to toggle the sidebar. In
> order to be sure if the sidebar is being opened or being closed there needs
> to be at least a way to read the current state of the sidebar in the current
> window (closed, opened). And as far as I know there is no such way currently
> without workarounds.

There are workarounds, you can use load/unload and send a message to the background when opened/closed.  But I agree, it makes it easy if there is a state variable.  The problem is that sidebars can be different in different windows, so this would have to be attached to some other API where it makes better sense.  Maybe add a sidebarState to Window:

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/windows/Window

The value would only be "open" if the addon's sidebar is open, it would be "closed" if some other sidebar is open or no sidebar is open.
You need to log in before you can comment on or make changes to this bug.