Now that Thunderbird supports BigFiles/FileLink, we could very well make use of it to offer makeshift file attachments. This is actually quite easy, I have a work in progress patch that implements the basic features. I just have to clean up error reporting, finish the UI work and some improvements on how the async operations are handled. Something I feel quite uncomfortable with is saving the cloud provider account key and original filename on the attachment object. It will be serialized as: ATTACH;X-MOZ-CLOUD-PROVIDER=account1;X-MOZ-CLOUD-FILENAME=08sim.pdf:http:/ /db.tt/FUsxqHrR I could likely get rid of the provider parameter, but how would I get the filename? Also, deleting attachments directly on the remote store only works within one event dialog session, as there is no method to delete a file using its public URL.

I've decided to push the strings for this bug in comm-central changeset dd788105a464 before the merge, this has been work in progress long enough. Lets get this show on the road, I promise^H^H^H^H^H^H^Hthink I'll fix the patch shortly after the merge.

There we go, here is the patch for comm-central. Please test thoroughly. There is one issue remaining, I use the FILENAME attribute from http://tools.ietf.org/html/draft-daboo-caldav-attachments-01 but I haven't yet found a good solution for the security issue. Theoretically there could be evil crafting something like this: ATTACH;FILENAME=your_bill.pdf:http://example.com/evil.scr to trick the user into downloading the file. I do have the tooltip set to the full uri, but I don't know if thats enough or how else we can prevent this.

Comment on attachment 644626 [details] [diff] [review] Fix - v1 Review of attachment 644626 [details] [diff] [review]: ----------------------------------------------------------------- Overall it works well, but there are a few issues: - The image in the attachment list doesn't show up for me until the file upload is completed, the loading icon doesn't ever appear. - When the upload fails, I don't see the error image either, the file isn't removed from the attachment list, and the following appears in the error console: Error: TypeError: documentLink.removeItem is not a function Source file: chrome://calendar/content/calendar-event-dialog.js Line: 1819 - With Storage calendar items, the file URL appears in the attachment list instead of the file name after restarting Thunderbird. ::: calendar/base/content/dialogs/calendar-event-dialog.js @@ +1782,4 @@ > return name; > } > > +function uploadCloudAttachment(attachment, cloudProvider, item) { Using "item" as the argument name is a little confusing here - when it is used in other functions in the file it usually refers to the calendar item, while in this case it refers to a list item element. A function comment might be useful here also.

(In reply to Matthew Mecca [:mmecca] from comment #3) Fixed all issues, except for: > - With Storage calendar items, the file URL appears in the attachment list > instead of the file name after restarting Thunderbird. > This is very unfortunate. Fixing the problem makes me want to fix so many other things though: * the prop is lost, because the storage calendar doesn't save it * this makes me want to change the cal_attachments table to save the whole icalString, but there is no setter for this in calIIcalProperty * this makes me want to add a such setter, but that means changing our C++ code * this makes me want to get rid of our C++ code * it also makes me want to rewrite the storage provider so the whole item is in one blob and only indexed values are saved in extra fields so you see, I think its easier to just accept this problem for now :) > Using "item" as the argument name is a little confusing here - when it is > used in other functions in the file it usually refers to the calendar item, > while in this case it refers to a list item element. A function comment > might be useful here also. Changed to listItem and added a function comment. Do you think you could review this in the next few days? I'd like to push this down to beta since we also have the strings there.

Comment on attachment 655288 [details] [diff] [review] Fix - v2 r=mmecca

Pushed to comm-central changeset 58cd3591ca84

Backported to releases/comm-aurora changeset 4ab2340c2d04

The comm-aurora patch obviously doesn't have the string changes.