1759231 - Downloads panel opens when extensions (e.g. Simple Tab Groups) automatically start and delete downloads, without user interaction. (investigate ways to prevent this)

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect, P2)

Description

[Chris Midgley]

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

Steps to reproduce:

Have downloaded some files in the current session, then continue browsing the web.

Actual results:

Download panel pops up, showing previously downloaded files.

Expected results:

Download panel should not pop up unless I trigger a download.

This has happened twice in approximately 8 hours. It's confusing, distracting and frustrating, and I don't know what the cause is (no download appears in the panel). I think it might be one of my extensions using browser.downloads.download to download something, then browser.downloads.erase to remove it from the list so I don't see it. I know "Simple Tab Groups" does this for backups.

I'm currently working around it by setting "browser.download.alwaysOpenPanel" in about:config to false, which stops opening the panel by default.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox::Downloads Panel' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Shane Hughes [:aminomancer]]

It doesn't seem like there's anything the extension can do to avoid this, nor any simple way to prevent this by patching DownloadsCommon. I guess a new DownloadCore property could be added like background that extensions could set. But that kinda defeats the purpose of the shouldOpenDownloadsPanel feature. I think the alwaysOpenPanel feature makes sense as a replacement for the UCT dialog if user is using vanilla Firefox without certain addons, but it fails to account for the huge variety of ways that downloads can be started, with or without user interaction.

I would propose the alwaysOpenPanel pref should just be disabled by default, but already there has been a lot of communication announcing this feature in release notes and SUMO, so that it would be confusing to disable it now. So it sounds like the only courses of action are to 1) extend the webextensions API so it can start a download without opening the panel; or 2) just permit the downloads panel to open without user interaction due to webextension activity.

Comment 4

[james]

I'm experiencing this bug as well. Extremely annoying - to the point where I went searching for this bug and created an account just to post this comment.

Comment 5

[Shane Hughes [:aminomancer]]

(In reply to james from comment #4)

I'm experiencing this bug as well. Extremely annoying - to the point where I went searching for this bug and created an account just to post this comment.

Thanks for sharing, I think it's clear why this is happening. It's expected for the downloads panel to open when a download begins, even if the download stops immediately after. But since this is a new feature, addons have not been developed with the expectation that this will happen. The reason for this feature is because the unknown content type dialog (that asks if you want to open or save the downloaded file) is no longer opened by default. So when a download finishes, there isn't an immediate affordance for opening the download unless the panel is opened.

I don't think this will be resolved by just removing that whole feature, and it seems kinda hacky for an addon to do this in the first place. In fact, the same conditions lead to the indicator attention state being set. So the download icon would appear (if set to auto-hide) and be highlighted with the "attention" color, even though the download was immediately deleted.

The reason this isn't noticed is because all of that is immediately reversed by a method that is invoked when the download is deleted, to reset the indicator state. Whereas the panel can't be automatically closed when a download is deleted, because the user might actually be interacting with the panel. And even if it could be, I think the panel opening and closing would take long enough to be painted, so it would look really janky.

If an addon did the same thing in another browser, it would cause the same problem, but with an empty "downloads bar" appearing instead of an empty downloads panel. So ideally the addon author will use a different method to do whatever it's trying to do, irrespective of how we go about resolving this.

My first thought was that we could add some new logic for deciding when the panel should be opened. That maybe the downloads API could be extended so that an addon could create a "background download" to do something unconvential. But on further reflection, this seems like it could become a nuisance or even a security vulnerability. If an addon is downloading something without user interaction then there definitely should be a very noticeable indication of that.

I thought I might do some testing to see if setting a brief timeout before opening would resolve this. That is, if the extension is starting a download and erasing it in the same synchronous execution context, then we could just wait a few milliseconds, then check that the download still exists before setting the indicator state and opening the panel. But it occurs to me that, as I noted, it'd only solve the problem for that specific application where both .download and .erase are invoked within a certain window.

It's bad practice to be doing that in the first place. Surely there must be a better approach. And if there isn't, then I think this should be an enhancement request for some new webextensions javascript API method. I think we ought to compile a list of extensions associated with this downloads panel issue, and communicate with their authors to find out whether new API features need to be added.

Relevant extensions

• Simple Tab Groups

Comment 6

[Shane Hughes [:aminomancer]]

Maybe I'm mistaken about "background downloads" being a potential problem though. After all, extensions were able to do something similar in the past (i.e., download a file without a "Save As" dialog or unknown content type dialog opening). It just seems like someone more knowledgeable about webextensions might object to exposing this directly as a parameter.

Also, it seems like whether there needs to be an affordance for downloads created by extensions, and whether there could be a delay on that, might be questions for the UX team.

Comment 7

[Chris Midgley]

Having worked with this for a while, I now find the panel opening less distracting. Before, I would see the little animation for downloads happening, but I could more easily ignore it. Now I am getting used to ignoring the panel as well :)

