Ublock: Page-selector in logger lists multiple lines for same tab

[yasmise]

Describe the issue

When a tab is moved to another window, page-selector in logger lists the tab multiple times.
Page-selector should list one line for a tab.

One or more specific URLs where the issue occurs

http://raymondhill.net/ublock/tests.html
This issue happens on every web sites.

Screenshot in which the issue can be seen

Steps for anyone to reproduce the issue

1. Open logger of uBlock Origin.
2. Open http://raymondhill.net/ublock/tests.html on new tab.
3. Right-click "Test your blocker" tab and move it to new window.
4. Click page-selector in logger, and it has two "Test your blocker" lines.
5. Move the tab back to original window by drag and drop.
6. Page-selector has three lines for the tab.

Your settings

• OS/version: Windows 10 Home, 1709
• Browser/version: Firefox 57.0, tested on clean profile
• uBlock Origin version: 1.14.18, default settings

Your filter lists

Default filter lists + JPN

Your custom filters (if any)

Nothing.

[gorhill]

Browser bug: a new tab with a new tab id is created when moving the tab to another window, but the WebExtensions framework does not call tabs.onRemoved for the old, obsolete tab.

Aside the tabs.onRemoved listener, I don't see any other event for uBO to hook into to be made aware that the old tab no longer exist. Aside the duplicate entries in the logger, this could theoretically lead to memory leaks in uBO but there is fortunately a "janitor" in uBO which will kick in every 15 minutes to find out such ghost tabs (I suppose there was such an issue a long time ago in Chromium) and remove them and clean up properly the resources.

The same code works fine on Chromium, no duplicate entries on the logger, so the issue here will have to be reported to Firefox devs.

[yasmise]

Thanks for your quick and kind reply. I've just reported to Bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1418539
I hope you could take a look at that, and correct me if my description is wrong or missing.
Many thanks.

[gorhill]

@yasmise Thanks for filing an issue. I wanted to create a minimalist test case so that Firefox devs do not have to deal with a complex extension to analyze the issue. I added that minimalist extension to 1418539.

[uBlock-user]

Click page-selector in logger, and it has two "Test your blocker" lines.

That doesn't happen in Firefox 59

Page-selector has three lines for the tab.

That happens on Firefox 59, in my case the tab number increase by one, so in total it's two.

[gorhill]

According to the minimalist extension, FF59 suffers the same issue, except that it does not throw errors at the console after moving back the tab to its original window.

[gorhill]

The issue was marked as invalid and by design. This makes no sense -- I won't bother with bugzilla anymore, taking time to create/investigate a minimal test case to help them save time in reproducing/narrowing the issue was fruitless.

No word on step 4 -- a new tab reported apparently created out of thin air to never be heard again.

It just makes no sense to extend the semantic of tabs.onAttached/tabs.onDetached to that of tabs.onCreated/tabs.onRemoved -- the semantic is to notify that a tab is removed from a window and added to another one, not that a tab is destroyed/created. There is a reason why the info for the attach/detach event is oldWindowId/newWindowId, and not oldTabId/newTabId. With Chromium a moved tab does not lose its identity when moved across windows, while with Firefox, it's arbitrary, and the sequence of events when moving a tab across windows is messy.

Here for the record, this is what happens with Chromium -- the sequence of events is exactly what the documentation would lead one to expect, starts with 7,19, ends with 7,19 -- and properly matching the browser's internals (7,19):

