1782205 - Download panel is not auto-opened when downloads are initiated by drag and drop

Status: NEW

(Firefox :: Downloads Panel, defect, P3)

Description

[Catalin Sasca, Desktop QA [:csasca]]

Attachment: drag and drop.gif

Found in

• 104.0b3

Affected versions

• Firefox 102.0
• Firefox 104.0b3
• Firefox 105.0a1

Affected platforms

• Windows 7 & 8.1
• Ubuntu 22.04
• macOS 11.6.8

Steps to reproduce

1. Launch Firefox
2. Drag and drop a link, for example from another browser to Firefox

Expected result

• The downloads panel auto-opens when a download is initiated

Actual result

• The downloads panel auto-opens only once first time, then doesn't open anymore

Regression range

• WIll see for a regression if there is one, seems that 102.0 is affected as well.

Additional notes

• The issue can be seen in the attachment.
• Windows 10 is not affected by the issue.
• "Show panel when download begins" setting is selected by default.

Comment 1

[Amir Yalchi]

2022-07-31T12:06:46.949000: DEBUG : Found commit message:
Bug 1741076 - do not open the download panel if the window is not active, r=NeilDeakin

Differential Revision: https://phabricator.services.mozilla.com/D135710

Comment 2

[Shane Hughes [:aminomancer]]

Judging from the recording and the fact that it doesn't affect Windows 10, I suppose this is not expected behavior, but the code related to bug 1741076 is not at fault. The reason bug 1741076 triggered this issue is just that before it, the downloads panel always opened when a download started, regardless of whether the window was active. But now, the panel is not supposed to open when the window is inactive.

The problem I'm seeing is that for some reason, the window is considered active the first time and not on subsequent drag-and-drops. So it should either never open in this context or always open in this context. But I suspect the reason that's inconsistent is a much lower-level issue, maybe with the drag service? What's happening in Windows 10? Is the panel always opening or is it never opening? Its showing the first time might be caused by panelHasShownBefore.

If we want the downloads panel to always open when dragging and dropping into the window, then we could try to manually set that flag when the download is created by drag-and-drop. But there might be an independent reason for not wanting to open the downloads panel under these circumstances. Bug 1761265 prevents the panel from opening when the download was triggered by a user action that is expected to create a download.

I don't know if this really counts, because the user action (drag-and-drop) is simpler and doesn't always create a download. But that would be worth considering, because I think that determines who is best positioned to work on this bug. If we want to ensure the downloads panel always opens on drag and drop, then I think the frontend team would want to add a new flag that overrides this check, and have the drag-and-drop consumer that leads to download creation set that flag. Such a flag might be useful for other download sources that require some other application to be focused.

If we don't want the panel to ever open for downloads created by drag-and-drop, then we could just do the same thing in reverse I guess. But if we don't want to set a rule for this specific action, and just use the normal rules (check the alwaysOpenPanel pref, check window activity), then it seems the problem, at least on Windows, might be a widget/toolkit thing.

I'm personally inclined toward seeing the downloads panel always open in the top window when a download is created by drag-and-drop, and I think it would be good to get a new flag since there might be some other download sources that come from another application and don't focus the Firefox window.

---

Edit: By the way, I think the panelHasShownBefore flag ought to be used a bit differently now that alwaysOpenPanel exists. The former long predates the latter so I think I understand why they might conflict. panelHasShownBefore was supposed to keep the panel closed after the user closed it. But at that time, it was the only check. Now there are a bunch.

So right now, the way it's functioning is as an override to the other checks that are supposed to suppress the panel under certain circumstances. With the exception of the new openDownloadsListOnStart flag, all the other checks that result in !shouldOpenDownloadsPanel (like browserWin == Services.focus.activeWindow) will be canceled out if this is the first download of the session. We're basically saying:

if (this.panelHasShownBefore && !shouldOpenDownloadsPanel) {
  show notification instead of panel...
}