The more troublesome part is that the panel steals focus. I can be typing away somewhere, then STG triggers a backup, and now I am no longer typing where I was. This is similar to concerns raised in https://bugzilla.mozilla.org/show_bug.cgi?id=1749998, except that the counter-argument there said it was reasonable to take focus in response to a user-triggered action, but here the panel is taking focus in response to a non-user triggered action.

Comment 8

[:Gijs (he/him)]

Yeah, this is like a much worse version of bug 1689929.

Luca, any idea what to do about this?

Comment 9

[Luca Greco [:rpl] [:luca] [:lgreco]]

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

Yeah, this is like a much worse version of bug 1689929.

I agree that this is worse than Bug 1689929 (imho this looks worse enough to be categorized as defect, Bug 1689929 was triaged as P5 and type enhancement).

Luca, any idea what to do about this?

Follows some preliminary notes about an idea I've been thinking of to deal with this issue.

I'm adding a discussion point for this issue to the list for our next WebExtensions API "design decisions triaging meeting",
to pass by the other WebExtensions peers both this issue and the idea described below and gather some more details based
on their perspective (and also other potential ideas).

In the meantime I thought it was still valuable to briefly describe it here so that we can also double-check how this idea
would look from the perspective of engineers working on the Downloads internals.

download panel to not be opened when an extension triggers a download without being handling user input

pros

• the extensions are still allowed to use the downloads API for the "periodic backups" use case
• the existing extensions would keep working without having to be updated to pass a new option to prevent this issue from being triggered

cons