tabs.onUpdated: 19 {"status":"loading","url":"chrome://newtab/"}
tab ids from event listening: (2) [7, 19]
webNavigation.onCommitted: 19 {"frameId":0,"processId":18,"tabId":19,"timeStamp":1511191161446.7449,"transitionQualifiers":[],"transitionType":"typed","url":"chrome-search://local-ntp/local-ntp.html"}
tab ids from event listening: (2) [7, 19]
tabs.onUpdated: 19 {"favIconUrl":"https://abs.twimg.com/favicons/favicon.ico"}
tab ids from event listening: (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
webNavigation.onCommitted: 19 {"frameId":14,"processId":18,"tabId":19,"timeStamp":1511191161502.1182,"transitionQualifiers":[],"transitionType":"auto_subframe","url":"chrome-search://most-visited/single.html?removeTooltip=Don%27t%20show%20on%20this%20page"}
tab ids from event listening: (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
tabs.onUpdated: 19 {"status":"complete"}
tab ids from event listening: (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
webRequest.onBeforeRequest: 19
tab ids from event listening: (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
tabs.onUpdated: 19 {"status":"loading","url":"https://news.ycombinator.com/"}
tab ids from event listening: (2) [7, 19]
webNavigation.onCommitted: 19 {"frameId":0,"processId":19,"tabId":19,"timeStamp":1511191164253.917,"transitionQualifiers":[],"transitionType":"auto_bookmark","url":"https://news.ycombinator.com/"}
tab ids from event listening: (2) [7, 19]
tabs.onUpdated: 19 {"title":"Hacker News"}
tab ids from event listening: (2) [7, 19]
tabs.onUpdated: 19 {"favIconUrl":"https://news.ycombinator.com/favicon.ico"}
tab ids from event listening: (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
tabs.onUpdated: 19 {"status":"complete"}
tab ids from event listening: (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
tabs.onDetached: 19 {"oldPosition":1,"oldWindowId":6}
tab ids from event listening: [7]
tabs.onAttached: 19 {"newPosition":0,"newWindowId":21}
tab ids from event listening: (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
tabs.onDetached: 19 {"oldPosition":0,"oldWindowId":21}
tab ids from event listening: [7]
tabs.onAttached: 19 {"newPosition":1,"newWindowId":6}
tab ids from event listening: (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]
all tab ids from tabs.query(): (2) [7, 19]

And this is with Firefox, starts with 1,3, ends with 1,3,4,5, or if one care to also listen to attach/detach events and (abusively) extend the semantic and consider them aliases of create/remove events (not needed with Chromium), ends with 1,4,5 -- meaning the moved tab lost its identity, and also no longer matching the browser's internals (1,5):

tabs.onUpdated: 3 {"favIconUrl":"chrome://branding/content/icon32.png#"}
tab ids from event listening: Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
tabs.onUpdated: 3 {"status":"loading"}
tab ids from event listening: Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
webRequest.onBeforeRequest: 3
tab ids from event listening: Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
tabs.onUpdated: 3 {"title":"news.ycombinator.com/"}
tab ids from event listening: Array [ 1, 3 ]
tabs.onUpdated: 3 {"status":"loading","url":"https://news.ycombinator.com/"}
tab ids from event listening: Array [ 1, 3 ]
webNavigation.onCommitted: 3 {"url":"https://news.ycombinator.com/","timeStamp":1511190921857,"frameId":0,"parentFrameId":-1,"tabId":3,"windowId":3,"transitionType":"link","transitionQualifiers":[]}
tab ids from event listening: Array [ 1, 3 ]
tabs.onUpdated: 3 {"title":"Hacker News"}
tab ids from event listening: Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
tabs.onUpdated: 3 {"favIconUrl":"https://news.ycombinator.com/favicon.ico"}
tab ids from event listening: Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
tabs.onUpdated: 3 {"status":"complete"}
tab ids from event listening: Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
webNavigation.onCommitted: 4 {"url":"about:blank","timeStamp":1511190927408,"frameId":0,"parentFrameId":-1,"tabId":4,"windowId":47,"transitionType":"link","transitionQualifiers":[]}
tab ids from event listening: Array [ 1, 3, 4 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
tabs.onDetached: 3 {"oldWindowId":3,"oldPosition":1}
tab ids from event listening: Array [ 1, 4 ]
tabs.onUpdated: 3 {"favIconUrl":"https://news.ycombinator.com/favicon.ico"}
tab ids from event listening: Array [ 1, 4, 3 ]
tabs.onUpdated: 3 {"title":"Hacker News"}
tab ids from event listening: Array [ 1, 4, 3 ]
tabs.onAttached: 3 {"newWindowId":47,"newPosition":0}
tab ids from event listening: Array [ 1, 4, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
all tab ids from tabs.query(): Array [ 1, 3 ]
tabs.onUpdated: 5 {"favIconUrl":"https://news.ycombinator.com/favicon.ico"}
tab ids from event listening: Array [ 1, 4, 3, 5 ]
tabs.onUpdated: 5 {"title":"Hacker News"}
tab ids from event listening: Array [ 1, 4, 3, 5 ]
tabs.onAttached: 3 {"newWindowId":3,"newPosition":1}
tab ids from event listening: Array [ 1, 4, 3, 5 ]
tabs.onDetached: 3 {"oldWindowId":47,"oldPosition":0}
tab ids from event listening: Array [ 1, 4, 5 ]
all tab ids from tabs.query(): Array [ 1, 5 ]
all tab ids from tabs.query(): Array [ 1, 5 ]
all tab ids from tabs.query(): Array [ 1, 5 ]
all tab ids from tabs.query(): Array [ 1, 5 ]

"This is the expected behavior".

Anyways, concerning how uBO can deal with the issue, I will leave it as is, I don't want to put special code paths to deal with that weirdness, this could cause regressions, including on Chromium which does not suffer any issue. As said, the janitor code will kick in at some point (every 15 minutes) to garbage collect invalid tab entries.

[gorhill]

Fixed in Firefox 61+: https://bugzilla.mozilla.org/show_bug.cgi?id=1398272.