Extensions do not get the correct discarded status of tabs. No tabs are reported as discarded.
Problem found to occur on Windows 7 after updating FF from v86 to v87. But not on Windows 10.

Want to add, this is only for tabs restored from a session restore. Manually discarded tabs via tabs.discard() are correct

Hello,

I’m from QA and I’m attempting to confirm the issue. Could you please provide detailed steps to reproduce as well as the extension used when you uncovered the issue?

Thank you !

• Check "Restore previous session" in Firefox options
• Install this add-on: https://addons.mozilla.org/en-US/firefox/addon/loadtabonselect3/
• Open a few web pages
• Close then reopen Firefox, but do not load any background tabs by focusing on them
• Click on the add-on button, and in the popup, hover over the recycle icon

The hover tooltip shows "Load all (0) discarded tabs", indicating no discarded tabs are found. Restored background tabs were reported as discarded in earlier versions of Firefox. However, if tabs are loaded, and then discarded by an extension (e.g. https://addons.mozilla.org/en-US/firefox/addon/unload-tabs/), then only those tabs discarded by the extension are detected

I'm actually having this issue in Tree Style Tab (https://addons.mozilla.org/en-US/firefox/addon/tree-style-tab/), but it's more complicated to show the effect in that add-on

Here's the about:support from a fresh profile on Nightly if that helps

Hello mesvam and thank you for the additional info !

I tried reproducing the issue on the latest Nightly (89.0a1/20210328213901), Beta (88.0b4/20210328185936) and Release (87.0/20210318103112) under Windows 10 x64 (despite you mentioning that the issue does not occur on this specific OS) and Ubuntu 16.04 LTS, without success. The add-on you pointed me to will correctly indicate the number of discarded tabs on the above mentioned OSes.
Unfortunately, I do not have access to a machine with Windows 7.

Ahh, ok. Thanks!

mesvam, since you're able to consistently reproduce this, could you run mozregression to find the affected version?
See https://mozilla.github.io/mozregression/

Bisected to this https://phabricator.services.mozilla.com/D102680
Thanks!

That's unlikely. Can you try again?

Ok, checking again, it looks like mozregression only narrowed it down to this range https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=37cbc73b07c3e0b8d9441bea2beae71ecdc1dd14&tochange=2c9004c95a7a7d39c2c1417b66fda37892398263
Might not be specific to part 5 only

That's still unlikely. The given changeset concerns an accessibility API. The reported bug is about a supposedly incorrectly reported flag in the frontend. Could it be that the tab is not actually discarded, but being restored (e.g. by one of your add-ons)?

If you are able to, can you create a minimal test extension for the STR (to rule out side effects from other extension API calls) and try to reproduce again?

That's a bit new to me, but I'll see what I can do. Thanks,

I made this extension to test tabs.query({discarded: true}), and it just logs the number to console. Fresh nightly profile with no other modifications or extensions. And mozregression is still pointing to https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=37cbc73b07c3e0b8d9441bea2beae71ecdc1dd14&tochange=2c9004c95a7a7d39c2c1417b66fda37892398263

But here's the thing, I also tried it on another Windows 7 machine with FF v87 (release) and could not reproduce this issue. I guess there's something different in Windows that prevents it from working

I've found the issue. The time tracking software ManicTime was using the Firefox accessibility service. Disabling accessibility services with accessibility.force_disabled = 1 in about:config fixes this. Though still no idea why this would affect the tabs API

I'm trying to reproduce as follows:

1. Visit about:config and set accessibility.force_disabled = -1 (note: -1 is force-enable, 1 is force-disable). Visit about:preferences and check "Restore previous session". Open example.com and a few other websites.
2. Install the add-on from comment 13 via about:debugging (also tried to set the extension ID in manifest.json and installing the xpi).
3. Quit Firefox and start it. Re-install the add-on if needed.
4. Open the console, enable logging of content messages.
5. Click the extension button and look at the console.

According to your report, it should incorrectly print 0 (meaning that all tabs are not discarded).
But I cannot reproduce your observation in 88.0a1 buildID 20210322093509. The number matches with the number of websites in the background.

Are you able to reproduce the issue without ManicTime?

Though still no idea why this would affect the tabs API

Is it a bug in the tabs API or are the tabs actually not discarded? If you focus the tab, is it reloading or has it been reloaded already?

Do you see any errors in the browser console?

Are you able to reproduce the issue without ManicTime?

I was able to reproduce this on another machine without ManicTime, Windows 10 with accessibility.force_disabled = -1.

Do you see any errors in the browser console?

Here are the messages in the browser console

Unknown category for SetEventRecordingEnabled: fxmonitor
Key event not available on some keyboard layouts: key=“i” modifiers=“accel,alt,shift” id=“key_browserToolbox” browser.xhtml
can't access property "browserPageAction", pageActionAPI is undefined pageActionExtras.js:35
Shimming these match patterns 
Array(20) [ "*://webcompat-addon-testbed.herokuapp.com/shims_test.js", "*://example.com/browser/browser/extensions/webcompat/tests/browser/shims_test.js", "*://example.com/browser/browser/extensions/webcompat/tests/browser/shims_test_2.js", "*://example.com/browser/browser/extensions/webcompat/tests/browser/shims_test_3.js", "*://static.adsafeprotected.com/vans-adapter-google-ima.js", "*://pagead2.googlesyndication.com/pagead/js/adsbygoogle.js", "*://auth.9c9media.ca/auth/main.js", "*://libs.coremetrics.com/eluminate.js", "*://connect.facebook.net/*/sdk.js*", "*://connect.facebook.net/*/all.js*", … ]
shims.js:17:19
can't access property "action", pageActionAPI is undefined pageActionExtras.js:25
can't access property "browserPageAction", pageActionAPI is undefined pageActionExtras.js:38
Uncaught (in promise) Error: An unexpected error occurred undefined
Uncaught (in promise) Error: An unexpected error occurred undefined
Uncaught (in promise) Error: An unexpected error occurred undefined
debuggee 'resource://devtools/shared/base-loader.js:289' would run builtin-modules.js:196:11

Is it a bug in the tabs API or are the tabs actually not discarded? If you focus the tab, is it reloading or has it been reloaded already?

The tabs do appear to be loading from a discarded state when focused. The throbber in the favicon appears, and time to first paint is much slower than other loaded tabs, and there's a spike in network usage. I don't know if there's a more reliable way to get that info.

Alex, can you try to reproduce this on Windows, Linux and macOS, and if you can reproduce, run mozregression to confirm?

STR in comment 15. The suspected regression range has been posted before, but I'd like a confirmation (and see whether it differs by platform).

Hello Rob!

As requested I tried the STR from Comment 15 on all the requested OSes, with the following results:

Windows 10 x64:

• on Nightly and Beta, there is nothing logged into the browser console (with content messages logging enabled), only in the add-on debug console. On Release, the browser console prints info which is not related to the issue as far as I can tell (but see the screenshot for reference)
• on all versions of the browser, the add-on console logs the value 0 when clicking the add-on icon.
• focusing a tab will start to reload it

Ubuntu 16.04 LTS:

• everything seems in order here, content messages are displayed in the browser console and furthermore, clicking the add-on icon will correctly print the number of websites in the background (which was 3 in my case as before restarting the browser I had example.com, facebook.com and wikipedia opened alongside about:debugging).

MacOS 11.2.2:

• same as on Ubuntu.

So it appears the issue is only on Windows.

Tested this with the latest Nightly (89.0a1/20210331215444), Beta (88.0b5/20210330185720) and Release (87.0/20210318103112)

Regarding the regression window, I cannot provide one...I’m using mozregression through CLI since I cannot use the GUI version (the GUI version is detected as malware and the .exe is deleted upon installation), but the CLI version gives me an error (WARNING: Process exited with code 4294967295) each time it tries to download the build I specified as bad or even when I don’t specify a bad build and it tries to download the most recent build.

(In reply to Alex Cornestean from comment #18)

Regarding the regression window, I cannot provide one...I’m using mozregression through CLI since I cannot use the GUI version (the GUI version is detected as malware and the .exe is deleted upon installation), but the CLI version gives me an error (WARNING: Process exited with code 4294967295) each time it tries to download the build I specified as bad or even when I don’t specify a bad build and it tries to download the most recent build.

Hi William, I believe you asked for mozregression-gui testing and bug reports recently?

Can you please connect with and help Alex with getting a working regression setup, he does most of QA for addons, and has been having issues for a while.

(In reply to Tomislav Jovanovic :zombie from comment #19)

Hi William, I believe you asked for mozregression-gui testing and bug reports recently?

Can you please connect with and help Alex with getting a working regression setup, he does most of QA for addons, and has been having issues for a while.

I can't promise anything (maintaining mozregression is not in my job description) but I do try to respond to bug reports (in the testing/mozregression component) and there is a #mozregression channel on chat.mozilla.org where folks can ask questions and get answers.

With regards to the above, a full log of the command line output (including the input) might help us debug things. Please file a new bug :)

Anti-virus false positive issues are being tracked in bug 1647533. We released a new version 7 days ago which should be a little better in this regard: https://github.com/mozilla/mozregression/releases/tag/4.0.16

I understand the situation :/ and thank you for all the work!

Alex, can you please follow up with logs/bug reports as William mentions. ^

Hello guys and thank you for the assist !

As requested I’ve submitted a bug regarding the CLI mozregression issue I’ve been having (https://bugzilla.mozilla.org/show_bug.cgi?id=1703166). I noticed and also specified in the bug that mozregression seems to fails when dealing with builds from 2021. Builds from 2020 are downloaded and run without any issues as far as I can tell atm.

Regarding the latest GUI version of mozregression for Windows, I cannot download it...Firefox sees it as dangerous and won’t download it, Chrome blocks it, Opera fails to download it as well.

Tried downloading version 4.0.14, which succeeds, but immediately after installation the .exe is deleted. So all this seems to be https://bugzilla.mozilla.org/show_bug.cgi?id=1647533 all over again, unfortunately.

I also forgot to attach a screenshot I mentioned in Comment 18. Sorry about that, Rob !

Got mozregression working properly again and managed to bisect the issue. Regressor appears to be https://bugzilla.mozilla.org/show_bug.cgi?id=493683 (Fire name/description change event when aria-labelledby/describedby content changes)

57:33.99 INFO: No more integration revisions, bisection finished.
57:34.00 INFO: Last good revision: 37cbc73b07c3e0b8d9441bea2beae71ecdc1dd14
57:34.00 INFO: First bad revision: 2c9004c95a7a7d39c2c1417b66fda37892398263
57:34.00 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=37cbc73b07c3e0b8d9441bea2beae71ecdc1dd14&tochange=2c9004c95a7a7d39c2c1417b66fda37892398263

It’s the same regression range as in Comment 13.

I'm not sure if it's related, but current code

  get discarded() {
    return !this.nativeTab.linkedPanel;
  }

https://searchfox.org/mozilla-central/rev/26ac39c8d5e3c4b93e3519be7302e15ae42e9833/browser/components/extensions/parent/ext-browser.js#812-814

is unreliable, sometimes the tab is unloaded but tab.discarded === true. I think that

  get discarded() {
    return !this.nativeTab.getAttribute("pending") === "true";
  }

should be used instead.

Just correcting my previous comment:

is unreliable, sometimes the tab is unloaded but tab.discarded === true. I think that

Corrected:

is unreliable, sometimes the tab is unloaded but tab.discarded === false. I think that

Also without the ! in get, of course.

return this.nativeTab.getAttribute("pending") === "true";

hey Jemie, can you please check out what's going on here?

Urgh. This is nasty.

Bug 493683 introduced a change that causes a11y to request the LABEL_FOR relation when building the a11y tree. For XUL tabs (the basis for browser tabs), this ends up calling nsIDOMXULRelatedElement::getRelatedElement. Unfortunately, it turns out that browser tabs override getRelatedElement to insert the browser. (This was done in bug 1287330.) The practical upshot is that when the a11y tree gets built, all the tabs get loaded. 😟 This also explains why tab documents get inserted as a result of that call, which was triggering bug 1689936 (since fixed).

I'm not entirely sure how to fix this. A11y does need to be able to figure out what tab labels a given tab panel, but we certainly don't want the tab panel to be created when we query that. AsyncTabSwitcher uses getRelatedElement, so we can't change that.

It's a bit of a hack, but maybe we can set some attribute on browser tabs that are disconnected? A11y could query that attribute and avoid calling getRelatedElement if the attribute is set. I don't think we can use the "pending" attribute because I think that is set by SessionStore, so I'm guessing it might not be set on tabs discarded after restore?

Note that whatever solution we come up with needs to work for all XUL tabs, since a11y doesn't know whether it's a browser tab or another kind of XUL tab. For example, I considered querying the linkedpanel attribute, but it seems XUL tabs might not have that attribute and might fall back to matching indexes.

Dao, do you have any recommendations here (or can you point me to someone who might)?

Gijs, might you have some thoughts on comment 30 or be able to redirect me to someone that might? This bug seems pretty nasty to me, so I'd like to have a path forward sooner rather than later. Thanks.

(In reply to James Teh [:Jamie] from comment #31)

Gijs, might you have some thoughts on comment 30 or be able to redirect me to someone that might? This bug seems pretty nasty to me, so I'd like to have a path forward sooner rather than later. Thanks.

I'm not sure I really follow, unfortunately. Some questions inline to help clarify:

(In reply to James Teh [:Jamie] from comment #30)

Urgh. This is nasty.

Bug 493683 introduced a change that causes a11y to request the LABEL_FOR relation when building the a11y tree. For XUL tabs (the basis for browser tabs), this ends up calling nsIDOMXULRelatedElement::getRelatedElement.

OK, but why? The tabs have their own labels. What's the "point" of calling GetRelatedElement? I found https://searchfox.org/mozilla-central/rev/e08a9aa5c5dfb77e8847ab7af3f753a9c3e551f8/accessible/xul/XULTabAccessible.cpp#100 but that doesn't really tell me what this LABEL_FOR relation is, or why we need it.

Unfortunately, it turns out that browser tabs override getRelatedElement to insert the browser. (This was done in bug 1287330.) The practical upshot is that when the a11y tree gets built, all the tabs get loaded. 😟 This also explains why tab documents get inserted as a result of that call, which was triggering bug 1689936 (since fixed).

I'm not entirely sure how to fix this. A11y does need to be able to figure out what tab labels a given tab panel, but we certainly don't want the tab panel to be created when we query that. AsyncTabSwitcher uses getRelatedElement, so we can't change that.

It's a bit of a hack, but maybe we can set some attribute on browser tabs that are disconnected? A11y could query that attribute and avoid calling getRelatedElement if the attribute is set. I don't think we can use the "pending" attribute because I think that is set by SessionStore, so I'm guessing it might not be set on tabs discarded after restore?

Note that whatever solution we come up with needs to work for all XUL tabs, since a11y doesn't know whether it's a browser tab or another kind of XUL tab. For example, I considered querying the linkedpanel attribute, but it seems XUL tabs might not have that attribute and might fall back to matching indexes.

Dao, do you have any recommendations here (or can you point me to someone who might)?

I mean, from a very quick look, it looks like besides the a11y callers, there are only 2 callers of getRelatedElement, and both are for tabs that are supposed to be selected. So we can either update getRelatedElement to return null if the tab is not selected, or, if there's some reason I'm missing that that doesn't work, introduce a different method either for the a11y code or for those 2 consumers to split the expectations (of inserting the panel, or not doing so).

(In reply to :Gijs (he/him) from comment #32)

Bug 493683 introduced a change that causes a11y to request the LABEL_FOR relation when building the a11y tree. For XUL tabs (the basis for browser tabs), this ends up calling nsIDOMXULRelatedElement::getRelatedElement.

OK, but why? The tabs have their own labels. What's the "point" of calling GetRelatedElement?

LABEL_FOR allows an a11y client to figure out what target element is labelled by this element. A client might query this, for example, to avoid double reporting of labels.

I mean, from a very quick look, it looks like besides the a11y callers, there are only 2 callers of getRelatedElement, and both are for tabs that are supposed to be selected. So we can either update getRelatedElement to return null if the tab is not selected, or, if there's some reason I'm missing that that doesn't work,

LABEL_FOR could get called on a selected tab.

introduce a different method either for the a11y code or for those 2 consumers to split the expectations (of inserting the panel, or not doing so).

Adding a new method for the a11y code would involve messing with XUL interfaces, so it'd probably be simpler to add another method for the other two JS consumers. The reason for my idea of using an attribute was to try to limit this obscure hack to the a11y code as much as possible (apart from setting the attribute), but I guess another method is hardly a big deal.

(In reply to James Teh [:Jamie] from comment #33)

I mean, from a very quick look, it looks like besides the a11y callers, there are only 2 callers of getRelatedElement, and both are for tabs that are supposed to be selected. So we can either update getRelatedElement to return null if the tab is not selected, or, if there's some reason I'm missing that that doesn't work,

LABEL_FOR could get called on a selected tab.

OK, but the selected tab will have a browser (or if not yet, needs one also for the two frontend consumers), so that doesn't have the perf problem we're trying to fix, right?

(In reply to :Gijs (he/him) from comment #34)

OK, but the selected tab will have a browser (or if not yet, needs one also for the two frontend consumers), so that doesn't have the perf problem we're trying to fix, right?

Oh, right. Durp. 😳 So yes, that should work.

Otherwise, callers might end up unintentionally binding the browser for lazy background tabs.
This was happening when a11y queried the LABEL_FOR relation while building the a11y tree, causing all lazy tabs to be loaded.
All callers of this method only need it to insert a browser when the tab is selected anyway.

https://hg.mozilla.org/mozilla-central/rev/7c0f2ddf91b0

Hello,

Verified the fix on the latest Nightly (90.0a1/20210523092307) under Windows 10 x64.

I’ve tried both the original STR and the STR from Comment 15 (using the add-on from Comment 13) and in each case, the correct number of discarded tabs is displayed (both in the pop-up of the add-on used in the original STR and logged in the browser console for the add-on in Comment 13), confirming the fix.