Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tabs.getCurrent() returns undefined when being called from the popup #316

Open
kiaraarose opened this issue Nov 3, 2022 · 22 comments
Open
Labels
inconsistency Inconsistent behavior across browsers proposal Proposal for a change or new feature supportive: chrome Supportive from Chrome supportive: firefox Supportive from Firefox

Comments

@kiaraarose
Copy link

I see that the MDN and Chrome documentation states that tabs.getCurrent() should return undefined when being called from an extension's popup. However, I think it's reasonable for a developer to want to know which tab the popup is currently open on. I'm curious to know what other developers think of this as I've seen some confusion about this online.

@tophf
Copy link

tophf commented Nov 4, 2022

It's the name which is misleading, a better name might be getOwnTab, but the behavior is correct: it should return undefined in a non-tab context such as the popup.

As for a new method, it would be essentially the same as (await chrome.tabs.query({active: true, currentWindow: true}))[0], however there are some problems with naming:

  • getActiveTab? but there's an active tab for each window
  • getCurrentTab? but it's a different thing
  • getPopupTab? but it can be used outside of the popup with the same result
  • getActiveTabForCurrentWindow? it'll be almost as long as the query code

@xeenon
Copy link
Collaborator

xeenon commented Nov 16, 2022

Safari currently returns the active tab in the active window when this is called from the background page. And it returns the active tab in the window showing the popup when called from the popup page.

@xeenon xeenon added the inconsistency Inconsistent behavior across browsers label Nov 16, 2022
@dotproto dotproto removed the agenda Discuss in future meetings label Mar 16, 2023
@carlosjeurissen
Copy link
Contributor

carlosjeurissen commented Sep 13, 2023

Use case:
When wanting to use executeScript in mv3, you are required to specify a tabId. This is problematic for extension popups as one would have to rely on tabs.query({currentWindow: true, active: true}). Which could be different from the tab the extension popup was opened on.

Proposal:
Let tabs.getCurrent() return the tab the extension popup was invoked upon. This makes sure executeScript is applied to the correct tab.

Since Safari has weird behaviour currently with tabs.getCurrent() (active tab in current window). It might make sense to use a different name. like tabs.getAssociated().

@carlosjeurissen carlosjeurissen added the proposal Proposal for a change or new feature label Sep 13, 2023
@rdcronin rdcronin added the supportive: chrome Supportive from Chrome label Sep 13, 2023
@rdcronin
Copy link
Contributor

I mentioned in #431, I mentioned I'd be supportive of changing this behavior to support returning the associated tab in an extension popup. In Chrome, extension popups are strictly associated with a single tab (switching tabs in the window closes the popup), so there is no ambiguity about which tab is associated. It's also inline (ish) with the motivation of the API, which is "get the tab this thing is running on". While it's not returning the document the script is running in, I think the intention of the developer is clear from this context.

As @carlosjeurissen mentioned, it may make sense to explore using a different name, not least because "tabs.getCurrent()" is endlessly confused with getting the currently-active tab; we'll discuss that more.

@Rob--W Rob--W added the supportive: firefox Supportive from Firefox label Sep 13, 2023
@Rob--W
Copy link
Member

Rob--W commented Sep 13, 2023

I am in favor of returning the tab that was clicked when the popup was opened.

If extensions are interested in getting the currently active tab, they can use browser.tabs.query({ currentWindow: true, active: true }) instead.

@carlosjeurissen
Copy link
Contributor

Tested behaviour in Safari Version 16.6 (18615.3.12.11.2):
The extension popup closes when:

  • one opens a new tab with cmd + t
  • one changes the active tab to another tab in the same window with the tabs api
  • one creates a new tab without active: false with the tabs api
  • one closes the active tab with the tabs api

In the scenario there are two tabs:

  1. some extension page, inactive
  2. another tab being active

When calling browser.tabs.getCurrent() in the extension page (tab 1), it will return the extension tab. Which is not the active tab in the same window.

@tophf
Copy link

tophf commented Sep 13, 2023

You will be introducing an inconsistency via this change, which deteriorates the platform, so it's best to add a new method like chrome.tabs.getActiveTab that will also benefit everyone in every extension context by removing the need to copypaste the same unnecessarily verbose boilerplate code browser.tabs.query({ currentWindow: true, active: true }).

@xeenon
Copy link
Collaborator

xeenon commented Sep 14, 2023

You will be introducing an inconsistency via this change, which deteriorates the platform, so it's best to add a new method like chrome.tabs.getActiveTab that will also benefit everyone in every extension context by removing the need to copypaste the same unnecessarily verbose boilerplate code browser.tabs.query({ currentWindow: true, active: true }).

chrome.tabs.getSelected was a thing, that basically did tabs.query({currentWindow: true, active: true}), but it was removed in v3.

@xeenon
Copy link
Collaborator

xeenon commented Sep 14, 2023

Tested behaviour in Safari Version 16.6 (18615.3.12.11.2): The extension popup closes when:

@carlosjeurissen I'm not sure I follow how this relates to this issue.

@carlosjeurissen carlosjeurissen added the implemented: safari Implemented in Safari label Sep 15, 2023
@carlosjeurissen
Copy link
Contributor

