1749784 - Overhaul DownloadsIndicatorDataCtor so it resets its attention state (on its own) when all downloads in the view have been removed

Status: RESOLVED FIXED

(Firefox :: Downloads Panel, defect, P3)

Description

[Shane Hughes [:aminomancer]]

In bug 1745624 we have added a menuitem to the downloads panel context menu that can delete a downloaded file. We were discussing that this menuitem and the delete ("Remove from History") menuitem do not reset the indicator's attention state. Normally the indicator attention state is reset when opening the panel or the all downloads view. But it's possible to have the all downloads view already open when a download's state changes.

In this condition, user can right click the download in the all downloads view and delete or remove it from history. After that has been done to every download element that also exists in the panel's list, the view will be empty. No tracked downloads remaining. When this happens, the download indicator's attention state is not reset, which is probably confusing for the user.

The DownloadsIndicatorDataCtor already listens for "onDownloadRemoved" notifications so my preliminary thought is this could be resolved by just updating this to add

if (!this._hasDownloads) {
  this.attention = DownloadsCommon.ATTENTION_NONE;
}

However, I had another thought too. Usually in this condition, only one of the downloads in the view is responsible for setting the attention state. Presumably, the user downloaded some files and then opened the all downloads view, resetting the attention state. But then another download's state changed, setting the attention again.

Now in this scenario there are multiple downloads tracked by the panel (which is not open) but only one is responsible for the attention state. So, deleting or removing that particular download should singlehandedly reset the attention state. But that requires DownloadsIndicatorDataCtor to track the download(s) responsible for setting its attention state.

That would be sort of like how AppMenuNotifications get added to a queue and the badge is updated in sequence with that queue. Each download would be mapped to an attention state and a state change for a given download would result in the download either being pushed to the top of the queue or being removed (in the case of ATTENTION_NONE). So we could have a situation like this:

Download 1: ATTENTION_SUCCESS,
Download 2: ATTENTION_WARNING,
Download 3: ATTENTION_SEVERE

Download 1 being removed from the list would remove it from the map and the indicator would be updated to ATTENTION_WARNING as Download 2 is now at the top of the list. Conversely, Download 3 being removed from the list would have no immediately visible effect since it's not responsible for the current indicator. But if Download 3 was removed from the list, and then Download 1 and Download 2 were removed from the list, instead of updating attention to ATTENTION_SEVERE it would update to ATTENTION_NONE, since there would be nothing left in the map.

So, any opinions on whether we should go with 1) the simple solution of only resetting attention when the downloads panel is fully empty (in addition to resetting it when the panel is opened/closed), or 2) the more complex solution of mapping downloads to attention states, using the top value in the map as the indicator's state, and clearing the entire map when attentionSuppressed is set?

Comment 1

[:Gijs (he/him)]

I'm curious what Marco thinks about the question in comment 0.

Comment 2

[Sam Foster [:sfoster] (he/him)]

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

Download 1 being removed from the list would remove it from the map and the indicator would be updated to ATTENTION_WARNING as Download 2 is now at the top of the list. Conversely, Download 3 being removed from the list would have no immediately visible effect since it's not responsible for the current indicator. But if Download 3 was removed from the list, and then Download 1 and Download 2 were removed from the list, instead of updating attention to ATTENTION_SEVERE it would update to ATTENTION_NONE, since there would be nothing left in the map.

My understanding is that the attention state on the download panel/indicator should always represent the most severe state of the active downloads. I.e. in the case where there are 3 downloads in the list, Download 1 is ATTENTION_NONE, Download 2 ATTENTION_WARNING, Download 3 ATTENTION_SEVERE, the state should be ATTENTION_SEVERE. When Download 3 is removed or its flag cleared, the state becomes ATTENTION_WARNING for Download 2. I ran into this in bug 1723094, and I think that with this chance this proposal might be a way to address that bug as well.

Comment 3

[Shane Hughes [:aminomancer]]

Attachment: Bug 1749784 - Overhaul DownloadsIndicatorDataCtor to track each download's attention state individually. r?Gijs

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9d6d755aa1c
Overhaul DownloadsIndicatorDataCtor to track each download's attention state individually. r=Gijs

Comment 5

[Marco Bonardo [:mak]]

I agree with Sam, and it is my understanding that's what the patch is doing, well done.

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/c9d6d755aa1c