646740 - worker.tab is incorrect when the given tab is in the background

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Bobby Holley (:bholley)]

The implementation of worker.tab contains a rather bizarre incantation:

>    topContentWindow= win.QueryInterface(Ci.nsIInterfaceRequestor)
>                            .getInterface(Ci.nsIWebNavigation)
>                            .QueryInterface(Ci.nsIDocShellTreeItem)
>                            .treeOwner
>                            .QueryInterface(Ci.nsIInterfaceRequestor)
>                            .getInterface(Ci.nsIDOMWindow);

This QIs its way up to XUL and back down again, always landing us in the active tab. This means that worker.tab is equivalent to require("tabs").activeTab, which is much less useful than the advertised behavior.

Comment 1

[Bobby Holley (:bholley)]

Attachment: patch+tests v1

Attached a patch and tests to fix the issue.

Comment 2

[Alexandre Poirot [:ochameau]]

Nice catch!

Unfortunately I think that win.top is going to throw domain access denied exception on iframe with different domain that upper document.
I'm going to take a look at this. Thanks for the patch and this unit test is very usefull.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 523210 [details] [diff] [review]
patch+tests v1

Nice catch! r=myk

Comment 4

[Myk Melez [:myk] [@mykmelez]]

https://github.com/mozilla/addon-sdk/commit/ee2e610a105c6a5bc7bda7f322064798d37016bc

Comment 5

[Bobby Holley (:bholley)]

(In reply to comment #2)
> Nice catch!
> 
> Unfortunately I think that win.top is going to throw domain access denied
> exception on iframe with different domain that upper document.

I didn't test it, but I'd assumed this wouldn't happen because the script is running with chrome context, not the context of the upper document.

Given that this was pushed, I assume this concern was resolved? I just wanted to make sure.

Comment 6

[Alexandre Poirot [:ochameau]]

I verified with differents domains and win.top still works.
So we can consider this as fixed.