File saved via save attachment in the attachment area doesn't show in the list of saved files (Ctrl+J)
Categories
(Thunderbird :: Mail Window Front End, defect)
Tracking
(thunderbird_esr102+ wontfix, thunderbird115 fixed)
People
(Reporter: rachel, Assigned: mkmelin)
References
(Regressed 1 open bug, Regression)
Details
(Keywords: regression)
Attachments
(1 file, 5 obsolete files)
48 bytes,
text/x-phabricator-request
|
wsmwk
:
approval-comm-beta+
|
Details | Review |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Firefox/91.0
Steps to reproduce:
When saving a file via double-click from the attachment area, the file doesn't appear in the list of saved files any more. Visible in TB 95 beta.
Reporter | ||
Comment 1•2 years ago
|
||
Likely a regression of bug 1737711.
Reporter | ||
Comment 2•2 years ago
|
||
Maybe something like this or this is missing here? Looks like the transfer stuff creates the download (entry) here (line 413).
Reporter | ||
Comment 3•2 years ago
|
||
Assignee | ||
Updated•2 years ago
|
Reporter | ||
Comment 5•2 years ago
|
||
Geoff, maybe TB should eventually take this patch before 102 since it's a regression from the "open attachment reshuffle". Likely needs rebasing to trunk. (Strange that no user complained since we got a complaint very early on.)
Reporter | ||
Comment 6•2 years ago
|
||
Comment 7•2 years ago
|
||
We want to create an item in the downloads history aka "Saved Files" every time
when an attachment get saved, so that users won't have to dig for that file
again.
Updated•2 years ago
|
Comment 8•2 years ago
|
||
(In reply to Rachel Martin from comment #5)
Geoff, maybe TB should eventually take this patch before 102 since it's a regression from the "open attachment reshuffle". Likely needs rebasing to trunk. (Strange that no user complained since we got a complaint very early on.)
Thanks Rachel for following up on this and offering your patch! This is a useful fix for an everyday basic workflow.
Geoff is on holiday for the next two weeks. I've applied this in my new (Artifact) Build Environment, and it does fix the problem without any side effects afasics. Patch uploaded for review. Awesome!
Reporter | ||
Comment 9•1 year ago
|
||
It would be preferable to keep passing in the path, instead of adding more reliance on nsIFile
nsITransfer,init()
requires a nsIURI
parameter, so it doesn't make sense to pass the nsIFile.path
instead of nsIFile
just to create another nsIFile
inside the function to create the URI.
Reporter | ||
Updated•1 year ago
|
![]() |
||
Comment 10•1 year ago
•
|
||
I have simplified the patch to keep using the file path instead of nsIFile. It seems to work for me on Linux. Please test on Windows, whether the constructed URI is correct and the downloads list opens the correct folder where the file was stored.
Comment 11•1 year ago
|
||
Comment on attachment 9297023 [details] [diff] [review] 1744709-save-double-click-download.patch Review of attachment 9297023 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, works on Windows. ::: mail/base/content/msgHdrView.js @@ +1554,5 @@ > async function saveToFile(path) { > let buffer = await new Promise(function(resolve, reject) { > NetUtil.asyncFetch( > { > uri: Services.io.newURI(url), Please use `sourceURI` here like in our original patch.
![]() |
||
Comment 12•1 year ago
|
||
Thanks, updated the patch.
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Updated•1 year ago
|
Assignee | ||
Comment 13•1 year ago
|
||
Comment on attachment 9297706 [details] [diff] [review]
1744709-save-double-click-download.patch v2
Looks good to me, r=mkmelin
Comment 14•1 year ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/c8e4b4cb97f8
Create a download when saving to file via double-click.
Updated•1 year ago
|
Updated•1 year ago
|
Comment 15•1 year ago
|
||
This breaks the test mail/test/browser/attachment/browser_openAttachment.js on Linux/Mac systems. Looks like the file permissions aren't what was expected.
Assignee | ||
Comment 16•1 year ago
|
||
Backout: https://hg.mozilla.org/comm-central/rev/600f0bbcf715659acd53bcd39616db5118b63b62
I suspect https://searchfox.org/comm-central/rev/0f01f315a67ccf35334446b28697032348e3b117/mail/base/content/msgHdrView.js#1643 isn't operating on the "right" file anymore.
Comment 17•5 months ago
|
||
Rebased patch. Magnus, can you please check the test failures, see comment #15.
Comment 18•4 months ago
|
||
Rebased after prettier changes.
Assignee | ||
Comment 19•3 months ago
|
||
Coming back to this bug, I don't think the bug as filed is 100% correct. We should only save to that list when the it's a save. If you open a temp file it shouldn't get into that list.
Assignee | ||
Comment 20•3 months ago
|
||
Assignee | ||
Updated•3 months ago
|
Assignee | ||
Updated•3 months ago
|
Assignee | ||
Updated•3 months ago
|
Comment 21•3 months ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #19)
We should only save to that list when the it's a save. If you open a temp file it shouldn't get into that list.
We had feedback that in TB 78 the file was put into the list even if it was opened via double click and temporarily saved. Apparently the user preferred to re-open the attachment from the "Saved Files" menu rather than looking to the message again. But sure, referring to the temp directory which is subject to garbage collection isn't a stable solution.
Assignee | ||
Updated•3 months ago
|
Comment 22•3 months ago
|
||
Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/80da1fdec431
Make saved attachments show up under Saved Files. r=aleca
Comment 23•3 months ago
|
||
Comment on attachment 9340002 [details]
Bug 1744709 - Make saved attachments show up under Saved Files. r=#thunderbird-reviewers
[Triage Comment]
Approved for beta
Comment 24•3 months ago
|
||
bugherder uplift |
Thunderbird 115.0b6:
https://hg.mozilla.org/releases/comm-beta/rev/c55396921b12
Updated•3 months ago
|
Description
•