currentWindow does not match any tabs in query from sidebar

RESOLVED FIXED in Firefox 54

Status

defect
P2
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: andy+bugzilla, Assigned: mixedpuppy)

Tracking

unspecified
mozilla54
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

(Whiteboard: triaged)

Attachments

(2 attachments)

Posted file sidebar.zip
In a sidebar add-on I do tabs.query({currentWindow: true}), I notice that consistently the first tab is not included. Later tabs that are created are included.

Attached is a simple add-on that (I think) should detect 1 tabs in both cases. However it outputs when loaded in about:debugging as a temporary add-on:

Got 1 tabs from query.
Got 0 tabs from query with currentWindow.
Summary: tabs.query({currentWindow: true}) in a sidebar, returns 1 less window → currentWindow does not match any tabs in query from sidebar
Summary: currentWindow does not match any tabs in query from sidebar → currentWindow does not match first tab in query from sidebar
Ok, Kris, you were right :)
Summary: currentWindow does not match first tab in query from sidebar → currentWindow does not match any tabs in query from sidebar
This happens because the sidebar frame is contained in a xul frame [ie. chrome://browser/content/webext-panels.xul], thus context.currentWindow is the sidebar xul frame.  We need to grab the parent of that.
Attachment #8839277 - Flags: feedback?(kmaglione+bmo)
Assignee: nobody → mixedpuppy
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: triaged
Comment on attachment 8839277 [details]
Bug 1340739 sidebar window is not the browser window,

https://reviewboard.mozilla.org/r/113954/#review115672

::: toolkit/components/extensions/ExtensionParent.jsm:369
(Diff revision 1)
> +    if (this.viewType == "sidebar") {
> +      return this.xulBrowser.ownerGlobal.document.parentWindow;
> +    }
>      return this.xulBrowser.ownerGlobal;

Let's just always return the root window:

    win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDocShell);
       .QueryInterface(nsIDocShellTreeItem).rootTreeItem
       .QueryInterface(nsIInterfaceRequestor).getInterface(nsIDOMWindow);
Attachment #8839277 - Flags: feedback?(kmaglione+bmo)
Comment on attachment 8839277 [details]
Bug 1340739 sidebar window is not the browser window,

https://reviewboard.mozilla.org/r/113954/#review115718

::: browser/components/extensions/test/browser/browser_ext_sidebarAction.js:155
(Diff revision 2)
> +      `,
> +      "sidebar.js": function() {
> +        Promise.all([
> +          browser.tabs.query({}).then((tabs) => {
> +            browser.test.assertEq(1, tabs.length, "got tab without currentWindow");
> +            return true;

No need for either `return true`.
Attachment #8839277 - Flags: review?(kmaglione+bmo) → review+
Depends on: 1340750
Pushed by mixedpuppy@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3ad2f410d2e7
sidebar window is not the browser window, r=kmag
https://hg.mozilla.org/mozilla-central/rev/3ad2f410d2e7
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.