1431943 - Let tabId be optional in pageAction methods

Status: RESOLVED WONTFIX

(WebExtensions :: Frontend, enhancement)

Description

[Oriol Brufau [:Oriol]]

It may make sense to require a tabId in enable/disable in order to discourage cluttering the location bar in all tabs.

However, I don't see the need to require it in the various get and set methods. For example, one can set a default_popup in the manifest and it affects all tabs, but then the script can't change the popup globally, only per tab. This is annoying and not consistent with browserAction.

So I think that tabId should be optional in setTitle, getTitle, setIcon, setPopup and getPopup. If omitted, it would get or set the global value to be used in tabs without a tab-specific one.

Comment 1

[Oriol Brufau [:Oriol]]

It seems it's possible to show a page action everywhere by default via show_matches, so I think the tabId should also be optional in pageAction.show() and pageAction.hide()

Comment 2

[Caitlin Neiman [:caitmuenster]]

Hi Oriol, this has been added to the WebExtensions APIs triage on February 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/1lB8M4WhF2xRrf4A5DlUNkKKiiuxfpxfx6CfHLv-FoAM/edit#
* Vision doc for WebExtensions: https://wiki.mozilla.org/WebExtensions/Vision

Comment 3

[Oriol Brufau [:Oriol]]

For example, currently setPopup signature is:

    browser.pageAction.setPopup({tabId, popup});

I want to also allow

    browser.pageAction.setPopup({popup});

This would set a global popup for all tabs that don't have a tab-specific pop-up.

This global popup would be read like this:

    await browser.pageAction.getPopup({});

It would be analogous for setTitle, getTitle and setIcon.

Similarly, show and hide currently require a tabId parameter:

    browser.pageAction.show(tabId);
    browser.pageAction.hide(tabId);

I would also allow

    browser.pageAction.show();
    browser.pageAction.hide();

This would override show_matches and hide_matches and set a global visibility for tabs without a tab-specific one.
It could be read as

    await browser.pageAction.isShown({});

Comment 4

[Shane Caraveo (:mixedpuppy)]

I'm starting to wonder how much we're diverging in some of these APIs where we need to maintain chrome compatibility.  So I wonder what functionality is enabled by this that is no otherwise possible.

Comment 5

[Shane Caraveo (:mixedpuppy)]

IOW, I wonder if a setDefaults api might be better, and leave tabId as required.  Maybe it's not really important.

Comment 6

[Caitlin Neiman [:caitmuenster]]

Hi Oriol, we're going to revisit this at the February 20 triage meeting. Would you be able to join us? 

Agenda: https://docs.google.com/document/d/1-edU5RRFo2TupsOW400AcTOj8yyQnXd0F7uSsqkEzck/edit#

Comment 7

[Oriol Brufau [:Oriol]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #4)
> So I wonder what functionality is enabled by this that is no otherwise possible.

For example, I guess an extension could expose some preference to the user to switch between different kinds of pageAction popups. And when the user changes that pref, the extension wants to change the popup globally.

Well, it's just the same that made tabId optional in browserAction and sidebarAction.
I don't see why pageAction should be more limited.

(In reply to Shane Caraveo (:mixedpuppy) from comment #5)
> IOW, I wonder if a setDefaults api might be better, and leave tabId as
> required.  Maybe it's not really important.

Might work, but that wouldn't be consistent with browser and sidebar actions.

(In reply to Caitlin Neiman [:caitmuenster] from comment #6)
> Would you be able to join us? 

Probably yes

Comment 8

[Oriol Brufau [:Oriol]]

So, if pageAction values can't be set globally, the workaround is setting them on every tab.
This should be done in tabs.onCreated and, due to automatic clearance on navigation, on tabs.onUpdated.

In my experience this has been a bit problematic. I believe the most reliable time to set the values again is when onUpdated says that the url changed, but this requires the "tabs" permission. The alternative is checking status, but sometimes this event only occurs for "loading" or only for "complete". So I end up setting the values regardless of the status value, which probably means I may be doing this more than necessary.

Anyways, when a tab opens, for a brief amount of time it will have the default manifest values before I assign what I want. So there will be some flickering, even if I reduce asynchronicity as much as possible.

Additionally, if I want to change the values, I have two possibilities:
 - Iterate all tabs and change the values for each one. This may be a problem if there are hundreds of tabs and I want to do this very frequently.
 - Set the updated values when a tab becomes active. This means there will be flickering when switching tabs.

Well, I think the flickering is less important for page actions than for browser actions (bug 1419893) because page actions are supposed to be visible only in specific tabs, unlike browser actions which are always visible in the toolbar. But an optional tabId would be nice and consistent with browser and sidebar actions.

Comment 9

[Mike Conca [:mconca]]

This feature request was denied at the 20-Feb-2018 design-decision-needed meeting, but the bug was never updated.

https://docs.google.com/document/d/1-edU5RRFo2TupsOW400AcTOj8yyQnXd0F7uSsqkEzck/edit#heading=h.hhpni8ijl0wx

Unfortunately, the meeting notes do not provide much context around the reason for the decision.