1750484 - "Delete" context menu should not remove the download history

Status: RESOLVED FIXED

(Firefox :: Downloads Panel, defect)

Description

[Masatoshi Kimura [:emk]]

When I downloaded a file and found that it was not useful for me, I'd delete the file, but I would not like to forget about that I used to download the file and download the useless file again. Removing the download history will revert the download link to the unvisited state, so I can't tell whether I used to download the file. Edge does not remove the download history.

Comment 1

[Shane Hughes [:aminomancer]]

The menuitem was intended to compensate for losing the ability to download files to the Temp folder and delete them automatically on application quit, so it deletes the session download and the history entry at the same time. Edge's menuitem says "Delete File" while Firefox's says "Delete." In Edge it requires multiple extra steps to clean up after a download, basically the use of 2 right clicks and 2 menuitem clicks. I don't think I would use the menuitem if it did that, since my goal was to implement a way of keeping the downloads clean.

I'm sure some users don't mind the clutter as much and would prefer to track the downloads so we can add another pref like browser.download.clear_history_on_delete. It could be an integer pref to support 3 options, 0 would replicate Edge's behavior, preserving the download in list and history; 1 would delete the download but preserve the history (which would mean download links remain visited but also that the download stays visible in the "all downloads" view); and 2 would maintain the current behavior, basically eradicating all trace of the download.

Comment 2

[Alice0775 White]

On Firefox96.0.1 with new profile, the temporary file will be automatically deleted from local HDD on Browser quit. And, the history entry of the file will remain in History as well as download entry in Library.

So, I vote for this bug +1.

Comment 3

[Shane Hughes [:aminomancer]]

The download entry in the library is the same as history. The library's all downloads view is just generated from a places query of downloads history. So there's no way to keep the download link "visited" without also leaving it in the library. I guess another property could be added but that would slow down the query I think. So that's another reason having 3 options is better than 2. I'm working on a patch but converting it from a session download into a history download during runtime seems complicated

Comment 4

[Masatoshi Kimura [:emk]]

The menuitem was intended to compensate for losing the ability to download files to the Temp folder and delete them automatically on application quit, so it deletes the session download and the history entry at the same time.

Comment #2 points out this claim is false. Firefox 96.0.1 deletes temporary downloads from the session download on exit, but it does NOT remove the history entry. So if the "Delete" menu item is intended to compensate for that, it should not remove the history entry. Personally I don't care about removing it from the session download.

In Edge it requires multiple extra steps to clean up after a download, basically the use of 2 right clicks and 2 menuitem clicks.

To be fair, Edge has a Trash icon in the download list. You don't have to open context menu to delete the file. Just click the icon. Of course it still takes more steps if you also want to remove the history. But again, Firefox 96.0.1 does not remove the history entry, either.

Comment 5

[Shane Hughes [:aminomancer]]

I'm the one who made the menuitem, I think I'd know my own intentions better than you. I made it with the express purpose of making it easier to keep my downloads clean. I made it in an autoconfig script last year, published it, then turned it into a patch and asked Gijs to submit it for me. As you can see in the description, the motivation is "being able to clean up these files from the context menu" which is "important since the ability to "temporarily" download files with Firefox is being removed as part of bug 1733587 to reduce the risk of data loss."

I really don't want to argue about this further, I volunteered my time for this over the holiday and I've spent the last several hours putting even more effort into yet another patch, to respond to your request. The least you could do is not accuse me of lying about my own work

Comment 6

[Shane Hughes [:aminomancer]]

Attachment: Bug 1750484 - Option to make the "Delete" download menuitem not remove history. r?Gijs

Comment 7

[:Gijs (he/him)]

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

I really don't want to argue about this further, I volunteered my time for this over the holiday and I've spent the last several hours putting even more effort into yet another patch, to respond to your request. The least you could do is not accuse me of lying about my own work

Hey Shane - to be clear, I've really appreciated all your help here. Of course I don't know exactly what :emk's intention was, but the way I read comment 4, I think that they didn't mean to accuse you of lying about your motivation or anything else. I think the point was only that in release Firefox 95/96, with the download improvements pref off, if a user downloads a file and in the dialog chooses "open with [app]", the file gets put in the temp folder and then deleted - but the history entry does not get deleted without other user action. The "delete" context menu deletes both the file and the history, so the result is a subtle difference with the previous behaviour.

Comment 8

[Shane Hughes [:aminomancer]]

Thanks. Sorry to sound defensive. I do see that it's a different behavior. I only mentioned it to explain why I thought adding a menuitem was important. I don't feel like gExternalAppLauncher.deleteTemporaryFileOnExit is the same kind of behavior. It isn't a user-prompted action at all, and it happens under totally different circumstances, so I don't think it should be dictating the behavior of new menuitems.

The removal of that behavior made the new menuitem seem more important, but now that our "cleanup" behavior is user-activated rather than automatic, it makes sense for it to clean up the download altogether, not just the file (at least by default). But if I'm really in the minority then my tastes shouldn't be inflicted on everyone else.

Regarding the default behavior, I guess we could always investigate that with telemetry, but adding telemetry on top of a pref seems heavyhanded for such a small thing. I was already worried that my new patch would be rejected because a whole pref just for a menuitem may seem like overkill. If I seem defensive it's probably because I don't want the pref addition to be rejected and the behavior to be changed to not remove history, since that's my personal preference

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4c2fd383406
Option to make the "Delete" download menuitem not remove history. r=Gijs,mhowell

Comment 10

[Marian-Vasile Laza]

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