942511 - All properties of the ActiveTab are undefined

Status: RESOLVED INCOMPLETE

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

Description

[shinichi.katsumata]

Attachment: bug.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release)
Build ID: 20131112160018

Steps to reproduce:

This sample script is show activeTab.id to console when context menu is shown.
 
STEP1.cfx run  ->  show addon sdk dev guide.
STEP2.Right click on above page.   -> shown activeTab.id correctry
STEP3.Click on the upper left of "OPEN NEW WINDOW".
STEP4.Right click on STEP2 page.   -> shown activeTab.id correctry
STEP5.Close STEP3 popup page.
*STEP6.Right click on STEP2 page.   -> shown activeTab.id undefined.*

NOTE:After Active tab change, shown correctry activeTab.id. 




===============
lib/main.js
===============
var tabs = require("sdk/tabs");
var self = require("sdk/self");
var cm = require('sdk/context-menu');

var attached = [];

tabs.on('ready', function (tab) {
  if( attached.indexOf(tab.id) === -1 ) {
    console.log("tab attached.");
    tab.attach({
      contentScriptFile: self.data.url("content.js"),
    });
    attached.push(tab.id);
  }
});


cm.Item({
    label: "post message",
    context: cm.PageContext(),
    contentScript: "self.on('context',function(){self.postMessage();});",
    onMessage: function () {
        console.log( "activeTab.id =>" + tabs.activeTab.id );
    }
});

tabs.open("https://addons.mozilla.org/en-US/developers/docs/sdk/latest/dev-guide/index.html");


===============
data/contentjs
===============
var elem = document.createElement("div");

elem.innerHTML = "<h1>OPEN NEW WINDOW</h1>";

elem.addEventListener("click",function(){
  var props = ['width=100','height=100'].join(',');
  window.open("http://www.google.com/",'',props);
});

document.body.insertBefore(elem,document.body.firstChild);

===============
environment
===============
- Addon SDK 1.15beta1
- Firefox 25.0.1
- Python 2.6.5
- Windows 7 64bit



Actual results:

ActiveTab.id (and Other Properties) is undefined.

NOTE:
  This problem happen only show popup window.   

  eg. window.open(url); -> no problem.
        window.open(url,'','width=100,height=100');  -> problem.
  
NOTE
  This problem may same problem as follow. ???

 Bug 653917 - ...tabs.activeTab.contentWindow is undefined 
 https://bugzilla.mozilla.org/show_bug.cgi?id=653917



Expected results:

I expect show current activeTab.id correctry everytime.

I like firefox addon sdk. 

thanks.

Comment 1

[Dave]