Which has the effect of guaranteeing that the downloads panel is shown on the first download created in a session, even if it was created by sources that definitely shouldn't show the panel. For example, the bug that initially prompted us to create this openDownloadsListOnStart flag was one that involved extensions downloading files without any user interaction as a storage method. Users were seeing the panel pop up for what appeared to be no reason, because as soon as the download was created, the extension cleared it out.

So, downloads created under those circumstances (as basically a hacky method of extension storage) should never open the panel, even if the panel has not been shown before. But as the checks are currently set up, we're basically saying we can only replace the panel with an indicator animation if the panel has previously been shown by this method. Both checks need to pass — the panel needs to have been suppressed by other checks (such as the extension user interaction check) AND the panel needs to have previously been opened. I think this just fell under the radar because the checks have become so numerous and confusing.

What we should be doing instead is to basically change the AND operation to an OR. If the panel was previously opened, and alwaysOpenPanel is disabled, then don't open the panel. Otherwise, if any of our other checks failed, don't open the panel. I think this is about right:

let shouldOpenDownloadsPanel =
  openDownloadsListOnStart &&
  aType == "start" &&
  DownloadsCommon.summarizeDownloads(this._downloads).numDownloading <= 1 &&
  browserWin == Services.focus.activeWindow;

// Current behavior is for the downloads panel to always open except in the
// circumstances checked above. However, the user can override this behavior by
// disabling the alwaysOpenPanel pref or by disabling the downloads panel
// improvements altogether. This will revert the panel to the traditional
// behavior of opening the downloads panel automatically once per session.
let alwaysOpen =
  lazy.gAlwaysOpenPanel &&
  Services.prefs.getBoolPref("browser.download.improvements_to_download_panel");

// If we're not showing the panel, then provide a visible notification in the
// topmost browser window. Also ensure that if openDownloadsListOnStart = false
// is passed, we always skip opening the panel. That's because this will only be
// passed if the download is started without user interaction, if a dialog was
// previously opened in the process of the download (e.g. unknown content type
// dialog), or if the download was created by certain context menu items.
if (aType != "error" && (!shouldOpenDownloadsPanel || (this.panelHasShownBefore && !alwaysOpen))) {
  DownloadsCommon.log("Showing new download notification.");
  browserWin.DownloadsIndicatorView.showEventNotification(aType);
  return;
}

this.panelHasShownBefore = true;
browserWin.DownloadsPanel.showPanel();

Comment 3

[Shane Hughes [:aminomancer]]

Gijs and Neil, see my edit in the post above when you get a chance. I previously ran into some test issues with panelHasShownBefore so I think this would be a good way to change it. I think that change alone might fix the inconsistency seen in OP's recording, where the panel opens on the first download and stops thereafter. But idk if there's some broader objectives for how to integrate panelHasShownBefore with the new downloads panel improvements, so let me know if this is off base.

Comment 4

[:Gijs (he/him)]

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

Judging from the recording and the fact that it doesn't affect Windows 10, I suppose this is not expected behavior, but the code related to bug 1741076 is not at fault. The reason bug 1741076 triggered this issue is just that before it, the downloads panel always opened when a download started, regardless of whether the window was active. But now, the panel is not supposed to open when the window is inactive.

The problem I'm seeing is that for some reason, the window is considered active the first time and not on subsequent drag-and-drops. So it should either never open in this context or always open in this context. But I suspect the reason that's inconsistent is a much lower-level issue, maybe with the drag service? What's happening in Windows 10?

It would not surprise me if window activation worked differently. Neil is the focus/drag-and-drop/windows-widget expert on this type of stuff so hopefully he can answer this.

Is the panel always opening or is it never opening? Its showing the first time might be caused by panelHasShownBefore.

Maybe Catalin can clarify.

If we want the downloads panel to always open when dragging and dropping into the window, then we could try to manually set that flag when the download is created by drag-and-drop. But there might be an independent reason for not wanting to open the downloads panel under these circumstances. Bug 1761265 prevents the panel from opening when the download was triggered by a user action that is expected to create a download.

