Add a context menu on a downloaded file in download panel to delete the file from local file
Categories
(Firefox :: Downloads Panel, enhancement)
Tracking
()
People
(Reporter: alice0775, Assigned: aminomancer)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Whiteboard: [fidefe-mr11-downloads])
Attachments
(1 file, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
Details |
Now new download panel would not delete temporary file when quit Browser.
So, please add a context menu to manual delete the temporary file from Browser UI.
Steps to Reproduce:
- Set PDF use Windows default application from about:preferences#general
- Open a pdf from web site.(e.g. https://www.kenken.go.jp/becc/documents/building/Definitions/modelBulding5000_v1.pdf)
--- PDF application(Adobe Acrobat) will open as expected - Close the PDF application.
- Open Download panel
--- There are no way to delete the temporary file from the Download panel - Quit Browser
Actual results:
There are no way to delete the temporary file from the Download pane.
And also, the temporary file would not be deleted automatically due to new download design behavior.
Expected results:
Add a context menu on a downloaded file in download panel to delete the temporary file from HDD.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 1•3 years ago
|
||
Based on conversations with product, we may add this but don't think we need this in order to ship the feature.
Assignee | ||
Comment 2•3 years ago
|
||
I can work on this. I also think we should add a "Move to..." menuitem that will basically give the sense of retroactively choosing "Save as..." in cases where the download system automatically saved the file to user's default directory. That way you can get some control back without requiring extra steps when you don't need to save in a particular directory. I'm not sure the safest way to implement this but I'll experiment with it and treat both menuitems as part of the same patch
Comment 3•3 years ago
|
||
(In reply to aminomancer from comment #2)
I can work on this. I also think we should add a "Move to..." menuitem that will basically give the sense of retroactively choosing "Save as..." in cases where the download system automatically saved the file to user's default directory. That way you can get some control back without requiring extra steps when you don't need to save in a particular directory. I'm not sure the safest way to implement this but I'll experiment with it and treat both menuitems as part of the same patch
Please don't try to do more than 1 thing in one patch (and 1 bug). I am much less convinced that "move to..." is a useful menuitem, and we should at least have some product + UX conversations about it. If you want to help, try for a patch with the delete item on its own first. We should have a separate bug for the other item.
Assignee | ||
Comment 4•3 years ago
|
||
Hey @Gijs do you have any input on this? I'm trying to pick the safest delete method. In other methods, the downloads UI just calls new FileUtils.File(path)
but they aren't using that to delete anything, so there's no risk of data loss anyway. With a delete menuitem, if we only use the path and the user overwrites the file at that path, we can end up deleting a file that doesn't genuinely correspond to the download entry. I guess I could do some kind of checksum but it seems a little janky to add all that for just a context menu item.
Then again, it would be inconsistent if we didn't delete the file in such a case, because every other file integration here only seems to care about path. Like if I delete a file, open the panel, the download is disabled/grayed out. Then if I make a new file in the same path with the same name, and open the panel again, the download item is restored to its former glory. And all the various commands work on it as you'd expect. So I guess it'd be a bit odd to do something different for a "delete file" command.
But if the concern about data loss is great enough, and there's an easy way to confirm the pedigree of the file, maybe we should implement it in all those methods that currently only check path. Such that making a new file with the same path would not re-enable the download item. That would make this a much bigger decision so I can't proceed much further on my own.
Comment 5•3 years ago
|
||
(In reply to aminomancer from comment #4)
Hey @Gijs do you have any input on this? I'm trying to pick the safest delete method. In other methods, the downloads UI just calls
new FileUtils.File(path)
but they aren't using that to delete anything, so there's no risk of data loss anyway. With a delete menuitem, if we only use the path and the user overwrites the file at that path, we can end up deleting a file that doesn't genuinely correspond to the download entry. I guess I could do some kind of checksum but it seems a little janky to add all that for just a context menu item.Then again, it would be inconsistent if we didn't delete the file in such a case, because every other file integration here only seems to care about path. Like if I delete a file, open the panel, the download is disabled/grayed out. Then if I make a new file in the same path with the same name, and open the panel again, the download item is restored to its former glory. And all the various commands work on it as you'd expect. So I guess it'd be a bit odd to do something different for a "delete file" command.
Right, I think this is enough of an edgecase that just deleting whatever exists at that path makes sense, also because...
But if the concern about data loss is great enough, and there's an easy way to confirm the pedigree of the file,
... there isn't, unfortunately, an easy way to do that. All the different OSes we support have different (sometimes filesystem-dependent) ways of tracking files, and we don't currently use them in downloads code (see also e.g. bug 1746386). For now, deleting what is at the path seems fine.
Assignee | ||
Comment 6•3 years ago
|
||
Alrighty I still haven't figured out mach so here's my patch if you wanna review it when you get back.
Assignee | ||
Comment 7•3 years ago
|
||
New "Delete" menuitem displays when the context menu is opened on a DownloadElement whose target file or partFile exists. On activation, it deletes the target file and clears any part or temporary data. Then, if it was the only download in the panel, it resets the download indicator's attention state.
Updated•3 years ago
|
Assignee | ||
Comment 8•3 years ago
|
||
Updated•3 years ago
|
Comment 10•3 years ago
|
||
bugherder |
Comment 11•3 years ago
|
||
Should we mention that feature in our release notes? Thanks
Comment 12•3 years ago
|
||
(In reply to Pascal Chevrel:pascalc from comment #11)
Should we mention that feature in our release notes? Thanks
Maybe. We're tweaking the behaviour in bug 1750484 so I'll wait with requesting one until that's hashed out.
Comment 13•3 years ago
|
||
Release Note Request (optional, but appreciated)
[Why is this notable]: New feature in 98 relating to the downloads changes launched in 97.
[Affects Firefox for Android]: No
[Suggested wording]: You can now delete downloaded files directly from the download panel and other download views using the context menu.
[Links (documentation, blog post, etc)]: n/a
Comment 14•3 years ago
|
||
Note added to Nightly 98 release notes, thanks.
Comment 15•3 years ago
|
||
Adding qe+ and reminder to add in testsuite for fx 98.
Comment 16•3 years ago
|
||
This enhancement seems to be working on Firefox 98.0b4(20220213185901) as well as Nightly 99.0a1(20220213214259). There is now a "Delete" option implemented when right-clicking on the temporary file from the Downloads panel. Checked on macOS Big Sur 11, Linux(Ubuntu 20.04) and Win11.
Updated•3 years ago
|
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Description
•