1140687 - "Remove Attachment" over a currently-uploading Filelink doesn't cancel the upload.

Status: RESOLVED FIXED

(Thunderbird :: FileLink, defect)

Description

[Diego Casorran [:diegocr]]

The summary says it all, basically.

When we use "Remove Attachment" over a Filelink attachment which hasn't finished uploading causes the attachment to get removed, but the file continues uploading in the background, and once it completes it throws the following exception:

Error: [Exception... "[JavaScript Error: "attachmentItem is null" {file: "chrome://messenger/content/messengercompose/MsgComposeCommands.js" line: 1248}]'[JavaScript Error: "attachmentItem is null" {file: "chrome://messenger/content/messengercompose/MsgComposeCommands.js" line: 1248}]' when calling method: [nsIRequestObserver::onStopRequest]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: resource://gre/components/nsBox.js :: nsBox__uploaderCallback :: line 155"  data: yes]
Source File: resource://gre/modules/Http.jsm
Line: 73

Using "Cancel Upload" and then "Remove Attachment" works fine. Hence, Thunderbird should send a cancel request to the Filelink component on removing (currently-uploading) attachments.

Comment 1

[Magnus Melin [:mkmelin]]

cancelFileUpload: http://mxr.mozilla.org/comm-central/source/mail/components/cloudfile/nsIMsgCloudFileProvider.idl#50

Comment 2

[Diego Casorran [:diegocr]]

This patch is supposed to fix the issue, however on re-packaging omni.ja with that new MsgComposeCommands.js it does nothing... I.e looks like RemoveSelectedAttachment() isn't even called (placed a Services.prompt there..) :-/

--- MsgComposeCommands-orig.js	2010-01-01 00:00:00.000000000 +0100
+++ MsgComposeCommands.js	2015-03-07 20:32:06.235980800 +0100
@@ -3768,6 +3768,10 @@ function RemoveSelectedAttachment()
         item.cloudProvider.deleteFile(
           file, new deletionListener(item.attachment, item.cloudProvider));
       }
+      if (item.cloudProvider && item.uploading) {
+        let file = fileHandler.getFileFromURLSpec(item.originalUrl);
+        item.cloudProvider.cancelFileUpload(file);
+      }
 
       removedAttachments.appendElement(item.attachment, false);
       // Let's release the attachment object held by the node else it won't go

Comment 3

[Diego Casorran [:diegocr]]

Attachment: MsgComposeCommands.js.diff v1

Got the problem from my former patch attempt, `originalUrl` is not set if the file hasn't finished uploading, so my block was never reached since it was throwing on the previous getFileFromURLSpec()

I've now merged both blocks into a single one, checking for originalUrl and falling back to `item.attachment.url` -- it have been tested and works like a charm.

Comment 4

[Magnus Melin [:mkmelin]]

Attachment: bug1140687_filelink_remove_attachment.patch

Made it an hg diff. Seems to work, I've sent it to try to see if there's anything unexpected - https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=08ea49ca96d0

Comment 5

[Magnus Melin [:mkmelin]]

Just unrelated failures, so lets land.

Comment 6

[Kent James (:rkent)]

Comment on attachment 8578257 [details] [diff] [review]
bug1140687_filelink_remove_attachment.patch

[Approval Request Comment]
We'll try it in nightly, then aurora, and consider later for esr31

http://hg.mozilla.org/comm-central/rev/c5a68209251d

Comment 7

[Kent James (:rkent)]

Comment on attachment 8578257 [details] [diff] [review]
bug1140687_filelink_remove_attachment.patch

https://hg.mozilla.org/releases/comm-aurora/rev/baeed39cdc3c

Comment 8

[Kent James (:rkent)]

Comment on attachment 8578257 [details] [diff] [review]
bug1140687_filelink_remove_attachment.patch

Not critical enough to uplift with 38.0 so close.