1440015 - Using tabs.move() to move tab between windows changes the 'tabId'

Status: RESOLVED DUPLICATE of bug 1398272

(WebExtensions :: Untriaged, defect)

Description

[dw-dev]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180118215408

Steps to reproduce:

Used my Tile Tabs WE add-on in Firefox 58, 59 & 60.

This add-on moves tabs between windows and relies on keeping track of tabs using tabId's.

To reproduce the problem, use these steps:
1. Install Tile Tabs WE.
2. Start Firefox and create 4 tabs, each with different content.
3. Right-click on the Tile Tabs WE button and select New Layout > 4 Tabs - Horizontal Grid.  This will show a layout with the 4 tabs in 4 separate windows.
4. Click on the Tile Tabs WE button. This will hide the layout and move the 4 tabs back into the original window.
5. Click on the Tile Tabs WE button again.  This should show the layout again with the original 4 tabs, BUT it fails and instead its shows the layout with 4 'about:newtab' tabs. (or sometimes it just fails and does not show the layout)

This use case in Tile Tabs WE has worked correctly since Firefox 51.

The failure happens because after using tabs.move() to move a tab between windows, the 'tabId' changes.

My understanding is that tabId's and windowId's should be constant for the lifetime of the tab or window.  They certainly are on Chrome - and they were on Firefox until recently.

If the tabId does not remain constant, there is no way to keep track of a tab as it moves between windows.

Tile Tabs WE has 26,000+ users on Firefox and is also available on Chrome (using identical source code).

This seems a fundamental regression and needs fixing quickly.

I would welome any suggestions about possible workarounds.








Actual results:

In setp 5, the layout is shown with 4 'about:newtab' tabs. (or sometimes the layout is not shown)


Expected results:

In step 5, the layout should be shown with the original 4 tabs.

Comment 1

[Emil Pasca [:emilpasca]]

User Agent:  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0

I have tested this issue on Windows 10 x64 with the latest Firefox release (58.0.2) and the latest Nightly (60.0a1-20180226100105) with Tile Tabs WE (6.5) and managed to reproduce it.
After following the STR described by the reporter, when reaching to step 5, you can observe that the layout is shown with 4 'about:newtab' tabs instead of the original 4 tabs.

I've performed a regression, the result is below:

Last good revision: ad7fe0c5020c483b3d9be2e8a2719612b62b71c0
First bad revision: b1474bb83406eecbc44f7fcd591993e7ef1a18fc
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ad7fe0c5020c483b3d9be2e8a2719612b62b71c0&tochange=b1474bb83406eecbc44f7fcd591993e7ef1a18fc


Since Kevin's account has been disabled, Tomislav, can you please have a look at this?

Comment 2

[dw-dev]

I've done a bit more testing.

If I comment out the chrome.tabs.onUpdated listener completely, then the problem does not happen.

If I just comment out the code within the listener callback function, then the problem still happens.

Interestingly, the chrome.tabs.onAttached listener does not seem to cause the problem.

Comment 3

[Tomislav Jovanovic :zombie]

I hope to look into this over the weekend, but if you want to help speed things up, can you please provide a minimal extension that only reproduces the issue, trying to understand the code of the whole extension can take time.

Comment 4

[dw-dev]

I have released a new version, Tile Tabs WE 7.0, that avoids this problem by using a chrome.webNavigation.onCompleted listener instead of a chrome.tabs.onUpdated listener. There are also many other changes to the featres and internal design.

However, the reported bug can still be reproduced with Tile Tabs WE 6.5.

I am almost certain that the bug happens in step 4, when the layout is being hidden, and is triggered by the chrome.tabs.move call in line 2292 of background.js:

    chrome.tabs.move(tabs[pos].id,{ windowId: parentWindowId, index: parentPos },
    function(tab)
    {
        ...

Please let me know if you need more information.

Comment 5

[Tomislav Jovanovic :zombie]

This is bug 1398272, fixing there.