I mean it all sounds to me like the only confusion (and/or a race condition) is about whether the window is active. If we fix that, we fix the bug.

Edit: By the way, I think the panelHasShownBefore flag ought to be used a bit differently now that alwaysOpenPanel exists. The former long predates the latter so I think I understand why they might conflict. panelHasShownBefore was supposed to keep the panel closed after the user closed it. But at that time, it was the only check. Now there are a bunch.

The original point of this flag was for the "old" behaviour of showing the panel the first time (ever) there was a download in a given Firefox profile, to help users discover the downloads panel.

I agree that we should not count the webextension cases where the panel also hides immediately.

But I think for e.g. user-iniated downloads, we probably do want to highlight the panel the very first time one occurs on a new user profile, even if we wouldn't otherwise open the panel. If we don't open the panel in that case, a new user would be confused about what happened / where their download has gone / ended up.

If you think that this flag needs changes to accomplish that, please file a separate bug. :-)

Comment 5

[Catalin Sasca, Desktop QA [:csasca]]

In reply to "Is the panel always opening or is it never opening? Its showing the first time might be caused by panelHasShownBefore."

The panel is opening only if initiating the first download on a clean profile. After that, it won't open anymore and only show the blue tick when the download is done exactly as in the attachment GIF.

Please let me know if you need any further infos about anything.

Comment 6

[Shane Hughes [:aminomancer]]

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

The original point of this flag was for the "old" behaviour of showing the panel the first time (ever) there was a download in a given Firefox profile, to help users discover the downloads panel.

I think I misunderstood something from bug 766263. For some reason I thought this pref was supposed to reset on each session, and then was later changed to persist between sessions. But upon closer examination I guess this has always worked the same way. So I see what you mean that if the panel has never been shown, we should show it no matter what except in that very narrow, unusual circumstance of a download launched by an extension without user interaction.

But I'm not convinced that would be a separate bug. I think that flag is actually the sole cause of the behavior Catalin reported. I did some testing and it seems the behavior on Windows 10 is actually the same as Catalin reported for other OSes. When I first tested it on my own profile, the panel didn't open. But I tried it on a new profile and it did open the first time. Upon setting browser.download.panel.shown to false, it opened again. Catalin's description sounds similar.

