1744709 - File saved via double-click in the attachment area doesn't show in the list of saved files (Ctrl+J)

Status: REOPENED

(Thunderbird :: Mail Window Front End, defect)

Description

[Rachel Martin]

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.

Comment 1

[Rachel Martin]

Likely a regression of bug 1737711.

Comment 2

[Rachel Martin]

Maybe something like this or this is missing here? Looks like the transfer stuff creates the download (entry) here (line 413).

Comment 3

[Rachel Martin]

https://github.com/Betterbird/thunderbird-patches/blob/main/91/bugs/1744709-save-double-click-download.patch

Comment 4

[Rachel Martin]

TB 91 is affected.

Comment 5

[Rachel Martin]

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.)

Comment 6

[Rachel Martin]

https://github.com/Betterbird/thunderbird-patches/blob/main/102/bugs/1744709-save-double-click-download.patch

Comment 7

[Thomas D. (:thomas8)]

Attachment: Bug 1744709 - Create a download item when saving attachments to file via double-click. r=#thunderbird-reviewers

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.

https://github.com/Betterbird/thunderbird-patches/blob/main/102/bugs/1744709-save-double-click-download.patch

Comment 8

[Thomas D. (:thomas8)]

(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!

Comment 9

[Rachel Martin]

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.

Comment 10

[:aceman]

Attachment: 1744709-save-double-click-download.patch

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

[b2]

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

[:aceman]

Attachment: 1744709-save-double-click-download.patch v2

Thanks, updated the patch.

Comment 13

[Magnus Melin [:mkmelin]]

Comment on attachment 9297706 [details] [diff] [review]
1744709-save-double-click-download.patch v2

Looks good to me, r=mkmelin

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c8e4b4cb97f8
Create a download when saving to file via double-click.

Comment 15

[Geoff Lankow (:darktrojan)]

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.

Comment 16

[Magnus Melin [:mkmelin]]

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.