I can confirm that this issue still occurs in Firefox 30. Here are the steps I followed to recreate it (step #6 shows the undefined properties):

1. Start Firefox w/ an empty tab. 

2. Go to firefox.com

3. Execute this code in the global page:

	console.log("tabs.activeTab = <" + tabs.activeTab + ">");
	console.log("tabs.activeTab.id = <" + tabs.activeTab.id + ">");
	console.log("tabs.activeTab.url = <" + tabs.activeTab.url + ">");

Results for the firefox tab are as follows:

	console.log: onepassword: tabs.activeTab = <[object Tab]>
	console.log: onepassword: tabs.activeTab.id = <-3-1>
	console.log: onepassword: tabs.activeTab.url = <https://www.mozilla.org/en-US/firefox/new/?utm_source=firefox-com&utm_medium=referral>

4. Select Window > Web Console and execute this JavaScript: 

	window.open("http://digg.com");
	
NOTE: A new window MUST open for the bug to happen. The issue will NOT occur if your settings are configured to use a new tab. I.e. double check this setting:

	https://www.evernote.com/shard/s2/sh/e88f16cb-fb15-4274-81b4-31cb4d0ab2aa/c84c9e620d408ac0ebd912b677c55412/deep/0/Tabs.png

5. In the new window that opens, the logs from step #3 in the global page yield the correct results:

	console.log: onepassword: tabs.activeTab = <[object Tab]>
	console.log: onepassword: tabs.activeTab.id = <-57-1>
	console.log: onepassword: tabs.activeTab.url = <http://digg.com/>

6. Close the newly opened window. The original window should appear with the Firefox tab having focus. Executing the logs in the global page yield:

	console.log: onepassword: tabs.activeTab = <[object Tab]>
	console.log: onepassword: tabs.activeTab.id = <undefined>
	console.log: onepassword: tabs.activeTab.url = <undefined>

Interestingly enough the activeTab itself is not null, but all the attributes are undefined.

7. Open a new tab (mine is configured to be the empty tab). Logs now show valid active tab properties:

	console.log: onepassword: tabs.activeTab = <[object Tab]>
	console.log: onepassword: tabs.activeTab.id = <-3-3>
	console.log: onepassword: tabs.activeTab.url = <about:newtab>

8. Close the tab in step 6 so the Firefox home page has focus again. The logs in the global page will show the proper tab properties:

	console.log: onepassword: tabs.activeTab = <[object Tab]>
	console.log: onepassword: tabs.activeTab.id = <-3-1>
	console.log: onepassword: tabs.activeTab.url = <https://www.mozilla.org/en-US/firefox/new/?utm_source=firefox-com&utm_medium=referral>

Please let me know if there is anything else I can do to help.

Cheers!

Comment 2

[Tomislav Jovanovic :zombie]

(In reply to Dave from comment #1)
> 3. Execute this code in the global page:

what does this mean? what is "global page"? 

are you doing this on some event? if so, which one?

Comment 3

[Dave]

Thanks for following up on this, Tomislav. 

I'm sorry for the confusion over the term "global page". I meant the Add-on code. The "global page" term is from Safari (Chrome calls it the background page) and I have the bad habit of using the term "global page" for all long running code. 

In my example, the console.log-s are in Add-on code which is called from a WebSocket. In case it matters, our WebSocket is created within our hidden frame (https://developer.mozilla.org/en-US/Add-ons/SDK/Low-Level_APIs/frame_hidden-frame) and calls the Add-on code when triggered by our main application.

Your question sparked the idea to try other code paths so I added these same logs to our Toolbar Button and Context Menu item. Here's a bit of the setup code we use so we're on the same page:

>	exports.main = function(options, callbacks) {
>		httpRequestObserver.register();
>		var toolbarbutton = require("toolbarbutton");
>		var tbb = new toolbarbutton["ToolbarButton"]({
>			"id": "onepassword-toolbar-button",
>			"label": "1Password",
>			"image": data.url("images/toolbar-firefox.png"),
>			"onCommand": function(params) {
>				console.log("\n\n\nTOOLBAR BUTTON!!!!");
>				console.log("TBB: tabs.activeTab = <" + tabs.activeTab + ">");
>				console.log("TBB: tabs.activeTab.id = <" + tabs.activeTab.id + ">");
>				console.log("TBB: tabs.activeTab.url = <" + tabs.activeTab.url + ">");
>			}
>		});
>	...

The Toolbar Button code is using Eric Vold's toolbar button; we'll be embracing the new ui.ActionButton approach soon. And for the context menu we have this code:

>	contextMenu['Item']({
>		  'label': "1Password",
>		  'contentScript': 'self.on("click", function (node, data) {' +
>             				 '   self.postMessage({\'name\':\'click\', \'data\':node});' +
>						 '  });',
>			'onMessage': function (payload) {
>				var name = payload['name'];
>				if(name === "click") {
>					console.log("\n\n\nCONTEXT MENU!!!!");
>					console.log("CM: tabs.activeTab = <" + tabs.activeTab + ">");
>					console.log("CM: tabs.activeTab.id = <" + tabs.activeTab.id + ">");
>					console.log("CM: tabs.activeTab.url = <" + tabs.activeTab.url + ">");
>				}
>		  }
>		});
>		...


After following the steps in Comment 1 to recreate the issue, I invoked both the Context Menu and the Toolbar button and got these logs:

>	CONTEXT MENU!!!!
>	console.log: onepassword: CM: tabs.activeTab = <[object Tab]>
>	console.log: onepassword: CM: tabs.activeTab.id = <undefined>
>	console.log: onepassword: CM: tabs.activeTab.url = <undefined>
>	
>	
>	TOOLBAR BUTTON!!!!
>	console.log: onepassword: TBB: tabs.activeTab = <[object Tab]>
>	console.log: onepassword: TBB: tabs.activeTab.id = <undefined>
>	console.log: onepassword: TBB: tabs.activeTab.url = <undefined>

Both the id and url are undefined, just as they were when called from the WebSocket. So it appears the activeTab has undefined properties when called from all contexts. At least, all the contexts that I could think of :)

I hope this helps. Please let me know if there is anything further I can help with or if there's anything else I can try.

Cheers!

Comment 4

[Dave]

I ran several more tests today to see if I could find a workaround to this issue.

First, the logs from the above comments were using an extension that was still including the Addon SDK inside our packaged XPI. Thinking there may be a conflict between the packaged SDK code and the one shipped within Firefox, I removed the `cfx` flags that preserved the SDK files within the XPI. Sadly this did not help.

Second, I removed all the old toolbar icon code that Erik Vold provided years ago, along with all of its dependencies. I had hoped there may be a conflict with this old code, but the issue persisted. 

Next, I reran the test using beta version of Firefox (currently 31.0) and Firefox Aurora (currently 32.0a2 (2014-07-08)) and the results are identical.

I'm out of ideas on what else to try so I welcome any suggestions for things to try. Are there any ways I could work around this? I tried using XPCOM but the example code[1] seems to be out of date.

Thanks!

[1]: Chapter 4: Using XPCOM - https://developer.mozilla.org/en-US/Add-ons/Overlay_Extensions/Firefox_addons_developer_guide/Using_XPCOM%E2%80%94Implementing_advanced_processes#Get_active_window

Comment 5

[Jeff Griffiths (:canuckistani) (:⚡︎)]

removing priority so that this shows up in triage this week for discussion.

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

Hey Irakli, I'm not sure if you want to work on this, if not could you please unassign yourself.

Comment 7

[Gaetan]

Hi, I came across that bug while trying to attach a contentScriptFile to the ActiveTab just after having close a popup window. After a popup window has been closed, all the properties of the ActiveTab are undefined and a contentScriptFile can't be attached to the ActiveTab (It doesn't fire any error though).

I ran some tests and it appears that the tab in the orginal window is not activated again when the popup window is closed. This bug occurs when a popup window is opened and closed. When opening a new window (File - New Window), the bug does not occur.

Code to reproduce the bug:

===============
lib/main.js
===============
var tabs = require("sdk/tabs");
tabs.on('open', function onOpen(tab) {
	console.log('------------------------------');
	console.log('tab is loaded');
	console.log('tab title: ' + tab.title);
	console.log('tab ID: ' + tab.id);
});
tabs.on('close', function onClose(tab) {
	console.log('------------------------------');
	console.log('tab is close');
	console.log('tab title: ' + tab.title);
	console.log('tab ID: ' + tab.id);
});
tabs.on('activate', function onActivate(tab) {
	console.log('------------------------------');
	console.log('tab is activate');
	console.log('tab title: ' + tab.title);
	console.log('tab ID: ' + tab.id);
});
tabs.on('deactivate', function onDeactivate(tab) {
	console.log('------------------------------');
	console.log('tab is deactivate');
	console.log('tab title: ' + tab.title);
	console.log('tab ID: ' + tab.id);
});


Steps to reproduce the bug:

Step 1
----------
cfx run -> it opens Firefox with an empty tab (named New Tab)

When Firefox is focus you should see these logs:

    console.log: firefox-addons: tab is activate
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -3-1

Step 2
----------
Go to Preferences - Tabs and uncheck "Open new windows in a new tab instead"

Step 3
----------
Open the Web developer tools (CTRL+SHIFT+I)

Step 4
----------
In the javascript console type: window.open('http://www.google.com')
and press Enter

Step 5
----------
You will get noticed that Firefox prevented a popup window to open.
Click on Preferences on the right of the message and click on Open 'http://www.google.com'
The popup window should open.

Step 6
----------
Close the popup window. You should see these logs:

    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is loaded
    console.log: firefox-addons: tab title: Google
    console.log: firefox-addons: tab ID: -33-1
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is activate
    console.log: firefox-addons: tab title: Google
    console.log: firefox-addons: tab ID: -33-1
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is close
    console.log: firefox-addons: tab title: Google
    console.log: firefox-addons: tab ID: -33-1

At that point, when the popup window is closed, it appears that the original tab (the one with ID: -3-1) is not activated again. I guess that's at that point that the properties of ActiveTab are undefined.

Step 7
----------
Open a New Tab (CTRL+T). You should see these logs:

    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is loaded
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -3-2
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is deactivate
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -3-1
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is activate
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -3-2

Step 8
----------
Close the tab opened in step 7. You should see these logs:

    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is close
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -3-2
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is activate
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -3-1

You can see that the orginal tab is activated only now. The properties of ActiveTab are now defined.


Expected results:
When the popup window is closed is Step 6, the original tab (with the ID: -3-1) should be activated again to become the ActiveTab.

Note:
The bug only occurs when a popup window is open with window.open(). When opening a new window from File - New Window and closing it the original tab is activated again and becomes the ActiveTab.

When opening a new Window (File -> New Window)
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is loaded
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -54-1
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is activate
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -54-1
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is deactivate
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -3-1
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is activate
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -54-1

When that window is closed:
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is close
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -54-1
    console.log: firefox-addons: ------------------------------
    console.log: firefox-addons: tab is activate
    console.log: firefox-addons: tab title: New Tab
    console.log: firefox-addons: tab ID: -3-1

You can see that in the case of Opening a new window from (File -> New Window) and closing it, the original tab (with the ID: -3-1) is activated again and becomes the ActiveTab.


===============
My environment
===============
- Addon SDK 1.17
- Firefox 35.0.1
- Python 2.7.6
- Ubuntu 14.04 TLS 64-bit

Comment 8

[Gaetan]

It might be related to https://bugzilla.mozilla.org/show_bug.cgi?id=989288 and
                       https://bugzilla.mozilla.org/show_bug.cgi?id=1003988 and
                       https://bugzilla.mozilla.org/show_bug.cgi?id=1003988

The events on the popup window are only fired once that popup window is closed. As you can see in my previous comment in the Step 6; these logs appears only when the popup window is closed. Also when opening and closing tabs inside that popup window no events are fired. Something interesting you can see, is that the initial tab (ID: -3-1) is not "deactivated" when when the popup window is opened. The SDK thinks that the initial tab (ID -3-1) is still activated so that's maybe why it doesn't get activated again when that popup window is closed.

These bugs are really problematic in the sense that a lot a websites are using window.open() and when they do we can't get the ActiveTab anymore (unless the user switches to another tab and switches back to the initial one). It simply breaks the behavior of the addons.

Note: That behavior is the same with the current development version from Github and with Firefox Nightly 38.0a1 (2015-01-25)

Comment 9

[Gaetan]

Attachment: pull request

Comment 10

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 8560025 [details] [review]
pull request

Hey! thanks for the patch :)

I asked a couple of questions and asked for a few changes to the test in the github pull request.  If you need any help just ask!

Comment 11

[Erik Vold [:erikvold] (please needinfo? me)]

Hey Irakli, it looks like Gaetan wants to tackle this.

Comment 12

[Gaetan]

Hey Erik, thank you for the reviews on my pull request :)