(In reply to Catalin Sasca, QA [:csasca] from comment #5)

The panel is opening only if initiating the first download on a clean profile. After that, it won't open anymore and only show the blue tick when the download is done exactly as in the attachment GIF.

Hey Catalin, from your description it sounds like the panel only opens once, on the first download of a clean profile. If you run this test again a couple times and confirm that the panel is not opening, can you then try setting browser.download.panel.shown to false in about:config, and try again? And you can try the same on Windows 10. I'm guessing you tested on Windows 10 but it wasn't a clean profile, so that pref was already set to true?

If that's right, then I think this is actually working exactly as expected. The panel is supposed to not open, but it does open for the first download of a profile, because of the way panelHasShownBefore is checked. If the top window is not active, but panelHasShownBefore is false, we're still going to open the panel. And then we set panelHasShownBefore to true, and then future downloads won't open the panel.

So I think we want to clean up that check a bit so that, if the window is not active or the download was launched by an extension without a user interaction, we skip opening the panel, in order to avoid the behavior where the downloads panel floats above other application windows. If it happens that the first download on a profile happens while the window is inactive, and we skip opening the panel, there will still be future opportunities to show the panel. We only set the flag to true if we actually open the panel. So instead of opening on the 1st download, it would just open on the 2nd download instead. Or whichever is the first download to happen while the window is active.

Edit: And actually, it turns out it's a lot harder to check specifically for the case where the download was started by an extension without user interaction, because we used the same flag for all the other cases as well. Right now, it's set up to skip showing the panel for any of those cases where openDownloadsListOnStart is false — regardless of panelHasShownBefore. So that includes the extension without user input case, but it also includes the cases covered by the other 2 bugs. So to patch that, we could create a new property only set by the downloads API. And then we could check if ((openDownloadsListOnStart && this.panelHasShownBefore) || !isHandlingUserInput) skip panel...

Comment 7

[Shane Hughes [:aminomancer]]

Attachment: Bug 1782205 - Don't open downloads panel if window is inactive. r=Gijs

Since bug 1741076, the downloads panel is already configured not to show
the downloads panel if the top chrome window is inactive. However, this
check doesn't matter if the downloads panel has never been opened before
in the profile (represented by pref browser.download.panel.shown). This
patch changes the check so that if the top window is not active, we
don't show the panel, no matter what.

Comment 8

[Catalin Sasca, Desktop QA [:csasca]]

Attachment: new_profile.mp4

Hey Shane. Yeah, we tried multiple times on every OS with clean profiles, and the behavior where it will open only the first time when a download is initialized is what happens except Windows 10 (I'll attach a video with this). The pref "browser.download.panel.shown" was OFF by default on every build we checked and after the first download the pref will switch to ON. And with the pref ON on Windows 10, the panel will still be shown

Comment 9

[Shane Hughes [:aminomancer]]

Catalin — I was able to reproduce the behavior in your new video. I think the reason it's always opening for you is because when you drag the link onto the toolbar, Windows brings the other window to the front. That makes it active, so it will always open the panel under those circumstances.

When I try it by just placing the windows side by side, the way it appeared in your earlier video, and don't drag the link onto the toolbar, I get the behavior as seen in the video. That is, if the profile is clean, the first download will open the panel, and all subsequent downloads will not open the panel.

Comment 10

[:Gijs (he/him)]

So it seems the issue here is caused by the fact that we do not activate/raise the window when we drop something on it.

Olli/Neil: is that something that we can change, or are there conventions / UX reasons that we don't want to change that? I found it a bit odd - clicking on a window normally brings it to the front on all OSes I know, so I'm somewhat surprised that dropping at the end of a drag doesn't. I'm probably missing something because I'm looking through a lens of it being inconvenient that the window isn't activated in this case...?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Whether to activate or not is something OS specific, no? Like on Linux moving mouse over may or may not activate the window depending on the settings.

I was testing "native" Gnome apps, like Gedit , and dnd drop doesn't seem to activate the target window.
Drop isn't a click. And I kind of like that behavior. I can drag stuff elsewhere, while keep the original window activated.

(random note, dnd on Linux Chrome seems to be broken in various ways. The dnd indicator being in totally wrong place etc.)

Comment 12

[Phabricator Automation]

Comment on attachment 9287893 [details]
Bug 1782205 - Don't open downloads panel if window is inactive. r=Gijs

Revision D153334 was moved to bug 1783643. Setting attachment 9287893 [details] to obsolete.

Comment 13

[:Gijs (he/him)]

I moved Shane's patch to bug 1783643 as it just avoids opening the panel in the first instance.

I think it's not great that we don't show the downloads panel at all as a result of drag and drop, though I think given comment #11 it makes sense not to do it while the window is inactive...

The remaining option I see is showing the dialog once a Firefox window does regain focus/activation. It'll be a bit tricky to implement to ensure we only show the panel in 1 window, and perhaps there needs to be some timeout (it's probably confusing if we show it 2 days later or w/e). But at least then in the common case where the user drops a URL and this results in (almost) no visible change in the window, if they click into the window, they will see what happened.

Does that seem reasonable, Neil & Olli?

Comment 14

[Neil Deakin]

I don't think we are doing anything specific to raise or not raise the window on a drop.

That said, other applications I tested on Windows 10 seem to raise the window when a drop occurs on it. Edge, opera, wordpad, explorer all do this. Not sure why Firefox isn't doing that.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Showing the dialog once FF window gains focus sounds reasonable to me.

(and it might be a separate bug if we don't activate window on drop on Windows)

Comment 16

[Shane Hughes [:aminomancer]]

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

The remaining option I see is showing the dialog once a Firefox window does regain focus/activation. It'll be a bit tricky to implement to ensure we only show the panel in 1 window, and perhaps there needs to be some timeout (it's probably confusing if we show it 2 days later or w/e). But at least then in the common case where the user drops a URL and this results in (almost) no visible change in the window, if they click into the window, they will see what happened.

Yeah I see what you mean. Scheduling the panel to open on window activate makes sense to me, but only if it's activated within a narrow time span like 15 seconds. I suspect if the limit is longer than that, it will come across like a bug. This might be helped by something I suggested in another bug though... I think that was bug 1740705. If we made the indicator animation more dramatic and added a numeric badge that counts downloads, we wouldn't need to worry so much about getting the user's attention by opening the panel.

It seems like this is coming up more frequently because we have a downloads panel instead of a downloads toolbar/pane. So I guess that sort of limits the contexts in which it's appropriate to open the downloads view. So for cases where the window is inactive, or another panel is already open, we could skip opening the panel and rely instead on the intensity of the visual/screen reader feedback. This would also help users who disable alwaysOpenPanel because they might not want the panel to interrupt their inputs, but still want a stronger feedback.

Also, if we think it's still worth opening the panel once the window is activated, I think a stronger indicator would help prevent that feeling like a bug. The badge could basically serve as an indicator that the panel is going to open once the window is activated, so users wouldn't be as likely to be surprised when it does open. That relationship between the badge and the panel opening on its own would be reinforced by the fact that the indicator attention state is cleared when the panel opens, so this automatic operation would reset the badge. And instead of only doing this on drag-and-drop, we could do this in general when a download happens when no Firefox window is active. (not sure how many sources that currently encompasses after the other patches we've made—maybe zero—but it would be good to have a default behavior in case any future sources are added)

Comment 17

[:Gijs (he/him)]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #15)

Showing the dialog once FF window gains focus sounds reasonable to me.

(and it might be a separate bug if we don't activate window on drop on Windows)

Having a separate bug for this sounds like a good idea if it's what other browsers do, per comment #14. I've filed bug 1783833 for that.

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

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

The remaining option I see is showing the dialog once a Firefox window does regain focus/activation. It'll be a bit tricky to implement to ensure we only show the panel in 1 window, and perhaps there needs to be some timeout (it's probably confusing if we show it 2 days later or w/e). But at least then in the common case where the user drops a URL and this results in (almost) no visible change in the window, if they click into the window, they will see what happened.

Yeah I see what you mean. Scheduling the panel to open on window activate makes sense to me, but only if it's activated within a narrow time span like 15 seconds. I suspect if the limit is longer than that, it will come across like a bug. This might be helped by something I suggested in another bug though... I think that was bug 1740705. If we made the indicator animation more dramatic and added a numeric badge that counts downloads, we wouldn't need to worry so much about getting the user's attention by opening the panel.

15 seconds sounds reasonable enough as a first guess for the timeout.

It seems like this is coming up more frequently because we have a downloads panel instead of a downloads toolbar/pane. So I guess that sort of limits the contexts in which it's appropriate to open the downloads view. So for cases where the window is inactive, or another panel is already open, we could skip opening the panel and rely instead on the intensity of the visual/screen reader feedback. This would also help users who disable alwaysOpenPanel because they might not want the panel to interrupt their inputs, but still want a stronger feedback.

Having other / stronger feedback does seem like it's worth discussing with product+UX but it's probably best done in a separate bug, if that sounds OK to you? It would have other repercussions and if there's one thing we've learned it's that whatever changes we make to downloads... everything breaks someone's workflow.

Comment 18

[Oana Botisan, Desktop QA]

Regression range:

• Last good: 2022-01-24
• First bad: 2022-01-25
• Potential regressor: Bug 1741076 - do not open the download panel if the window is not active