Implement browser.tabs opener functionality

NEW
Assigned to

Status

()

Toolkit
WebExtensions: Frontend
P2
normal
a year ago
17 days ago

People

(Reporter: kmag, Assigned: kmag)

Tracking

unspecified
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tabs] triaged)

MozReview Requests

()

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

Attachments

(1 attachment)

This includes the `openerTabId` properties of the `tabs.create` and `tabs.update` methods, and of Tab objects.
Blocks: 1213477
Whiteboard: [tabs]

Updated

a year ago
Flags: blocking-webextensions+
Priority: -- → P2
Whiteboard: [tabs] → [tabs] triaged
Assignee: nobody → kmaglione+bmo
Created attachment 8722260 [details]
MozReview Request: Bug 1238314: [webext] Implement tabs API `openerTabId` properties. r?billm

Review commit: https://reviewboard.mozilla.org/r/35935/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35935/
Attachment #8722260 - Flags: review?(wmccloskey)
Comment on attachment 8722260 [details]
MozReview Request: Bug 1238314: [webext] Implement tabs API `openerTabId` properties. r?billm

https://reviewboard.mozilla.org/r/35935/#review33025

::: browser/components/extensions/ext-utils.js:589
(Diff revision 1)
> +    let opener = tab.owner || this._tabOpeners.get(tab);

I don't think the owner property is a reliable way to do this. See here for example:
http://searchfox.org/mozilla-central/source/browser/base/content/tabbrowser.xml#1824

I'm also unsure if the owner is even set if a page does window.open. I don't think it is.

This bug could require some platform support. needinfo me if you need help with the window opening APIs. e10s is the hard case. Here's where the logic is:
http://searchfox.org/mozilla-central/source/dom/ipc/ContentParent.cpp#5507
Attachment #8722260 - Flags: review?(wmccloskey)
https://reviewboard.mozilla.org/r/35935/#review33025

> I don't think the owner property is a reliable way to do this. See here for example:
> http://searchfox.org/mozilla-central/source/browser/base/content/tabbrowser.xml#1824
> 
> I'm also unsure if the owner is even set if a page does window.open. I don't think it is.
> 
> This bug could require some platform support. needinfo me if you need help with the window opening APIs. e10s is the hard case. Here's where the logic is:
> http://searchfox.org/mozilla-central/source/dom/ipc/ContentParent.cpp#5507

Aside from it being cleared after an unrelated tab is opened, I think `owner` serves exactly the same purpose as `opener` does in Chrome.

As for `window.open`, it sets `owner` to whatever the currently-selected tab is, whether or not it's the actual opener. That should probably be fixed, but I think it's a separate bug.
I just don't think the owner property of <tab>s was designed for this purpose. It's a property that serves a very specific purpose in the Firefox UI. Here's another issue: switching tabs nulls out the owner:
http://searchfox.org/mozilla-central/source/browser/base/content/tabbrowser.xml#1041
(I think the selected condition is only true if we're switching to the same tab as the current one.)

In addition, if a window.opened tab is opened in the background (controlled by a pref), then the owner will always be null.
You have some good points, but I think that either way we need to map this to the owner at some point.

When opening a tab, especially when it's related to the current tab, setting the owner to the opener gets us the expected tab placement behavior, which is the thing that I'm most concerned about right now.

And when the owner is set when a tab is first opened, it's always set to what we'd consider the owner in the Chrome API (ignoring the bugs relating to background tabs).

I suppose we probably shouldn't be setting the owner property on `tabs.update`, since that doesn't result in any useful documented behavior, from the add-ons perspective.

I think the best solution is probably to keep using aOwner, but try to make the behavior more consistent, and probably store it in a separate 'opener' property even in cases where we wouldn't set .owner.
Iteration: --- → 48.1 - Mar 21
Duplicate of this bug: 1258823
Duplicate of this bug: 1276346
Duplicate of this bug: 1293727

Comment 9

9 months ago
Any progress? This feature is quite required for addons which open new tabs related to the current page. Currently there is no way to open new tab related to the current tab in some cases.

For example: a context menu command to open tabs from selection.
https://addons.mozilla.org/firefox/addon/text-link/
Currently there is only one way to open a related tab: window.open() in a content script. But it is blocked for context menu commands, because extra context menu items must be defined in a background script. (To open a new tab from context menu, chrome.tabs.create() is not useful because the new tab is always opened at the end of the tab bar, even if it is quite related to the current tab. Thus I thought to use window.open() instead, and I sent a message from the background script to a content script via chrome.tabs.sendMessaeg(). Then window.open() in the script is blocked because it is not triggered by any user action in the content area.)

Updated

8 months ago
Component: WebExtensions: Untriaged → WebExtensions: Frontend
Flags: blocking-webextensions+
Duplicate of this bug: 1311504
Blocks: 1310331
No longer blocks: 1310331

Comment 11

7 months ago
After reading the whole bug I still don't get it. What is preventing this from being made? Is it the difficulty of finding what the "current tab" is before opening the new tab?

Updated

5 months ago
webextensions: --- → ?
Duplicate of this bug: 1325818

Comment 13

5 months ago
Any plans on fixing this? As in WebExtensions documentation it says openerTabId property is supported in Firefox?

Comment 14

5 months ago
Just voted for this bug too, since it's one thing we're missing for API parity when porting a Chrome extension to WebExt.

(Additionally and quite surprisingly, instead of `openerTabId` being simply ignored and triggering a warning while it's unsupported, it instead throws "Type error for parameter createProperties" and prevents the tab from opening altogether, forcing the use of UA sniffing where I'd expect graceful degradation.)

Comment 15

2 months ago
Kris, can we polish this patch off and land?
webextensions: ? → +
Flags: needinfo?(kmaglione+bmo)
With something similar to the implementation in the attached patch, yes. Making the changes to tie it to window.opener would take me a good week and require some deep platform changes.
Flags: needinfo?(kmaglione+bmo)
Does Chrome allow for opener removal?
It looks like the attached patch only allows changing the tab and not removing it entirely.
Also worth noting in Chrome:
- openerTabId isn't present for window.open popups
- Running update {openerTabId: x} returns in the value of tab.create()
  - However it doesn't change the value of window.opener in the actual tab?
I've been trying out Tree Tabs (https://addons.mozilla.org/en-US/firefox/addon/tree-tabs/) and it currently can't distinguish between a tab opened via middle-clicking a link (which I want to open as a child tab) from a tab opened via Ctrl-T or a bookmark (which I want to open as a top-level tab at the bottom of the tab list). I was told by the Tree Tabs author that this was due to the following.

> There is no way to distinguish where tab came from in current API. In 
> Chrome/Opera/Vivaldi tab opened from middle click gives openerId, Firefox
> does not.

Is this functionality covered by this bug? I'm not sure. If it's different, let me know and I'm happy to file a separate bug.
Duplicate of this bug: 1360958

Comment 21

19 days ago
Any news on this? Without it new tabs in my extension open at the end of the list as Nicholas pointed out. I have to make all tabs open as children which is just bad.
You need to log in before you can comment on or make changes to this bug.