@xeenon it was relevant in the general discussion on what naming should be used and I was asked to add it to the issue.

@tophf What is the inconsistency introduced here? tabs.getCurrent() would not necessarily return the active tab in the current window. There are cases in which the tab returned by tabs.query({ currentWindow: true, active: true }) is not the same as the tab the extension was opened on. So it should not be seen as an equivalent.

@tophf
Copy link

tophf commented Sep 15, 2023

By definition getCurrent returns the tab that owns or contains this context, but the popup is not really owned by or contained inside the tab, it's just loosely associtated with a tab due to a design decision and it may even change some day as there is no physical barrier to prevent that. The original name is unfortunately based on a very misleading concept of "current", it was already vague, and now if you blur it even more it'll cease to be a meaningful concept altogether and people will start to wonder why calling getCurrent in an inactive tab returns that very tab and not the "current" one. See? It was always a bad name.

P.S. Looking at the amount of questions and misunderstanding around this method on stackoverflow over the past decade, I suggest we just deprecate it and introduce new methods like getOwnTab, getFocusedTab (names TBD) that will be self-explanatory or at least not as inherently deceptive as getCurrent.

@carlosjeurissen
Copy link
Contributor

carlosjeurissen commented Sep 15, 2023

@tophf That's a very good point. The reasoning behind using browser.tabs.getCurrent was the fact it is already implemented in Safari. There can also cases in which the extension popup is a tab. Which would make it impossible to even use tabs.query.

Taking that in account. something like browser.tabs.getAssociatedTab() would very much be preferred to get the tab the extension popup was opened on / associated with / acts upon.

@carlosjeurissen carlosjeurissen removed the implemented: safari Implemented in Safari label Sep 15, 2023
@tophf
Copy link

tophf commented Sep 15, 2023

Yeah, although I'd prefer chrome.action.tabId number just like chrome.devtools.inspectedWindow.tabId, if it can be implemented.

@carlosjeurissen
Copy link
Contributor

carlosjeurissen commented Sep 15, 2023

@tophf In that case something like browser.action.associatedWindow.tabId would be preferred. So it can have more than just the tabId. Potentially we can even make a unified inspectedWindow. Something like browser.tabs.associatedWindow and let it have the same interface as browser.devtools.inspectedWindow.

Thoughts from the browser vendors?

@hanguokai
Copy link
Member

I agree with the following:

  • tabs.getCurrent() return undefined, because the popup is not a tab.
  • use action namespace for popup

For running executeScript() in the popup, obtaining tabId is sufficient. (Window info can be obtained via windows.getCurrent()).

@carlosjeurissen
Copy link
Contributor

@hanguokai Because of the issues described in previous answers browser.windows.getCurrent() would not always give the same info. You would actually need to query browser.tabs.get(tabId) to get the data you need.

@hanguokai
Copy link
Member

@carlosjeurissen I'm not sure what use case you're talking about. executeScript() doesn't need Window info, so I don't understand why need action.associatedWindow instead of a Tab or tabId directly.

@carlosjeurissen
Copy link
Contributor

carlosjeurissen commented Sep 17, 2023

@hanguokai in this context, associatedWindow refers to the tab window. Not the browser window.

To clarify what we are actually talking about, something like associatedContext makes the most sense to me. In general using associatedContext makes more sense for devTools as well since it also applies to serviceWorkers which are not windows.

@carlosjeurissen
Copy link
Contributor

@xeenon Do you know of real-world extension cases in which runtime.getCurrent is used for this purpose?

In general I still consider it a bug and it prevents extension developers from being able to easily detect if they are running in a tab-like environment or in an extension popup. To overcome this one can attempt to exclude the current URL to make getCurrent more reliable as such:

getCurrentTabWithTabsApi().then((tabInfo) => {
  if (!isSafari || !tabInfo) return tabInfo;

  // attempt to exclude associated tab
  if (tabInfo.url === window.location.href) return tabInfo;
});

@oliverdunk seems tabs.getCurrent does not return a tab in the extension-page embedded options page. While in Firefox it does return the tab. Is this intended?

@tophf
Copy link

tophf commented Apr 10, 2024

Is this intended?

Probably caused by a security check that disallows any processing of a top chrome:// URL, but I'd argue this is a bug in this case because the returned tab object doesn't expose anything that can't be achieved via chrome.tabs.query.

@xeenon
Copy link
Collaborator

xeenon commented Apr 11, 2024

@carlosjeurissen Yes, the Userscripts extension uses tabs.getCurrent() in the background page. This is something I wanted to change/fix in WebKit’s implementation, but it ended up breaking that extension.

@carlosjeurissen
Copy link
Contributor

@xeenon Thanks for clarifying! That makes a lot of sense! That ship has sailed then.

As for finding out the associated tab id. What are your thoughts? Something like browser.action.associatedTabId? Introducing this would also allow extensions to find out if what is returned by tabs.getCurrent() is the associatedTab or it's own tab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inconsistency Inconsistent behavior across browsers proposal Proposal for a change or new feature supportive: chrome Supportive from Chrome supportive: firefox Supportive from Firefox
Projects
None yet
Development

No branches or pull requests

9 participants