chrome.tabs.onRemove does not remove tab before calling the callback function

RESOLVED FIXED in mozilla51

Status

P2
normal
RESOLVED FIXED
2 years ago
8 hours ago

People

(Reporter: u576804, Assigned: fiveNinePlusR, Mentored)

Tracking

49 Branch
mozilla51

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: tabs, triaged)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160727004019

Steps to reproduce:

subscribe to the tabs.onRemoved event and supplied a callback function that queries the chrome.tabs.query() method to get a list of all tabs.


Actual results:

The tabs included the removed tab.


Expected results:

I expect the tab to not be in the list of tabs at this point in the tab's lifecycle.
(Reporter)

Updated

2 years ago
Component: Untriaged → WebExtensions: General
Product: Firefox → Toolkit
I guess we should probably just filter out tabs where `tab.closing` is true.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 2

2 years ago
Ok, I think I can tackle this one if you want to assign it to me.
Sure. Thanks!
Assignee: nobody → fiveNinePlusR
Mentor: kmaglione+bmo
Status: NEW → ASSIGNED

Updated

2 years ago
Assignee: u576804 → fiveNinePlusR

Updated

2 years ago
Priority: -- → P2
Whiteboard: tabs, triaged
Duplicate of this bug: 1293485
(Assignee)

Comment 5

2 years ago
Created attachment 8779574 [details] [diff] [review]
bug1291830_patch_1.diff
Attachment #8779574 - Flags: review?(kmaglione+bmo)
Comment on attachment 8779574 [details] [diff] [review]
bug1291830_patch_1.diff

Review of attachment 8779574 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the unused code removed.

::: browser/components/extensions/test/browser/browser_ext_tabs_events.js
@@ +263,5 @@
> +    });
> +
> +    let url = "http://example.com/browser/browser/components/extensions/test/browser/context.html";
> +    Promise.all([browser.windows.getCurrent()]
> +    ).then(windows => {

No need for `Promise.all`. In fact, no need for `getCurrent()` either, since you don't actually use the result.
Attachment #8779574 - Flags: review?(kmaglione+bmo) → review+
(Assignee)

Comment 7

2 years ago
Created attachment 8779896 [details] [diff] [review]
bug1291830_patch_1.diff

Fixes the coding style issue raised in review.
Attachment #8779574 - Attachment is obsolete: true
Attachment #8779896 - Flags: review?(kmaglione+bmo)
Attachment #8779896 - Flags: review?(kmaglione+bmo) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 9

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32eb078cd5dd
Do not return tabs that are in the process of closing from a `chrome.tabs.query` call. r=kmag
Keywords: checkin-needed

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/32eb078cd5dd
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Duplicate of this bug: 1301089

Comment 12

2 years ago
Thanks! I've added this contribution here: https://wiki.mozilla.org/Add-ons/Contribute/Recognition#September_2016

Comment 13

a year ago
This bug still occurs. I tested on Firefox 54 and Nightly 56.

Comment 14

a year ago
I can confirm this bug is not resolved. There is also another problem which might be a whole new bug, but not sure, I try to describe:

Have some code that subscribes to tabs.onCreated and tabs.onRemoved. Now, close the last tab with index i and very quickly open a new one (Ctrl+W, Ctrl+T). The tabInfo.index in the onCreated callback has the value i+1, whereas it must be i.

I think that's the reason for a bug in the add-on tabcenter-redux: https://github.com/eoger/tabcenter-redux/issues/124
(Assignee)

Comment 15

a year ago
In general, I think the way to report this is in a new bug and to reference this old bug in there for additional info. It helps the developers track their time more effectively and cuts out the prior discussion that is usually not relevant.

Comment 16

a year ago
Hello,

The problem is not fixed at all.

It appears in versions 56, 57 and in the Nightly...

It's really annoying when you need to update the tabs after one has been closed.

Is there someone working on this issue? Can we change the irritating status "RESOLVED FIXED".

Thanks

Comment 17

a year ago
As simple sample if needed:

const onRemovedTab = (removedTabId, removeInfo) => {

	console.log("Tab removed: " + removedTabId);

	const queryOpenedTabs = windowId => {

		chrome.tabs.query({ windowType: "normal", windowId: windowId }, openedTabs => {

			let tabIndex = null;

			for (const openedTab of openedTabs) {
				if (openedTab.id === removedTabId) {
					console.error("removed tab still there!!");
					tabIndex = openedTab.index;
					break;
				}
			}

			//Removed tab is still there, check in session with index
			if (tabIndex) {
				const gettingSessions = browser.sessions.getRecentlyClosed({ maxResults: 1 });
				gettingSessions.then(session => {
					if (session[0].tab) {
						if (session[0].tab.index === tabIndex) {
							console.log("As expected, removed tab is in the object session.");
						}
					}

				});
			}
			else {
				console.log("removed tab is gone.");
			}

		});
	};

	// the setTimeout is used for test to increase the timeout and see how many time to wait...
	setTimeout(() => queryOpenedTabs(removeInfo.windowId), 50);
};

chrome.tabs.onRemoved.addListener(onRemovedTab);
(Assignee)

Comment 18

a year ago
It definitely was fixed a year ago but it seems that the structure of the code got changed and now there is apparently a regression. I think you should file a new bug as that's the common practice.

Comment 19

a year ago
Ok thanks, I've created a new bug https://bugzilla.mozilla.org/show_bug.cgi?id=1409540

Updated

6 months ago
Product: Toolkit → WebExtensions
status-firefox51: fixed → ---
See Also: → bug 1513220
You need to log in before you can comment on or make changes to this bug.