I think the changes I did in the first commit was the best solution to fix the bug actually, so I think I will get back to that. But I would like to have some guidance before pushing new changes.

You asked me to use getTabId(getActiveTab()) instead of require("../tabs").activeTab.id. But I noticed that the behavior was different:

getTabId(getActiveTab(getFocusedBrowser())) returns the right tab ID even after a window popup was closed. So the bug doesn't occurs in that case. I used it as a work-around in the addon I'm working on actually. I looked more into the code and I found out that it does a call to getSelectedTab(window) in /lib/sdk/tabs/utils.js on line 276. It's returning gBrowser.selectedTab. So it's actually returning the current selected tab which is not the current activated tab in every case. For example after a window popup is closed. And a similar behavior is when Firefox just started, I noticed that the current selected tab is correct, but at that point the selected tab is not activated yet (so require("../tabs").activeTab.id will return undefined) The tab got activated only when clicking on the Firefox window (But maybe that's another issue? Or a normal behavior?).

So that's why I used require("../tabs").activeTab.id, because I found out that it's only when trying to get the active tab ID property that way that the bug occurs.

Your insight about that would be really appreciated :) I don't know much about the SDK and I don't know if the differences of behavior between these two methods was wanted or not. I just wanted to give my observations on that. Do you think what I did in the first commit was a good way to fix the bug? Or should we look to make the two methods returns the same thing?

