Closed
Bug 842148
Opened 11 years ago
Closed 9 years ago
tab.close creates ghost tab
Categories
(Add-on SDK Graveyard :: General, defect, P3)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: zepeuj, Assigned: evold)
References
Details
Attachments
(1 file)
I've found something strange with my add-on: When I close a tab "not ready", the tab is well closed but after this, when I list the tabs using 'browserWindows.activeWindow.tabs', the tab still exist with title 'undefined' and url 'undefined'. If I list the tabs using 'var tabs = require('sdk/tabs');' the closed tab doesn't appear. It only happens using 'var activeWindowTabs = require("sdk/windows").browserWindows.activeWindow.tabs;' I use the SDK 1.13.2. Here's a sample code to reproduce the problem: function addTabListener(){ var tabs = require('sdk/tabs'); tabs.on('open', function(tab) { closeTabOpened(tab); }); } function closeTabOpened(newtab) { var activeWindowTabs = require("sdk/windows").browserWindows.activeWindow.tabs; for each (var tab in activeWindowTabs) { try{ if (tab.index == newtab.index) continue; } catch (e){ //tab not ready continue; } if (tab.url == newtab.title){ newtab.close(); return; } } } function listAllTabs(kill) { var count = 0; var urlMap = {}; var activeWindowTabs = require("sdk/windows").browserWindows.activeWindow.tabs; var tabs = require('sdk/tabs'); console.log(activeWindowTabs.length); console.log(tabs .length); for each (var tab in tabs) { console.log('tab found', tab.title, tab.url); } for each (var tab in activeWindowTabs) { console.log('tab found', tab.title, tab.url); } } Thanks
Assignee | ||
Comment 1•11 years ago
|
||
Hmm I can't reproduce this.. do you have a sample add-on that I can try?
Assignee | ||
Comment 2•11 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 714946 [details] Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/796 I wrote some tests for this bug since I didn't see any, but I can't reproduce it..
Attachment #714946 -
Flags: review?(poirot.alex)
Assignee | ||
Comment 4•11 years ago
|
||
cherry-picked the tests on to the 1.13.2 tag and they all pass.
I'll create an add-on tomorrow for your tests and send you the link. The more important for the test, it's to close the tab when it is 'opened' before it is 'ready'.
I've created an add-on 'Test close tabs' accessible at https://builder.addons.mozilla.org/package/174373/latest/ This add-on close the new opened tab and list all existing tabs in console. Steps to reproduce the problem: 1 - open a new tab => the tab is closed and no ghost tab is listed in the console. 2 - again open a new tab => the tab is closed and this time a ghost tab is listed in the console.
Assignee | ||
Comment 8•11 years ago
|
||
(In reply to zepeuj from comment #6) > I've created an add-on 'Test close tabs' accessible at > https://builder.addons.mozilla.org/package/174373/latest/ > > This add-on close the new opened tab and list all existing tabs in console. > Steps to reproduce the problem: > 1 - open a new tab => the tab is closed and no ghost tab is listed in the > console. > 2 - again open a new tab => the tab is closed and this time a ghost tab is > listed in the console. ok I can reproduce with your example add-on now, thanks for this!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•11 years ago
|
Status: NEW → UNCONFIRMED
Ever confirmed: false
OS: Windows 7 → All
Hardware: x86_64 → All
Why the status has been changed to unconfirmed? Something wrong in the way to do?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Assignee | ||
Comment 10•11 years ago
|
||
(In reply to zepeuj from comment #9) > Why the status has been changed to unconfirmed? Something wrong in the way > to do? oops my mistake, I think I was trying to confirm it, noticed it was already and changed the state by mistake, sorry!
Assignee | ||
Comment 11•11 years ago
|
||
So this isn't a regression, but it is an error that is more obvious now because of the patch that landed in bug 833783. https://github.com/mozilla/addon-sdk/commit/b06eb5ba5d95fa7c2ccfb916ed7a4bc388579a9a It looks like this example code. as far back as 1.10 at least, would report an ever increasing value for `console.log('nb tabs with activeWindowTabs :', nbActiveWindowTabs);` and cause an error if one tried to access an properties on on the tab. Traceback (most recent call last): File "chrome://browser/content/browser.xul", line 1, in oncommand <?xml version="1.0"?> File "chrome://browser/content/browser.js", line 9161, in BrowserOpenTab openUILinkIn(BROWSER_NEW_TAB_URL, "tab"); File "chrome://browser/content/utilityOverlay.js", line 206, in openUILinkIn openLinkIn(url, where, params); File "chrome://browser/content/utilityOverlay.js", line 318, in openLinkIn isUTF8: aIsUTF8}); File "chrome://browser/content/tabbrowser.xml", line 1130, in loadOneTab isUTF8: aIsUTF8}); File "chrome://browser/content/tabbrowser.xml", line 1374, in addTab t.dispatchEvent(evt); File "resource://jid1-rysibkibbbgqbg-at-jetpack/api-utils/lib/tabs/observer.js", line 50, in handleEvent this._emit(EVENTS[event.type], event.target, event); File "resource://jid1-rysibkibbbgqbg-at-jetpack/api-utils/lib/events.js", line 123, in _emit return this._emitOnObject.apply(this, args); File "resource://jid1-rysibkibbbgqbg-at-jetpack/api-utils/lib/events.js", line 153, in _emitOnObject listener.apply(targetObj, params); File "resource://jid1-rysibkibbbgqbg-at-jetpack/api-utils/lib/windows/tabs.js", line 101, in _onTabEvent this._emitEvent(type, wrappedTab); File "resource://jid1-rysibkibbbgqbg-at-jetpack/api-utils/lib/windows/tabs.js", line 106, in _emitEvent tabs._emit(type, tab); File "resource://jid1-rysibkibbbgqbg-at-jetpack/api-utils/lib/events.js", line 123, in _emit return this._emitOnObject.apply(this, args); File "resource://jid1-rysibkibbbgqbg-at-jetpack/api-utils/lib/events.js", line 153, in _emitOnObject listener.apply(targetObj, params); File "resource://jid1-rysibkibbbgqbg-at-jetpack/test-addon/lib/main.js", line 18, in closeTabOpened(tab); File "resource://jid1-rysibkibbbgqbg-at-jetpack/test-addon/lib/main.js", line 26, in closeTabOpened listAllTabs(); File "resource://jid1-rysibkibbbgqbg-at-jetpack/test-addon/lib/main.js", line 50, in listAllTabs console.log('activeWindowTabs tab title:', tab.title); File "resource://jid1-rysibkibbbgqbg-at-jetpack/api-utils/lib/tabs/tab.js", line 113, in TabTrait<.title get title() getTabTitle(this._tab), File "resource://jid1-rysibkibbbgqbg-at-jetpack/api-utils/lib/tabs/utils.js", line 75, in getTabTitle return getBrowserForTab(tab).contentDocument.title || tab.label; TypeError: getBrowserForTab(...) is null
Assignee | ||
Comment 12•11 years ago
|
||
https://github.com/mozilla/addon-sdk/pull/796 This pull request has a test that reproduces the problem (ie the test fails), I don't have a fix for this right now and given it has been a long standing issue and that we plan to rewrite the tabs module for Firefox in order to remove Traits, I'm not sure if we should dig in to this other than to land the test with bug 724632.
Depends on: sdk-kill-traits
Assignee | ||
Comment 13•11 years ago
|
||
Dave you should probably take another look at this and see how important you think it is.
Flags: needinfo?(dtownsend+bugmail)
Comment 14•11 years ago
|
||
We'll look at this again in the next triage
Flags: needinfo?(dtownsend+bugmail)
Priority: P2 → --
Priority: -- → P3
Comment 15•11 years ago
|
||
Comment on attachment 714946 [details] Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/796 Reseting review request, as it doesn't make sense to land broken tests ;) Otherwise, this bug may be fixed with this: https://github.com/mozilla/addon-sdk/pull/838 Bad stuff happened when doing tab related action during "open" tabs event.
Attachment #714946 -
Flags: review?(poirot.alex)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → evold
Assignee | ||
Comment 16•11 years ago
|
||
darn this isn't fixed still..
Reporter | ||
Comment 17•11 years ago
|
||
Hi, Just to explain why I've used the "open" tab event instead of the "ready" tab event: My goal is to close a "duplicated" tab comparing the URL but I don't want/need to wait for the new tab to be fully loaded as I may want to close it. So once the tab is opened I check the tab title (which may be set with the URL) or wait the URL is not "about:blank" or "about:newtab" then if the new tab URL (or tab title) matches an existing tab URL, I close the new tab. For the moment I use the "ready" tab event but it is not efficient as I need to wait for the tab to be fully loaded before to close it. Thanks.
Reporter | ||
Comment 18•11 years ago
|
||
To complete my previous comment, if you have a better alternative, I'll be glad to know. :) Thanks
Assignee | ||
Updated•11 years ago
|
Assignee: evold → nobody
Assignee | ||
Updated•10 years ago
|
Attachment #714946 -
Flags: review?(rFobic)
Comment 19•10 years ago
|
||
Comment on attachment 714946 [details] Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/796 Sure we need this tests, but we should not land them without fix. I'm assigning this to myself and will include or do right after: https://github.com/mozilla/addon-sdk/pull/1207
Attachment #714946 -
Flags: review?(rFobic) → review+
Updated•10 years ago
|
Assignee: nobody → rFobic
Assignee | ||
Comment 20•9 years ago
|
||
Hey Irakli, are you working on this? if you could you please unassign yourself.
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(rFobic)
Comment 21•9 years ago
|
||
Since he is not responding, I'm resetting the assignee. If you are indeed still working on this :irakli, please re-assign yourself.
Assignee: rFobic → nobody
Comment 22•9 years ago
|
||
It was on my list along with other 100 bugs so probably unassigning was a good move.
Flags: needinfo?(rFobic)
Comment 23•9 years ago
|
||
Commit pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/d87a3e67fecf8fb90453ff54ca7963a931c68e0f Bug 842148: adding test for reported issue of closed tab existing in tab iterators r=gozala From https://github.com/mozilla/addon-sdk/pull/796
Assignee | ||
Comment 24•9 years ago
|
||
This was fixed in bug 854982, so I've now landed the test that I wrote for this bug.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → evold
You need to log in
before you can comment on or make changes to this bug.
Description
•