• requires some changes on both the WebExtensions and Downloads internals (likely not the kind that we would want to uplift, but that's just an upfront guess)
• the download icon would still be flashing (but that it is tracked by the separate Bug 1689929 and I wouldn't count it as a blocking issue to consider proceeding with fixing this one)

changes likely to be required

(there may be more but this are the ones that I can think of up front):

• on the WebExtensions internals side:

  • on the child process side, we may change ProxyAPIImplementation.callAsyncFunction to make sure to propagate to the parent process (along with the other call details) a flag to be able to determine in the parent process if the method was called while handling user input (which we could achieve by forwarding the value got from this.context.window.windowUtils.isHandlingUserInput at the time we hit that internal method)
  • on the parent process side, we should then make sure to propagate the new isHandlingUserInput flag to the methods that are actually implementing the WebExtensions API methods in the parent process ExtensionAPI subclasses (so that we may get that additional flag as part of the call to the downloads.downloads API method implemented in ext-downloads.js)
  • on the ext-downloads.js side, then propagate the new isHandlingUserInput flag to the Download internals (e.g. passing it as part of the call to Downloads.createDownload or download.start()

• on the Downloads internals side:

  • account for the optional flag (e.g. one named isHandlingUserInput or extensionsIsHandlingUserInput if we will prefer to only make use to it for the calls originated by the extensions) and eventually store it in the downloads object, so that we may then use it in the Downloads UI internals to determine if the downloads panel should be opened automatically or not

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:mak, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 13

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #9)

...
I'm adding a discussion point for this issue to the list for our next WebExtensions API "design decisions triaging meeting",
to pass by the other WebExtensions peers both this issue and the idea described below and gather some more details based
on their perspective (and also other potential ideas).
...

As anticipated I have discussed about the approach described in comment 9 with the rest of the Add-ons Team in our WebExtensions API "design decisions triaging meeting" last week, and we agreed that the proposed approach (and related changes that would be needed on the WebExtensions side) sounds reasonable and we are ok to proceed.

I just filed Bug 1761828 for the WebExtensions side of the changes described in comment 9 (and I'm also happy to assign it to myself if we are all in agreement to proceed in this direction).

If the proposed approach sounds reasonable also from the perspective of the Firefox Desktop engineers, could someone take care of filing (and picking up) another bugzilla issue for the changes needed on the Downloads internals side?
(which would be taking care of adding the option to the Downloads internals, the one that we would then pass based on the user handling flag in our call originated from ext-downloads.js as part of Bug 1761828).

Comment 14

[Marco Bonardo [:mak]]

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #13)

could someone take care of filing (and picking up) another bugzilla issue for the changes needed on the Downloads internals side?

I suspect we could just use this same bug, or should the frontend part be ready before the add-ons part?

Comment 15

[Shane Hughes [:aminomancer]]

I think it might be safer to add the download property first, than to pass an unrecognized property to the download constructor?

Luca, regarding the property name — isHandlingUserInput seems great for the WebExtensions code, but for the download constructor I think the property should be inverted and more generic. There might be other situations when we want to prevent the panel from showing, besides a lack of user input. And most of the time, we want the panel to open, so it makes sense for the property that stops the panel from opening to be falsy by default. Since the property is (at least as of now) only used for stopping the panel from opening, how is dontOpenPanel for the name? It's clear and it'll be false by default. So in the createDownload call, it would just be passing dontOpenPanel: !isHandlingUserInput

On the download internal code, I have another question regarding how this fits in with bug 1689929 (or doesn't, preferably), but I will post it in another bug report.

Comment 16

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Shane Hughes [:aminomancer] from comment #15)

I think it might be safer to add the download property first, than to pass an unrecognized property to the download constructor?

sure, that sounds reasonable:
the download property could be added first and tested on its own, then in the change on the WebExtensions side we would make sure to pass the option and cover it with an integration test (which would make sure the behavior of the downloads panel then match the expected one when that download property is being set based on the "isHandlingUserInput" flag propagated internally).

Luca, regarding the property name — isHandlingUserInput seems great for the WebExtensions code, but for the download constructor I think the property should be inverted and more generic. There might be other situations when we want to prevent the panel from showing, besides a lack of user input. And most of the time, we want the panel to open, so it makes sense for the property that stops the panel from opening to be falsy by default. Since the property is (at least as of now) only used for stopping the panel from opening, how is dontOpenPanel for the name? It's clear and it'll be false by default. So in the createDownload call, it would just be passing dontOpenPanel: !isHandlingUserInput

Yep, +1 I also agree:
the download property name should reflect the expected behavior, dontOpenPanel sounds like a good name as it would be immediately suggest what is the expected behavior when that is passed (and isHandlingUserInput would just be likely the name of the flag we will be propagating internally).

On the download internal code, I have another question regarding how this fits in with bug 1689929 (or doesn't, preferably), but I will post it in another bug report.

Sounds good, I'll take a look to the question in the other bug report.

Comment 17

[Shane Hughes [:aminomancer]]

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #16)

Sounds good, I'll take a look to the question in the other bug report.

Great, I'm beginning work on bug 1762033. As for the question I mentioned, it's just the remarks at the end of my first comment in bug 1762033.

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.
:mak, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 19

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1759231 - Downloads panel should not open on extension created downloads created while not handling user input. r?mak!,robwu!

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1eb191c96aef
Downloads panel should not open on extension created downloads created while not handling user input. r=mak,robwu

Comment 22

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1eb191c96aef

Comment 23

[Ardelean Oana, Desktop QA [:oardelean]]

I was able to reproduce this on Firefox 100.0(20220428192727) on macOS 11 with various extensions enabled( especially Simple Tab Groups). The flash is very brief but it's there and it usually appears shortly after a browser restart. I think it's safe to mark this verified as fixed on Firefox 101.0b6(20220512193916), Nightly 102.0a1(20220512213051) on macOS 11, Win10 64-bits and Ubuntu 20.04, since I've been waiting for the flash to appear for a couple hours with no success. Please ni? me if you still encounter this issue.