Comment 13

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Gaetan from comment #12)
> Hey Erik, thank you for the reviews on my pull request :)
> 
> I think the changes I did in the first commit was the best solution to fix
> the bug actually, so I think I will get back to that. But I would like to
> have some guidance before pushing new changes.
> 
> You asked me to use getTabId(getActiveTab()) instead of
> require("../tabs").activeTab.id. But I noticed that the behavior was
> different:
> 
> getTabId(getActiveTab(getFocusedBrowser())) returns the right tab ID even
> after a window popup was closed. So the bug doesn't occurs in that case. I
> used it as a work-around in the addon I'm working on actually. I looked more
> into the code and I found out that it does a call to getSelectedTab(window)
> in /lib/sdk/tabs/utils.js on line 276. It's returning gBrowser.selectedTab.
> So it's actually returning the current selected tab which is not the current
> activated tab in every case. For example after a window popup is closed.

If you want to use the `getTabId(getActiveTab(getFocusedBrowser()))` just be sure to include tests that would fail if `getTabId(getActiveTab())` were used, to ensure we don't make that change by mistake in the future.  You seem to have identified a case where one has to be replaced by the other so we'd just need a test to encapsulate that logic.


There are many race conditions in the tab code on Firefox, and sometimes the event order is different of different platforms (osx vs windows for example).  It's something that we need to often workaround, then we can report bugs for the Firefox to get the platform fixes made that we need in order to eventually remove our workarounds.  For instance in this case we might want to file a FIrefox bug about the activeTab being incorrect in the two cases you've mentioned.

