1755570 - File deleted from Downloads Panel contextual menu should have a different UI from the one deleted from disk

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, enhancement)

Description

[Petruta Horea [:phorea], Desktop Test Engineering]

Affected versions

• Firefox 98 beta 4
• Nightly 99.0a1

Affected platforms

• all

Steps to reproduce

1. Download a small file from link
2. Wait for the download to complete
3. Right click on the file from Downloads Panel and select the Delete option

Expected result

• Downloaded file is deleted and the UI indicates that the action was performed using the browser

Actual result

• File moved or missing is displayed under the file name, same as when the file is removed from disk.

Regression range

• Not a regression

Comment 1

[Shane Hughes [:aminomancer]]

What should the string say? File deleted? File deleted by user?

Comment 2

[:Gijs (out for now; he/him)]

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

What should the string say? File deleted? File deleted by user?

I think just "File deleted" would be fine.

Comment 3

[Shane Hughes [:aminomancer]]

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

I think just "File deleted" would be fine.

Do you mean we should only change the string so Firefox just assumes any file that's missing was "deleted" instead of "moved or missing"? Or do you mean we should add a new string that's exclusive to downloaded files deleted by Firefox (and logic for ascertaining that)?

For the latter, presumably we need to add a flag to the session download object when user activates the "Delete" menuitem. (Maybe a new "legacy state" value because that parameter is already cached? The term legacy state makes it sound like it's gonna be obsolete or something though)

But then it'll get lost between sessions so I guess we also need to update the metadata in the history, so when the "all downloads" view retrieves the DownloadHistory list, it remembers which downloads we deleted.

Comment 4

[Shane Hughes [:aminomancer]]

Attachment: Bug 1755570 - Add download deleted state to UI. r=Gijs

Add a new property to downloads such that downloaded files deleted from within
Firefox (currently just by the context menu item) are marked "File deleted"
instead of "File moved or missing" (this adds a new translation). Also refactor
downloadsCmd_deleteFile and downloadsCmd_cancel to clean up some related issues.
Add a new download history metadata property so that Firefox can persist this
"File deleted" state between sessions.

This should resolve bug 1755570 as well as bug 1755728. Bug 1755729 is a
separate issue, more like an enhancement, because missing/moved downloads have
never allowed resume/retry. They couldn't be resumed as they were stopped, not
paused. They could conceivably be retried but this would be adding a whole lot
of new core download logic, since the target and saver are in a very different
state for a stopped download with a deleted file than what they'd be if the
download was merely canceled.

So, this patch won't give the user an opportunity to resume/retry deleted
downloads. Previously we had a problem where we made it look like you could
retry a download if you used the "delete file" command while the download was in
progress. This patch will make sure that using the "delete file" command
actually finalizes the download. So it will just fix the immediate problem where
some menuitems and buttons simply don't work in edge cases. A later patch can
implement a new affordance that will allow "retrying" downloads that were
already downloaded/interrupted and deleted, whether from within Firefox or not.

Comment 5

[Adrian Florinescu [:aflorinescu]]

Given the timing of the last Firefox 98 Beta, I would assume that this fix won't make it time. Shane, confirm please?

Comment 6

[Shane Hughes [:aminomancer]]

It's in a fully functional state, just needs another review. I was thinking about refactoring it further so it wouldn't require as much explanation in code comments, but that would probably go in a separate patch since it'd be a lot of code. Better to just land it asap before anyone notices imo. But I'll leave it to Gijs as to whether there's anything I missed in this latest revision

Comment 7

[:Gijs (out for now; he/him)]

We built the last few betas for 98 and this involves locale changes so either way it isn't making beta 98 - it will probably make 99 though, I don't think there's much left though I might not be able to review until tomorrow.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a290f5c38535
Add download deleted state to UI. r=Gijs

Comment 9

[Cristian Tuns]

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

Comment 10

[Adrian Florinescu [:aflorinescu]]

Let's put this on our radar to verify.

Comment 11

[Adrian Florinescu [:aflorinescu]]

Verified as fixed on Firefox 100.0b8 and Nightly 101.0a1 (2022-04-20) on Windows 10, Ubuntu 22 and Mac 11.