> a similar behavior is when Firefox just started, I noticed that the current
> selected tab is correct, but at that point the selected tab is not activated
> yet (so require("../tabs").activeTab.id will return undefined) The tab got
> activated only when clicking on the Firefox window (But maybe that's another
> issue? Or a normal behavior?).

That sounds like another bug, could you make a new bug report for that please?

Comment 14

[Gaetan]

(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #13)
> If you want to use the `getTabId(getActiveTab(getFocusedBrowser()))` just be
> sure to include tests that would fail if `getTabId(getActiveTab())` were
> used, to ensure we don't make that change by mistake in the future.  You
> seem to have identified a case where one has to be replaced by the other so
> we'd just need a test to encapsulate that logic.

Did you mean if I wanted to use require("../tabs").activeTab? Because that's only by using this, so the High-Level API, that the bug occurs. That's why I used it in my first commit, that's the only way to identify that the bug happens and so the only way, I think, to fix it. What I did in my first commit was checking if require("../tabs").activeTab.id was returning "undefined", and if that was the case I was sending an activate event to the selected tab.

> That sounds like another bug, could you make a new bug report for that
> please?

Sure, I will do it :)

Comment 15

[Gaetan]

I didn't include a test in the case where getTabId(getActiveTab(getFocusedBrowser())) where used instead of require("../tabs").activeTab because if it's used, the test I made will already fail.

(In reply to Gaetan from comment #14)
> > That sounds like another bug, could you make a new bug report for that
> > please?
> 
> Sure, I will do it :)

Actually I think I was mistaken, it's weird beacause I didn't find a way to reproduce that bug.

Comment 16

[Ben McCann]

This bug was fixed by https://github.com/mozilla/addon-sdk/commit/53d1725c7f31544ae5c700a30d26ff6be9559b4f. However, there is still a PR pending to add a test to ensure we don't regress https://github.com/mozilla/addon-sdk/pull/1856

Comment 17

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 8560025 [details] [review]
pull request

I'll try to review this sometime this week.

Comment 18

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/ff1c14e7c50555d3ef30787f02251855c376d121
Bug 942511 - Adding a test to ensure that this bug won't occur anymore.

https://github.com/mozilla/addon-sdk/commit/8c993580747af8e767dce0c1e9e29a04d2ba406f
Merge pull request #1856 from colbat/activetab-properties-undefined-942511

Bug 942511: Test undefined properties of ActiveTab. r=Mossop

Comment 19

[Gaetan]

The PR that contains the test was merged. And, as said before, this bug was fixed by https://github.com/mozilla/addon-sdk/commit/53d1725c7f31544ae5c700a30d26ff6be9559b4f. So I think we can update the status as RESOLVED and FIXED. (But looks like I can't mark it as FIXED)

Comment 20

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562