378046 - Mail composition: opening/editing attached file sometimes unexpectedly opens/edits original file (only if attachment was added via TB OR drag-and-drop (non-MAPI) AND draft has never been closed yet): MAPI and non-MAPI behaviour should be consistent

Status: NEW

(Thunderbird :: Message Compose Window, defect)

Description

[Thomas D. (:thomas8)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Version 2.0.0.0 (20070326)

When composing new mail in TB (currently at 2.0), opening an attachment from the attachment pane of the message compose window will unexpectedly edit the original file (in the location from where it was taken) instead of opening/editing a snapshot COPY of that file. This bug occurs only if the following conditions are met:
- the attachment was added starting out from TB's UI OR using drag and drop
(whereas if you start out from Windows Explorer > Send to, TB will correcty create a snapshot copy of the attached file)
- the draft mail window has never been closed and reopened (which also triggers TB to create its own snapshot copy of the attached file).


Reproducible: Always

Steps to Reproduce:
1. Compose mail, add attachment via TB UI or drag and drop (NOT via Explorer, Send to)
2. Double-click on attached file in message composition's attachment pane to open it with external app like notepad etc.
3. 
Actual Results:  
Thunderbird opens the original file from its original location in my file system for editing

Expected Results:  
Thunderbird should open its own snapshot copy of the attached file from temporary directory

You might argue this is a question of philosophy: What does "adding a file attachment" mean? In principle, there are two options:
1) adding a dynamic pointer to the ORIGINAL file in the user's file system
2) adding a snapshot COPY of that file as it is at the time of attaching (for which the snapshot copy is saved in some temporary location for thunderbird use only; for technical reasons, this might be realized, too, with a pointer to that file, but it's a pointer to thunderbird's copy of the file, not to the original)

Until today, I always believed Thunderbird takes a snapshot copy of my attached file, and editing the attachment from within TB means editing the snapshot. In fact, this holds true if you attach your file(s) using Explorer's Send-To command: Open it from within TB and you are working on thunderbirds COPY of your file. HOWEVER, if you use drag and drop, or add your attachments from inside via thunderbird's respective commands, things are different: Open it from within TB and you will be working on your ORIGINAL file (which causes another open bug*, namely that TB is sometimes unable to save drafts). Whereas we can argue about which of the options as outlined above is "better", surely the METHOD of adding an attachment should not lead to completely different handling.

But don't you think that's the end of confusion. Depending on HOW you added your attachment, and from WHERE you open it (TB main window or stand-alone message window), there as many as 5 different locations/names where Thunderbird may search for or (temporaliy) keep your attachment:
- in your file's original folder
- in C:\Documents and Settings\User\Local Settings\Temp\moz_mapi\
- in C:\Documents and Settings\User\Local Settings\Temp\
- under any of these names: test.txt, test-1.txt (that's the FIRST instance, so it's not to avoid overwriting another file), nsmail-1.txt
- as a mime attachment inside your mail
I do see there are technical and historical reasons for all this, but the current implementation is really too complex and very unpredictable.

E.G., try this:
1) Open a saved(!) draft mail with text.txt attachment
--> TB will create a nsmail-1.txt tmp file in C:\Documents and Settings\User\Local Settings\Temp\ (hovering the mouse over attachmed file in mail composition's attachment pane reveals the temporary location)
2) open windows explorer and edit and save temporary attachment file (nsmail-1.txt) with external app like notepad
Now comes the irritating part, scenario a)
3a) close your draft window without saving
--> Nothing happens. No questions. Temporary file including your manual changes is simply deleted, so changes are lost, even though the tooltip claims that this file is currently attached.
Irritation, scenario b)
- do steps 1 and 2, and then continue like this:
3b) save your draft mail before closing it
--> surprise: Thunderbird saves the temporary file with your manual changes before deleting it.

Personally, I believe that it will be much more intuitive for users to think of attachments as a snapshot copy of the original file (option 2), and not a pointer to the current version of the file (option 1). There are a couple of bugs around this issue.

Comment 1

[Thomas D. (:thomas8)]

This is a dataloss bug, therefore critical. It is still there in TB3a1 shredder. Please anyone confirm (using steps BELOW)? Should be blocking?

I admit this bug might turn out a can of worms (which led to the long posting above...), but someone will need to fix it anyway. Here's the initial scenario for confirming the bug:

Steps to reproduce / Test Case:
--------------------------------
(Do the following steps rather quickly as draft autosaving might spoil the experience)
1. Create new sample.txt file in your Windows Desktop folder, open it, type in text "hello world" and save file (using external app like notepad.exe). Close Notepad.exe to ensure file is free.

2. Compose mail and add sample.txt from Desktop as attachment using TB's "Include an attachment" button (the method matters! see above for details)
(Do not close or save your draft, and make sure it doesn't autosave in between) 

3.) Right after adding attachment, double-click on the attachment from within TB's attachment pane (composition window) to open with notepad.exe, type text "asdf" after "hello world", save file in notepad and close notepad.

4. Open sample.txt directly from your Windows Desktop folder and be shocked that you have just edited your original file (on Desktop) even though you thought you were just working on your mail's attachment (as a snapshot COPY of your original file).

See the problem? When you're through, start playing with saving your draft, closing your draft and reopening, then editing draft's attachment from TB's attachment pane, and find a whole lot of strange things happening (as I attempted to start describing in the first posting of this bug).

N.B. nsmail.txt is not a very descriptive filename when editing attachments (after they have been attached), and the days of netscape are gone for quite some time... :)

Regards

Comment 2

[David Ascher (:davida)]

I appreciate the confusion, but this is far from a blocker.  I'm also not convinced it warrants the dataloss keyword.

Comment 3

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #0)
Thanks for your detailed analysis report on current confusing Tb's behaviour in "attach file".

FYI.
When file send via MAPI, there are 2 kind of bugs;
 (1) Bug 344034, Bug 356808
     If file of same file name already exists in MOZ-MAPI, file is not copied.
     Thus, it's impossible to send two versions of file at same time.
 (2) Bug 356919, Bug 405717
     In some situations, copy in MOZ-MAPI is not deleted after mail send.
     Then problem of (1) occurs. Thus, oldest version is always sent.
Please be careful when testing/discussing Tb's attachment related behavior via MAPI.  

Comment 4

[Thomas D. (:thomas8)]

Thanks, Wada, for illustrating some of the problems that are related to this bug. a) Will any of the fixes for the bugs listed in your comment #3 fix this bug 378046?
b) If not, do we agree that TB should NOT edit the original user's attachment source file after attachment has been added and opened from inside TB?
c) If yes, do you think that this bug (bug 378046) should/could be confirmed?
Finally, what do you mean by "be careful when dicussing TB's attachement related behavior?"

Thomas

Comment 5

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #4)
> a) Will any of the fixes for the bugs listed in your comment #3 fix this bug 378046?

No. MAPI related bugs merely make discussion in this bug confusing. If "multiple versions of original file" is involved, and/or if "undeleted file in MOZ_MAPI" is involved, test results by those bugs can mislead us to wrong direction.

> b) If not, do we agree that TB should NOT edit the original user's attachment
> source file after attachment has been added and opened from inside TB?

I agree with you.
I also think that "a snapshot copy upon attach operation always" is the best way to avoid confusion, and to keep consistency of Tb's behaviour on attachment while compose.

> c) If yes, do you think that this bug (bug 378046) should/could be confirmed?

I don't think this bug is problem of "incorrect/different coding from design". I think "design change request"(change from "with no clear design" to "with clear design" is also a "design change"=="Enhancement"). And when "Enhancement", I think "Confirm" means "developers agreed on that the enhancement will be needed". So I can say nothing about "this bug should be confirmed or not", but I think design/code change is better to be done to avoid confusion by users. 

> Finally, what do you mean by "be careful when dicussing TB's attachement related behavior?"

(1) When MAPI is involved, there are two kinds of "original file".
    - Pure original file (user's view)
    - Original file for "Compose" of Tb(file copied in MOZ_MAPI)
(2) MAPI related bugs can mislead us to wrong direction.
I think discussion on MAPI case is better to be avoid in this bug initially.

Comment 6

[Thomas D. (:thomas8)]

Okay, now I see your point. Thanks for your helpful comment #5, Wada. I am glad we agree TB should always and consistently use snapshot copy of user's source file upon attach (vs. current behaviour: TB sometimes uses pointer to user's first original attachment source file).

It is right that this bug is primarily about the NON-MAPI attachment cases, as those are handled differently by TB (technically speaking) and have their own flaws as mentioned by WADA in comment #3. However, exactly this different handling forms part of problem underlying this bug, both concerning the user experience (A) and possible direction for fix (B):

A) User Experience: We can't expect the average user to have any idea about the difference between the adding of attachments via MAPI or NON-MAPI methods. As a user, I will expect TB to behave consistently in handling my file attachments regardless of HOW these were initially added during composition (MAPI vs. other methods). As WADA and I have mentioned, and as has been "appreciated" by David Ascher, too, the current inconsistency in TB's attachement handling behaviour is very likely to leave user's confused, to the least: While TB's attachment handling behaviour will be consistent in ~80% of all scenarios (snapshot copy), it will unexpectedly handle them differently for the remainder of scenarios (pointer to user's original source file). In fact, the average user may not even notice the inconsistent behaviour, but chances are that he will end up editing an alleged "snapshot" attachment in good faith while in fact he is spoiling his original source file (referenced by "file pointer"). Therefore, while I appreciate that the borderline between "confusing design" and "bug" may be somewhat fuzzy, I would still consider this inconsistent and unpredictable behaviour a BUG (vs. RFE) with potential dataloss (in some cases).

B) Possible direction for FIX: As far as possible, MAPI and NON-MAPI methods of attachment handling should share the same code. I'm not a programming expert, but I take it that once the file to be attached has been "received" by Thunderbird (via whatever method), *storing and editing* of the "snapshot copy" attachment file should use the same routines regardless of the method of reception. E.g., MAPI and NON-MAPI temporary attachment files should be saved in only one and the same temp directory. Likewise, the routines for checking for existing temp file copies or temp-storing multiple versions of file(s) with same name could share the same code. Recoding in that area will be required anyway when fixing the MAPI bugs mentioned by WADA in commment #3: (1) Bug 344034, Bug 356808 //(2) Bug 356919, Bug 405717.

@WADA, as we agree on the main point of this bug/rfe, and as you have a whole lot more knowledge and standing in here than little me, do you think it might be an idea for you to ask for "wanted‑thunderbird3"?

Comment 8

[Marco Trudel]

I think the keyword "dataloss" is correct here. At least one of the users I support had substantial data loss because of this inconsistent behavior (Bug 447585).

Comment 9

[Marco Trudel]

The easiest solution would be to remove the feature of opening attachments directly. Saving should be enough. Otherwise - imho - Thunderbird starts acting like it's more than a MUA and allows to do things that the OS is here for. Better keep these things separate (like Seamonkey).

Comment 10

[moz]

PROBLEM
- A user I recently switched from Outlook to Thunderbird lost data this way:
With Outlook she was used to attach a spreadsheet to a mail, double click in the attachment, make edits and send the mail, but... with Thunderbird she was editing the "original file", not what she thought it was the "attached file" (Thunderbird thinks they are the same).

SOLUTION
- That Thunderbird make a copy of the file to attach and treat it like the attached file, so then:
 - it does not matter if the original file gets erased, changed, since it was already attached. 
 - if the attached file is changed, the original file is not affected.

ADDITIONAL COMMENTS
This is more important than it seems, there are a lot of Outlook users (and a lot of people that thinks that an attached file is a file, not a "pointer" to an original file).

Comment 11

[Thomas D. (:thomas8)]

As per comment #7 (duplicate), comment #8 (dataloss), and comment #10 (more dataloss danger for all former outlook users), and looking at confirmed Bug 391190 (message "forward as attachment" from pop or local folder loses attachment if original email is moved/deleted before message is sent), which suffers from the same problem as this bug:
Isn't it about time that this bug be given Status NEW?

Comment 12

[Magnus Melin [:mkmelin]]

->NEW

Comment 13

[Lewis Rosenthal]

Thanks for the xref in "my" bug 319058, Magnus.

I agree that this is a matter of philosophy. I view the attachment paradigm as attaching a pointer to a file, and would not consider editing the file by opening it from the mail client. Perhaps a user option for this is in order?

For my money, I don't want the snapshot taken until the email is sent. Then I have a time & date of the snapshot based upon the time of sending. Otherwise, the question remains, "*which* version of the file is attached?" If we only ever make a single snapshot of the file (at the time of sending), we *always* have a reference timestamp.

Some people might disagree with that, and of course, that's what our preferred environment (Tb and/or SM) excels at doing: providing user choice. Therefor, a pref for one default behavior over another might be the best way to go. (As an OS/2 user, I don't use MAPI, and I have few apps which will actually "send to email," so I have no experience form that perspective. I am only commenting insofar as the ability to edit an attachment, not the different behavior between attachment methods.)

Cheers.

Comment 15

[Hendrik Maryns]

(In reply to comment #14)
> *** Bug 475527 has been marked as a duplicate of this bug. ***

Note that that bug requests the inverse from this one: I *expect* TB to open the actual file, such that I can finish a draft while already attaching it to a message window.  Workflow: work on a document you want to send to somebody, while working, you think of remarks you want to put in the mail, and to be sure not to forget the attachment (you know, the typical mistake), already attach the document.  Maybe this process is interrupted and you shut down and restart the next day or something etc.

I think the conclusion is the following: there should be *consistent* behavior, independent of the protocol, and this should be *clearly documented*.  Optionally, a switch to get the other behavior, or some checkbox somewhere or whatever can be offered to get the other behavior.

Comment 16

[Tim Retout]

Attachment: Take a snapshot of attachments before adding them.

This is based loosely on HandleAttachments() in MapiHook.cpp, but doesn't bother with a separate temporary directory.

Comment 17

[Magnus Melin [:mkmelin]]

Since we can't reliably use the original file, the proposed route to make a tmp copy of the file when adding the attachment may be the least confusing solution... 

Tim, if the patch is ready for review, see
http://developer.mozilla.org/en/docs/Getting_your_patch_in_the_tree

Comment 18

[Marco Trudel]

Actually, there are people expecting exactly the opposite behavior. So, wouldn't the least confusing solution be to not give a possibility to directly open an attachment? If they can only be saved to somewhere, it will obviously always be a copy...
The current approach seems to just move the problem. Before, the ones expecting a copy were confused, now the ones expecting the original will be confused. It even keeps the "dataloss" property. Before, because the original file was edited (if for instance data was removed from a file), now if the user adds data and then decides to discard (not sending the mail) expecting the original was changed.
So, If this patch will be committed, I'm afraid there will be a lot of new bugreports...

Comment 19

[rsx11m]

(In reply to comment #18)
> wouldn't the least confusing solution be to not give a possibility to directly
> open an attachment? If they can only be saved to somewhere, it will obviously
> always be a copy...

Agreed, I frankly don't see plausible use cases for opening the attachment in the composition window other than verifying its content that the correct file has been attached. The other option would be to enforce read-only opening, as is now done for attachments in displayed messages.

Taking a snapshot still makes sense though, given that the attachment source can be changed, moved, or deleted before the message is sent. So, this would resolve other pending bugs. Corresponding issues exist for inline content, e.g., images as part of the message body included from a file (bug 404922).

The trade-off for snapshots is obviously that all attachments are copied first, regardless of their sizes (is this a performance issue?). Also, in line with the MAPI example in comment #3, care has to be taken that the temporary copy is properly deleted after sending or discarding the message, or deleting the attachment (will "attachment.temporary = true;" ensure that?).

Comment 20

[Bryan Clark (DevTools PM) [:clarkbw]]

I can't really see the use case for editing an alternate 'tmp file' copy of an attachment.  I understand how some people have come to expect this behaviour as it partially works now but it seems a little unexpected; perhaps it's just the 'unknown' of a tmp vs. real file that is most unexpected.

My current understanding is it's good to use this 'tmp file' copy purely for the time in between attaching a file to an email and sending the email so we don't completely lose the file due to a move / delete on the file system.

My suggestion for a design for this would be to keep the interactions simple and explain the issues to the person.  This allows for both types of behaviour indicated to happen and means people can understand why it did happen because we're not complicated about the choices.

1) Always make a tmp file copy of any file attached to a message, just in case
2) Always send / open the original file in the messages, not the tmp file unless the original couldn't be located
3) When TB detects the original file is gone via send or open indicate to the person that they are opening / sending a copy of the file; allow them to find the original
** Our notification should subtle on detect (if we can just auto detect) and louder on send / open.  This would be important on opening a draft message, we don't want to use an alert until it's critical.

Storyboard of someone wanting to edit a copy goes like this:
* Attach a file to the Compose window (tb makes a tmp copy)
* Move the original attachment to another location in the file system
* If TB auto-detects the original is gone we place a (!) indicator in the attachment list
* Opening that attachment warns the user the original file is gone, allowing them to locate that file or open the copy we saved for them.
** Alternatively this alert is shown when sending a message, likely we can add a "[x] don't alert me about this any more".

The story of someone expecting the file they attached and updated at the last second is the actual file they are sending works as a normal send, no re-attaching is required of the user to make certain they are sending the updated version.

Comment 21

[rsx11m]

(In reply to comment #20)
> 1) Always make a tmp file copy of any file attached to a message, just in case
> 2) Always send / open the original file in the messages, not the tmp file
> unless the original couldn't be located

That's kind of combining the overhead of a snapshot with the ambiguity which version has actually been sent... ;-)

> * If TB auto-detects the original is gone we place a (!) indicator in the
> attachment list

I could imagine that some indicator shows up already whenever the modification time of the original file is more recent than the snapshot, thus warning the user that something has changed with this attachment (it could also have been modified outside of Thunderbird). An item in the context menu or optionally the send dialog could then provide a choice whether the original or the earlier snapshot should be used. This would resolve the ambiguity, at least for those cases where the message wasn't opened from a draft.

Comment 22

[rsx11m]

The most comprehensive suggestion I can come up with is the following:

1) When adding an attachment, always create a temporary copy and give it
   read-only permissions (that's attachment 362208 [details] [diff] [review] plus permissions).

2) Note full path to attachment and modification time for file attachments.

3) Replace "Open" in the context menu with "Open Original", which will open
   the file referred to by the original path to allow modifications.

4) A new context menu item "Verify" opens the temporary snapshot, read only.

5) In regular intervals, check if path is still valid and if the modification
   times still match, otherwise highlight the attachment in the list.

6) If the path is valid and the modification times differ, provide an item
   "Update" in the context menu, which discards the current snapshot and
   creates a new one.

7) To cover bug 475527, saving as draft encodes the current snapshot and adds
   the original path and modification time of the file in respective headers.

8) When opening a message from a draft, restore the snapshot along with path
   and time-stamp information, highlight the attachment if the original path
   is no longer available or the modification time doesn't match the original.

9) When the message is sent, the snapshot is attached with the name derived
   from the original path, and the temporary file removed after encoding.

This should cover all cases discussed here, provide a mechanism for inline content in other bugs, and - except for the time stamp - would also allow updating an attachment dragged from a web page for which only a URL exists.
On the other hand, this would also imply quite a bit of effort to implement,
so whatever inbetween may work best as a trade-off.

Comment 23

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #21)
> (In reply to comment #20)
> > 1) Always make a tmp file copy of any file attached to a message, just in case
> > 2) Always send / open the original file in the messages, not the tmp file
> > unless the original couldn't be located
> 
> That's kind of combining the overhead of a snapshot with the ambiguity which
> version has actually been sent... ;-)

It's ambiguous because we'd have both, but the tmp only being a backup in case the first is lost.  Since we'd always warn (3) if we only have the copy left I hope it relieves the ambiguity.  

> > * If TB auto-detects the original is gone we place a (!) indicator in the
> > attachment list
> 
> I could imagine that some indicator shows up already whenever the modification
> time of the original file is more recent than the snapshot, thus warning the
> user that something has changed with this attachment (it could also have been
> modified outside of Thunderbird). An item in the context menu or optionally the
> send dialog could then provide a choice whether the original or the earlier
> snapshot should be used. This would resolve the ambiguity, at least for those
> cases where the message wasn't opened from a draft.

That's possible, except it creates an indication that people can't really take action on.  If we alert (!) "The file has changed!" (in an unknown way... it's been modified) a person could accept that it's changed but wouldn't really understand why it's change.  We could try to show some kind of a diff of the snapshot vs. the original, but that's a huge mess.  Instead I think it's correct to make assumption that when a person attaches a file and then goes and changes the file they are making last minute edits before sending.  At the same time if a person deletes the file or moves it we can offer to send a copy (from date $X) or help them locate the file.

Comment 24

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #22)
> 3) Replace "Open" in the context menu with "Open Original", which will open
>    the file referred to by the original path to allow modifications.

Other than the parts of detecting modification I think we're on the same path.  Detecting modification is going to lead to classic application (user) errors.  A typical issue can be that editors touch the file while closing it and then the user gets this "Document Modified" question.

1) Write a document in a Word Processor, save it without closing the application
2) Attach the document to the compose window
3) Double check the attachment in the word processor application and close the application
4) Hit send on the email
5) Get "The Document has been modified!" alert from TB

You could try some tricks like comparing the data of the two documents, but you never know when an application is going to write out a timestamp in it's own meta data and making your compare function useless.  Avoid the modified detection, it's a bit of a rat hole.

Comment 25

[rsx11m]

The difference between comment #20 and comment #22 is that I would treat the snapshot as the relevant copy to be sent out, allowing the user to update its content after changes to the original have been made on purpose. In contrast, your suggestion would use the original as before and only revert to the snapshot if the original is gone.

The modification detection is a can of worms, I agree, thus the maximum that could be done is a "this attachment has been touched" notification. This would still be good to have to alert the user to the fact that something happened with this attachment after it was added, being it on purpose or accidental, and to allow the user a choice between the snapshot or the updated version.

Comment 26

[Lewis Rosenthal]

...and what do we do when the original is on removable media (or a network) which has been disconnected? We would need to handle the possibility that after the snapshot, not only might the file change, but the file might not be available for comparison at all.

IMO (and I apologize for throwing this into the ring at this late date), this entire thing is starting to grow well beyond the scope of the original bug, which was to behave in an expected (and logical) manner. I appreciate the wealth of ideas which are developing from this discussion, but what I see happening is a major change and an entire can of worms...

Bryan, reading your comment #24, I'm trying to wrap my head around the ability to attach a file - and take a snapshot of it - while the document is still open. Many word processors (and other apps) will completely disallow access to an open document. I couldn't agree more with your last paragraph!

If we start going down the path of comparing documents, we are surely going to get ourselves into trouble. Are we doing a byte-by-byte comparison or merely comparing external timestamps? md5sums? Ugh...

How about a simple menu selection:

Attach live
Attach static

In the former case, a link to the original file is made. In the latter, a snapshot is taken. Default behavior may be set with a pref. Done. Finito.

Thoughts on that??

Comment 27

[Marco Trudel]

IMHO, all the suggested file comparison, warning and notification seem completely unnecessary (if they can be implemented at all). Not even a temporary copy is needed...
The simplest solution would still be to give no possibility of directly open an attachment, only saving them. Directly open attachments is bad/dangerous anyway since the files (which might contain sensitive data) will remain in the system temp folder and the user usually doesn't know this.
But I also like the idea of Lewis with "Attach live"/"Attach static" (although "Add copy" and "Add live" would seem better (but not yet perfect) names).

But as I said multiple times already, I'd go for not allowing to open them...

Comment 28

[Tim Retout]

(In reply to comment #19)
> Also, in line with the MAPI example in comment #3, care has to be
> taken that the temporary copy is properly deleted after sending or
> discarding the message, or deleting the attachment (will
> "attachment.temporary = true;" ensure that?).

It's meant to from my reading of the documentation, but I suspect Bug 405717 still applies. I have seen temporary files hanging around.

Comment 29

[rsx11m]

(In reply to comment #27)
> IMHO, all the suggested file comparison, warning and notification seem
> completely unnecessary (if they can be implemented at all). Not even a
> temporary copy is needed...

When writing down the "comprehensive suggestion" in comment #22, I had the feeling myself that it was overdone (as you may judge from its last sentence). Using a temporary-file snapshot nevertheless resolves quite a few issues related to the current attach-when-sending behavior.

The confusion effect on the user's side right now is the ambiguity when the attachment content is actually used, expecting a snapshot is more intuitive.

> The simplest solution would still be to give no possibility of directly open an
> attachment, only saving them.

This was the motivation for making the temporary copy read-only, thus the user can verify what was attached but cannot make any modifications, those should be always done on the original file (possibly outside of Thunderbird itself only).

> But I also like the idea of Lewis with "Attach live"/"Attach static" (although
> "Add copy" and "Add live" would seem better (but not yet perfect) names).

I'm happy with the switch solution as well. It emphasizes giving the user the choice when the attachment content is added to the message, thus making all other considerations of what happens when the file status changes, etc., mood as long as the user is aware of what he or she is doing.

How about "Attach a Snapshot" and "Attach when Sending" as label names?

To also accommodate bug 475527, files would have to be treated as detached if "Attach when Sending" is chosen and the message is saved as a draft, thus the original file is still examined only when the message is reopened and sent. This would imply solving detach/send issues related to bug 327302 first.

Comment 30

[Thomas D. (:thomas8)]

Some more thoughts on this from the reporter of this bug!
Trying to sum up and expand on what has been said so far:

a) The current behaviour when opening attachments after they have been added is unpredictable: for the same message, sometimes it opens the original source file, sometimes a snapshot copy. So the main focus of this bug is predictability (not more, and not less).

b) TB's current attachment method is based on attaching a snapshot copy, which in theory can be edited, too. Due to various flaws and technical limitations, both the snapshot and the editing part of it sometimes don't work as expected. It follows that implementing "attach live = add attachment when sending" is a new feature (this bug could be fixed without implementing "live attachments").

c) Whether attachments should be attached immediately (immediate static snapshot copy) or only just before sending ("live" attachment until message is sent), is a question of taste / philosphy / usage patterns. Therefore, users should be given the choice.

d) Choice between "attach static" and "attach live" should be implemented at two levels: firstly as a default preference and secondly, as a menu choice for individual attachments. 

e) For the record, BOTH methods eventually attach a snapshot of the file; the main difference is the point of time when the snapshot is taken. Keep this in mind when labelling the choices. So far, the following labels have been suggested (the last one added by me):
1) "Attach static" vs. "Attach live"
2) "Add copy" vs. "Add live"
3) "Attach a Snapshot" vs. "Attach when Sending"
4) "Attach now" vs. "Attach when Sending"
Appropriate labelling should also consider attachment editing options, which are currently under discussion in this bug: "Attach live" might make a good caption if double-clicking on the attachment allows live editing of the original file; if we were to disable direct editing as suggested by Marco Trudel, "Attach when sending" might be better. On the other hand, "Attach static" might create wrong impressions if snapshot attachment can still be edited.

FOR EACH of the two methods (attach static vs. live), the following decisions are due:

f) Which opening mode, if any, should be allowed on the attachment?
1) disallow direct opening of attachments from TB (allow saving only, as suggested by Marco Trudel, comment #27)
2) allow opening read-only (as suggested by rsx11m, comment #19) 
3) allow opening for editing (as in current implementation, though faulty)

g) Should the attachment be mime-encoded and saved with the message before sending, and if yes, when?
1) no mime-encoding before sending (might be a good idea for "attach live", cf. rsx11m's comment #29, "files would have to be treated as detached")
2) mime-encoding when saving draft and/or
3) mime-encoding when closing draft and/or
4) mime-encoding [only] when sending message
5) mime-encoding [on the fly] when saving draft as external .eml file

IMO, this point should get much more attention, as it can cause a lot of technical and user experience problems, especially if you allow direct editing (f-3). E.g., with a live attachment, any mime-encoded version saved with the message before sending runs risk of being outdated compared to the live version which will eventually be sent. On the other hand, NOT having a mime-encoded version of the attachment with the mail might lead to other problems, e.g. with previewing attachments during composition, as per Bug 377377 (Mail compose window should show preview of attached files). However, preview could also use live source file.

h) There should be some sort of visual indicator for each attachment if it is "live" or "static", like an overlay icon. E.g. you could use "link" overlay icon for "live" attachments.

Note that f) and g) might be answered differently for the two methods!

From my (limited) point of view, some aspects to consider (evaluating some of the above options):

A1) (Use link, no mime snapshot for live attachments before sending) Make live attachments pure links (cf. "detached"), do NOT encode a mime copy in the message. Mime copies are very static by definition, and you'll always have two versions of your "live" attachment to get you into version trouble. If attachment preview gets implemented, it must get the file from its original location instead of decoding from embedded mime.

A2) (Do not disable opening of attachments from TB during composition) I can understand the motive, but from a usability point of view I think Marco Trudel's proposal in comment #27 to disable direct opening of attachment completely would NOT be a good idea, as I'll explain in A3 and A4.

A3) (Allow direct edit for live attachments) Live attachents should be directly editable from TB. Attaching a "live" link to a given attachment source and then disabling that link and forcing the user to open the attachment source from outside thunderbird will certainly get usability complaints.

A4) (Force read-only for static attachments only) However, disallowing the editing of a static snapshot might be consistent and avoid a lot of technical and user experience problems. E.g. when while you edit a snapshot with some external application like MS Word, technically you cannot delete the snapshot. Not deleting snapshots is a security problem if they are in the user's temp folder without user awareness. Also, user experience might be bad when users forget to save their modified snapshot in external app and so it can't be updated in TB. Forcing read-only for static snapshots spares us from a lot of problems with tmp-files (cf. comment #19). I think mime-encoding the snapshots is fine.

A5) (Bad user experience with read-only for static attachments) Having said which, personally, I'd much like to be able to edit static attachments directly from TB, if there's any way this could be reliably realized. Before you disable editing of static attachments, check out the user experience: users will be forced to save attachment somewhere, modify, and then re-attach. Not nice at all but perhaps the lesser evil.

I think it's very clear that whatever route this is going to take, the whole thing is VERY complex and there won't be a golden way to do this. It'll take a lot of hard thinking to consider and distinguish all the details of both the technical side and the user experience when you go ahead with this. For each of the two methods and each possible combination of the variables as outlined above in f) and g), we'd need something like a tree diagram, one for the technical side and one for the user experience...

Thanks everyone for their contributions!

Comment 31

[Tim Retout]

(In reply to comment #30)
> b) TB's current attachment method is based on attaching a snapshot copy, which
> in theory can be edited, too.

From what I have seen, this is just not true, unless you are using a radically new definition of 'snapshot copy' - one that doesn't involve copying. The only case where copying takes place is in MAPI, which is entirely separate. The patch I have started on attempts to change that.

Personally I do not agree that end users want another configuration variable or menu item to choose their "attachment style". Thunderbird should in my opinion simply implement what every other sensible mailer does - take a copy of the attachment at the time it was added.

This is sensible because otherwise there is no guarantee that the attached file will not be deleted between the time it is attached and the time 'Send' is clicked. It will also be consistent with how the MAPI code path works. It will then not be necessary to remove the 'Open' feature on attachments.

If anyone disagrees, I would urge them to write a patch, rather than more wall-of-text comments. :)

Comment 32

[rsx11m]

(In reply to comment #31)
> The only case where copying takes place is in MAPI,

Correct, the only other case where a snapshot is made applies to pasting contents from the clipboard, which is currently implemented for the message body but not for attachments (that would be bug 379186 or bug 335783).
The other snapshot-like case is when the message is saved as a draft.

> Personally I do not agree that end users want another configuration variable or
> menu item to choose their "attachment style".

Well, opinions here are rather diverse, so it would make sense to somehow allow both use cases and not just one of them.

> If anyone disagrees, I would urge them to write a patch, rather than more
> wall-of-text comments. :)

If we want to have something done in time for TB 3.0 and not a later version, your patch - with or without a (hidden?) pref added to execute the snapshot code or bypass it for the "attach as reference" case - would certainly be good for a feasible solution. All the "nice" extensions discussed here could be deferred to follow-up bugs as desired.

Two questions for the current patch:

(1) Did you verify that the temporary file is removed, either when the message is sent or the application closed? We don't want a history of attached files piling up in /tmp.

(2) If I remember correctly, attachment.url and attachment.name can be different, thus is still the name of the original file used as the attachment name or is it the temporary name? Well, it looks like origFile.leafName would derive it from the real name anyway.

Comment 33

[Tim Retout]

(In reply to comment #32)
> (1) Did you verify that the temporary file is removed, either when the message
> is sent or the application closed? We don't want a history of attached files
> piling up in /tmp.

Yes - I've now investigated this a bit more.  Most times this is fine, but there seems to be one edge case where cleanup does not happen: when I try to 'Save as Draft' or copy the sent mail into a remote folder, and then have to cancel things. (I'm mostly working without a net connection right now, so these never succeed.) This seems to be bug 405717 / bug 356919. For some reason the attachment destructor does not get called in these cases?

> (2) If I remember correctly, attachment.url and attachment.name can be
> different, thus is still the name of the original file used as the attachment
> name or is it the temporary name? Well, it looks like origFile.leafName would
> derive it from the real name anyway.

Yes, attachment.name is untouched by this patch. The temporary filename is based on the original leafname to ensure the right icons are chosen in the UI (from what I remember of comments in MAPI code), but it doesn't make a difference to the sent email.

Comment 34

[Magnus Melin [:mkmelin]]

Comment on attachment 362208 [details] [diff] [review]
Take a snapshot of attachments before adding them.

This patch doesn't apply to hg tip (it wouldn't have when submitted either, I think)

This sure is a bit complicated... and sounds like implementation would easily get out of hand.

Maybe it could be good to do something along the lines of this patch. 
 - we'd at least be consistent and always send the file snapshot from when the file was attached.
 - people who want to do last minute edits would have to open the (tmp) file *from the message*

... and make opening an attachment from a re-opened draft/message open a *read-only* copy - if it doesn't already.

Am I missing some case here?

Comment 38

[Thomas D. (:thomas8)]

Bug 222495 confirms the current tendency of this bug with developers suggesting to use immediate snapshot upon attaching. It also points out the potential security leakage (breach of privacy) of this currently inconsistent and non-transparent behaviour:
> Either way this is a nasty bug, with possible unintentional security
> leakage -- i.e. if file has been replaced with new contents that contain
> sensitive info, you just accidently released the sensitive info --
> even though the file didn't contain the info when you attached it.
That's privacy risks in addition to dataloss as per comment #1, and confirmed (by reality) in Marco's comment #8 --> Major.

Bug 317007 advocates for the immediate snapshot, too and describes a business usage scenario where snapshots are individually edited before send (against the suggestion to make snapshots read-only, cf. e.g. Marco Trudel's comment #27 and rsx11m's comment #22, section 4). I have a feeling (but no data) that there might be more businesses out there who use it that way, if only because Outlook seems to have it. Marco, do you think that user's should not even be able to verify (view) the snapshot (read-only)?

Comment 39

[Marco Trudel]

Actually it would be cool having the possibility to view snapshots (readonly). But I don't think this can be implemented (applications might ignore that files are not writable (change the access rights)). Also, this creates the problem that sensitive data might remain in the system temp folder (happened to me occasionally (found months later files I would have preferred not being there)).
So, I'd go for creating a snapshot on adding files to a mail. Also, I think only saving attachments to somewhere would be better.
But having the possibility to edit them would be more user friendly. But, as mentioned above, it might be dangerous. Also it's inconsistent since attachments from received files can not be edited (respectively changes will not be saved into the mail).

Comment 40

[L A Walsh]

After I have added a file to the email, I wouldn't expect to be able to edit that *instance* of the file.  I would expect to be able to view the attachments, and if I don't like them, to delete them and re-add updated attachments -- but when I add them, they should be 'static copies' of the file as it was at the time I added it.  To do otherwise, is not really 'adding them' to my composed email but only recording my desire to add the, _later_, contents of that attachment to the email.  "Later", the file may not be what I meant to add -- which is the main reason why this 'issue' is a problem.  What I attach now, and what may get attached by the time I hit 'Send' may not be the same.  That can cause problems.

For email that comes in, I want to be able to delete attachments (i.e. - save message, but not the wonking-big 150MB file that "they" included, because I already saved that to disk) from the message. 

I don't think I it makes sense to "add" new attachments, or edit existing attachments of a received email unless I am doing so to a copy of the email that I create.  I think this is what happens now -- if I delete or modify the attachments of a received email, a copy is made and I see the copy and the original in my IMAP folder(s).  The original goes away when I delete it -- that's  fine the way it is.

Comment 41

[Wayne Mery (:wsmwk)]

dupe to bug 356808?

Comment 42

[rsx11m]

No, the WIP patch provided there wouldn't solve anything discussed here. Attachment snapshotting (or not) is a broad issue and spread across various
bugs with different scopes, bug 356808 is specifically about MAPI usage.

Comment 43

[Phil Lacy]

OEXP works like TB. write file attach file, edit attachment also edits orig file.

which means file doesn't get to be an attachment (actual data becoming part of message) until saved or sent.

Makes sense to me, whoever heard of editing an attachment. User would obiously want to edit the real file because that is what it appears to be.

I vote WONTFIX

Comment 44

[Hendrik Maryns]

(In reply to comment #43)
> OEXP works like TB. write file attach file, edit attachment also edits orig
> file.
> 
> which means file doesn't get to be an attachment (actual data becoming part of
> message) until saved or sent.
> 
> Makes sense to me, whoever heard of editing an attachment. User would obiously
> want to edit the real file because that is what it appears to be.
> 
> I vote WONTFIX

That is also the behavior I’d like, but the ‘only if’ etc. in the title makes my think this is not a simple WONTFIX.  The behavior has to be made consistent first.

Comment 45

[Phil Lacy]

using 'send to' is one of the inconsistencies but I got a patch to make it consistent

Comment 46

[Thomas D. (:thomas8)]

(In reply to comment #43)
> OEXP works like TB. write file attach file, edit attachment also edits orig
> file.
> which means file doesn't get to be an attachment (actual data becoming part of
> message) until saved or sent.

Don't forget TB is autosaving your composition every x mins.
 
> Makes sense to me, whoever heard of editing an attachment. User would obiously
> want to edit the real file because that is what it appears to be.

Please read previous comments to see that there is NOTHING obvious about the desired behaviour. Different users want different things. For many users, the attachment does NOT appear to be the real file, but a snapshot copy of that file.

> I vote WONTFIX

Again: Please read previous comments before voting for resolutions that aren't solutions.

Comment 47

[Phil Lacy]

(In reply to comment #46)
> Please read previous comments to see that there is NOTHING obvious about the
> desired behaviour. Different users want different things. For many users, the
> attachment does NOT appear to be the real file, but a snapshot copy of that
> file.

ok, but it seems editing a snapshot of the file would be more of a feature request. IMHO I wouldn't go for that.

This bug may be best served by locking the file from edits or deletes while in compose window (including edits by TB). Once saved as draft I believe it becomes embedded in the message.

I would hate to see us tackle the structure of creating managing *and* cleaning-up temp files just for the feature of being able to edit the snapshot or preventing indirectly editing via the original file.

so I change my vote to lock the file. That way, like many other programs do, the compose window takes control of the file or advises the user that the file can't be added because it is in use by another program.

Comment 48

[Hendrik Maryns]

(In reply to comment #47)
> 
> so I change my vote to lock the file. That way, like many other programs do,
> the compose window takes control of the file or advises the user that the file
> can't be added because it is in use by another program.

Seems reasonable to me.  Every other solution is too complex.  (Both for implementation and for the unaware user.)

Comment 49

[Phil Lacy]

(In reply to comment #47)
> so I change my vote to lock the file. That way, like many other programs do,

Seems we currently do not lock files and have no code for such.

Comment 50

[Thomas D. (:thomas8)]

(In reply to comment #47, comment #48, comment #49)
> lock the file ...

We can't lock original files that have been added via mapi since they can still be in use by another application (like Word, after File > Send To > Email Recipient (as attachment)).

> ...or advises the user that the file can't be added because it
> is in use by another program.

That's a perfect usability killer for many mapi cases like sending files from within applications (like Word). We certainly do NOT want to tell the user "Sorry, but we can't attach the file because Word is still using it".

If you really want to lock files, you'll need a TB-owned snapshot copy. For many reasons mentioned in this bug, "immediate snapshot copy when attaching" needs to be the default behaviour (for detailed Scenario/Steps, why "attach later when sending" doesn't work in many cases: see bug 356808, comment #35). "Attach later when sending" is a new feature that needs a separate implementation and respective UI changes.

I'm not sure if locking files is a good idea or even needed, at all. We shouldn't if we don't have to.

Comment 51

[L A Walsh]

If someone adds a file to an extension, OR drag-n-drops a file on an open compose window, TBird should make a private copy of the file at the time the user drops the file on the compose window.

This would be compatible with Windows actions actions -- when dropping a file on a target on a different file-system (or target type), Windows makes a copy and drops the private copy on the recipient.

A mail message would be considered it's own private "file-system" (rooted in the mail-message itself), so any drag-n-drop operation, of a file, should force the creation of a private file that shouldn't need to be locked -- since it should be a "private" temp file.

Comment 52

[Thomas D. (:thomas8)]

Tim Retout (assignee), could you provide an updated patch which addresses Magnus' Comment 34?

Comment 53

[Thomas D. (:thomas8)]

Dependent bug 516373 lends further support to the preferred short-term solution of this bug (take snapshot copy immediately when attaching). Note how reporter naturally assumes that attachment has already been attached so it should be safe to delete it from original location:

> Steps to Reproduce:
> 1. Compose message
> 2. Attach file
> 3. Move file to trash (not the attachment, the actual file)
> 4. send message
>
> Actual Results:  
> Error: Sending of message failed. Unable to open the temporary file
> /path/to/attachement. Check your 'Temporary Directory' setting.
>  
> Expected Results:  
> Message is sent with the attached file
> 
> Suggested fix:
> 1. Copy file to temp dir when adding it to the list of attachments

Comment 54

[Phil Lacy]

OExp is like TB. It uses the actual file while message is under composition.

It would seem to be ideal to create the message attachment part when attached and save it in a buffer. I would think we should forgo the idea of modifying a file while it is attached which is really stretching convenience. It doesn't make too much sense to me. Not being able to edit a file you attach to a message would be understandable, logical and not frustrating to the user.

Comment 55

[steve]

We have users who have a generic file and want to send it to users x y and z, but doesn't want user x to see tab 2 or whatever, so when he attaches it to the email, opens it and removes the data user x doesn't need it actually removes the tab from the original file...  That is not what should happen.  It should be:

attach file -> file is copied to a temp location, then attached.

Comment 56

[Lewis Rosenthal]

I disagree (strongly) with Comment 55. We shouldn't become a "versioning platform." If certain users should not be given a particular version of a file, then that version should be created in the document-generating app *first*, then that modified version attached to an email and sent.

If someone wants to create an extension which could then edit an attached "snapshot" and re-attach the edited version, then as Phil essentially said in Comment 47, that's a horse of a different color.

Obviously, different people work in different ways. I don't attach a file until it's ready to fly, and if I *do* decide to edit it after attaching and before sending, I delete the attachment and re-attach, and I don't create yet another copy of the document by opening it from the attachment pane, either. I don't know how many people work like I do, but most of my clients do, because I preach disk space frugality to them.

Comment 57

[Phil Lacy]

comment 56 makes the most sense to me for an email client. I don't think the majority of our users expect a basic feature of an email client to do anything more then compose email and attach or remove files.  If we are to feature the ability to call on a word processor, spread sheet, music editor, picture editor or other document processor to edit various parts of the email (attachments) while under composition then I think the policy makers of TB need to give that go ahead. I think it's a little loosey goosey myself.

Maybe Magnus or Bienvenue or dmose can give us a thumbs up or down on that functionality.

Comment 58

[steve]

I don't think you all get what I said..  My people aren't making 5 versions of one file, they want to use one file, edit it, send it.  Then their edited version is gone (or at least in the temp dir)...  NOT creating generic.xls generic1.xls etc.  This method SAVES (assuming that the temp location gets cleared) disk space!    

I'm not saying that Thunderbird needs to become a "versioning platform", but the fact that it makes sense (for Thunderbird) to make a copy of the file(s) into a temp location that is writable by the user so that IF people want edit an attachment, they can WITHOUT fear of making the changes into the real file.

Hell, at least make it an option to allow editing of ORIGINAL file attachment or COPY of the file attachment.

This is a feature that is in Outlook, and as we convert users to Thunderbird, they get bit by this, as they are used to and comfortable doing it...

Comment 59

[Lewis Rosenthal]

I think we understood your original request, Steve. However, I believe that the majority belief here is that we take a *static* snapshot at the time of attachment, freezing the document in time, and *not* allowing edits to the snapshot (too much file locking/unlocking, etc.).

My point was that if five different versions of a document need to be sent to five different people, then the app generating those documents should handle either creating multiple versions of the document in different files *or* some type of security levels within *one* copy of the file, and that either way, it's not TB's (or SM's) business to do any of this but attach *a* file.

The option to allow editing of the original file attachment is a greater undertaking than one might think, especially considering that we may not know *how* the file was attached in the first place (e.g., via mapi vs manually clicking the attachment pane).

The mere fact that a "feature" exists in Outlook (or any other mail client for that matter) should not dictate that we follow suit *unless* it can be determined that this is an expected (and workable) feature (i.e., if Outlook allows for editing emails in MS Word, that doesn't necessarily mean that we should do the same, but rather, we should improve our own editor so that there is no need to use an external editor).

Anyway, just my two cents, and as this is not a discussion forum but rather a bug tracker, we should probably spare everyone the philosophy.

Comment 60

[Thomas D. (:thomas8)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.7) Gecko/20100713 Lightning/1.0b2 Thunderbird/3.1.1

(In reply to comment 54 ff.)

There are a lot of misconceptions, wrong impressions, inaccuracies and even falsehoods in the previous comments. Furthermore, discussion is moving away from the primary goal of this bug as I reported it. So I'll have to clarify some of this. (Terminological caveat: when I say "TB-owned", I do NOT imply that we need to lock the file).

0) Composition vs. received attachments

This bug is exclusively about attachments *at composition time*. Attachments of received messages are an entirely different matter and not subject of this bug. (When you open/edit attachments from *received* messages, we save them to TEMP/ and lock them "read-only", which is good behaviour but not prejudicial for this bug).

1) Detailed anatomy of current inconsistent behaviour - goal of this bug, non-goals

This bug is primarily about making the user experience of attachment behaviour during composition *consistent*. It is NOT about removing existing options (like that of opening attached files for editing at composition time), it's about making these options behave consistently and predictably. Right now (TB 3.1), there's at least two competing behaviours:

a) For attachments attached via MAPI (e.g. Explorer's Send-to command), TB creates an immediate snapshot copy in TEMP/MOZ_MAPI temporary folder, and links to that TB-owned snapshot. Opening/editing of the attachment will open/edit the SNAPSHOT copy from moz_mapi (and leave the original untouched, so that user can keep, edit, rename, remove (USB-Stick), or even delete the original file without any implications for the attachment).

b) For attachments attached from *inside* TB (e.g. File > Attach, or drag&drop), TB does NOT immediately create a TB-owned snapshot copy. Instead, it links to the ORIGINAL file. Opening/editing of the attachment will open/edit the ORIGINAL file. This is different behaviour from a) and thus unexpected and dangerous for users that usually use method a): They will think they are working on the snapshot, while they are in fact working on the original (this bug).

c) Regardless of how the attachment was added (a or b), i. e. regardless if it's currently a snapshot in moz_mapi or original file in user's document folders, TB will *always* create a TB-owned snapshot and link to that if you close and reopen the draft message. For both a) and b), opening/editing of the attachment will furtheron only open/edit the SNAPSHOT copy. However, that copy is now in user's TEMP/ folder (as a third location in this game). So in cases of b), user will suddenly open/edit the snapshot instead of the original, which is unexpected, inconsistent and equally dangerous (user makes changes believing he works on saved original, but it's just a temp copy that will be deleted by TB after sending - again, this bug.)

2) Short summary of current inconsistent behaviour:

- depending on the method of attaching(!), TB will either create and link to an immediate snapshot OR just link to the original
- after closing and reopening a draft, TB will always(!) create a snapshot
- user can never easily/reliably know in advance whether double-clicking on the attachment will open/edit a snapshot copy or the original file (this bug). We cannot expect the average user to double-check attachment tooltip paths each time (where at least we don't lie about the current path).
- regardless of the method of attaching, we currently technically provide the option of opening AND editing at composition time

3) Short Summary of general goal of this bug

The behaviour of opening/editing attachments (original file vs. snapshot copy) should be consistent at any point of composition time, regardless of the method how the file was attached, and regardless of whether the draft happens to have been closed and reopened or not (this bug).

4) UI Requirements for fixing this bug: Reasons for the proposed and widely-accepted default behaviour of *immediate snapshot copy* at the time of attaching

The unified default behaviour needs to be that TB creates an *immediate snapshot copy* of the original file upon attaching (on which there was a wide and profound agreement in this bug, notwithstanding legitimate feature requests to optionally offer "attach when sending" behaviour where user deliberately attaches a live link to the original file and we only take the snapshot upon the moment of sending. But that needs another RFE). Reasons:

a) only an immediate snapshot upon attaching ensures that the file will actually be available later upon sending in exactly the same state as at the time of attachment. Otherwise, file might be altered, renamed, removed, or even deleted in the meantime, due to the natural and justified UI intuition that after attaching, the file is actually "attached", which means its IN the email, which means it is a COPY of the original at the time of attaching and NOT the original itself.

b) The attachment icon we show in attachment pane underlines this idea: it's no different from a normal file icon, so it indicates there is a file object INSIDE the mail. If you want a live link to your original file attached instead, we need a different and unambiguous UI for that, like a link overlay icon (needs a new bug).

c) For obvious reasons (see a), we are currently taking the snapshot anyway as soon as the user decides to close the draft. Immediate snapshot when attaching just makes this behaviour more consistent and predictable.

d) compare real-life behaviour: when you attach a document to a letter, you prepare at least one physical instance of the document that you intend to send. You attach that physical copy with a paper clip to the covering letter, and put both in the envelope. It's now attached to the letter in the envelope. It's no longer in your folders, nor on your desk, it's really in your letter. If you want to look at it again before sending, you need to open the envelope, and you can then alter the document before sending (e.g. add hand-written notes, cross things out, or discard it and attach something new).

5) Details about the non-goal of this bug (keep open/edit option, do not lock snapshot files)

As laid out in 0), 1) and 2), we currently *do* allow editing of any attached file *at composition time* by opening it directly from the attachment panel (albeit inconsistently).  All we do is pass the file path to some external application. Nothing magical here, and far from making us a "versioning platform" or anything over the top. Iow, we are currently NOT locking any of the attached files *at composition time*, neither the snapshots nor the originals. I have never heard of any problems with regards to that feature, so I assume this is working, and it's not technically necessary to remove it for fixing this bug. Many people want and use that feature in TB. If you want to remove that feature, open another bug. Not here! (And I don't see a majority for locking the file, either).

6) Technical requirements to fix this bug, and unify the behaviour:

- I assume that both Moz_mapi and TB-internal methods of attaching initially at some point pass/know the full path of the original file to be attached.
- From that point onwards, mapi and non-mapi routines should be unified into a single routine that uses the path of the original to create an immediate snapshot copy (see 4), in a unified TB-only subdirectory of TEMP (or some better location, which would be another bug again).
- From attachment panel, we always link to the temporary snapshot copy that we have saved.
- For this bug, do not change anything else in terms of UI, nor lock the snapshot.

Comment 61

[Thomas D. (:thomas8)]

Unfortunately, we haven't heard from assignee, Tim, for more than a year (last comment of his was comment, and he hasn't replied to comment 52. Tim, please re-assign yourself if you can spare some time to update the patch along the lines of comment 34.

Comment 62

[peter]

One of the consequences of this bug, as mentioned in comment 60, is that the attached file can change before it is sent.  I agree that the attached file should be a snapshot of what it was when it was attached, unless the user specifies otherwise.

But this bug has been around for almost 5 years without apparently much resolution, and no activity apparently in about 1.5 years.

Until such time as it is actually fixed, I propose a partial interim solution which presumably is much easier to implement:

Before sending, TB should check whether the file has changed since the time it was attached, and if so ask the user something like:  "Attachment[s] <filename> [<filename> ...] has/have changed since the time at which you attached it/them.  Send anyway?"

Comment 63

[Lewis Rosenthal]

That's an excellent suggestion, Peter, though some thought must go into how we make the determination that the file has indeed changed between the time it was attached and the time the user attempts to send the message. Right now, there is no mechanism (of which I am aware) to do this (checksum the file, I would guess). Such an operation can potentially cause considerable overhead at the time of attachment (I am fanatical about email attachments not exceeding 3MB or so, but still, people tend to attach the most outrageous things, and checksumming a 25MB attachment - and then again, the original, the location of which we would then need to track - could pose a considerable delay).

So, while the suggestion is a good one, I'm not sure how we get from point A (where we are now) to point B without some major stuff going on in between.

The above notwithstanding, per Thomas' comment #60, this bug is specifically about the inconsistency in behavior between what happens when a file is attached via MAPI and when a file is attached through some other (direct) means. If we *always* create a snapshot of the file, per Thomas' recommendation, then our behavior (for better or worse) will at least be consistent.

I think that again, while your suggestion has much merit, Peter, it is a different RFE, and should be set forth as such in a new bug.

Thanks for taking the time to get this bug moving again, too.

Comment 64

[peter]

(In reply to Lewis Rosenthal from comment #63)

Ok, in that case:

1.  Modify my suggestion to only look at the timestamp or existence of the file.  Change the message to "timestamp has changed" or "file no longer exists".  Then cost is trivial.

2.  Ok, I will move to a new bug.

3.  I don't think it's an RFE, it's a workaround for the current bug, but whatever...

Comment 65

[peter]

(In reply to peter blicher from comment #64)

My suggestion is now bug 722094.

> (In reply to Lewis Rosenthal from comment #63)
> 
> Ok, in that case:
> 
> 1.  Modify my suggestion to only look at the timestamp or existence of the
> file.  Change the message to "timestamp has changed" or "file no longer
> exists".  Then cost is trivial.
> 
> 2.  Ok, I will move to a new bug.
> 
> 3.  I don't think it's an RFE, it's a workaround for the current bug, but
> whatever...

Comment 66

[WADA:World Anti-bad-Duping Agency]

(In reply to peter blicher from comment #62)
> I agree that the attached file should be a snapshot of what it was when it was attached,
> unless the user specifies otherwise.

FYI.
Bug 722929 is for the "unless the user specifies otherwise" part.
Because attachment file is also read upon "Save As Template" as done in "Save As Draft", attached file data in template mail is always data when the template mail was created. Bug 722929 is enhancement request on attachment file processing in template mail. Required mechanism for it will be easily enhanced to feature for "always send latest file when mail is sent by Send/Send Later, instead of snapshot upon start of mail composition".

I prefer such feature to "attachment file modification check by Tb".
If a way to request "Don't read attachment file until I actually send this mail" exists, user can know that default behavior is "snapshot at start of mail composition"("pseudo snapshot" though). I believe user's confusions will be reduced by such feature even if Tb's "snapshot" is not fully "snapshot upon start of composition". And, such feature is also a solution of problem like "Tb freezes a while during mail composition when big files are attached due to auto-save especially when IMAP Drafts folder".
There are three kinds of users.
(a) Always wants snapshot upon start of mail composition.
(b) Always wants latest file when he executes Send operation.
(c) Sometimes wants and expects "snapshot" because he alters file content for
    other works after he starts mail composition, but sometimes wants and expects
    "latest file upon send" because he updates attachment file after he starts
    mail composition in order to send newer version of file.
Feature by Bug 722929 will reduce war between (a) and (b), and will fulfill requirement of (c).

Comment 67

[peter]

While the current bug and bug 722929, which it blocks, will, as best I can understand the discussion in those two bugs, solve my concerns in comment 62, I wish to return to my point in comment 62.  Namely, it has been a very long time since anyone did anything about this bug, and since it is currently assigned to no one, there is no reason to believe anything will be done about it in the foreseeahle future.

That is why I proposed bug 722094;  namely, so that some of the ill effects of this bug can be mitigated with a (presumably) very simple fix while we wait for the current bug to bubble up to the surface of someone's todo list.  I note that it also addresses the points (a), (b), (c) raised in comment 66.

Comment 68

[Thomas D. (:thomas8)]

(In reply to peter blicher from comment #67)

Peter, thank's for reviving this bug. I've made my points, I can't fix it myself, so all I can do is wait and ensure that this is not derailed by wrong claims about the current behaviour.

> While the current bug and bug 722929, which it blocks, will, as best I can
> understand the discussion in those two bugs, solve my concerns in comment
> 62, 

True. Comment 62 says default expectation for attaching is A) immediate snapshot when attaching.

* The *minimal* fix for this bug is:
- make A) the default behaviour
- remove incompletely realized and thus confusing, dataloss, and privacy-corrupting behaviour of B) - snapshot when sending.
For reasons why, see comment 60, 4a)-d), and e), ux-conformity with webmailers.

* The *best* and ultimate solution for this bug is:
Implement both A) *and* B) (this bug *and* Bug 722929) as strictly separate and consistent features, because different users need different behaviours, or even both behaviours concurrently for different attachments. So most likely, we ultimately want a per-attachment choice between A) and B).

> That is why I proposed bug 722094;  namely, so that some of the ill effects
> of this bug can be mitigated with a (presumably) very simple fix while we
> wait for the current bug to bubble up to the surface of someone's todo list.

If anyone knows of any such todo lists for existing bugs, please let me know.

> I note that it also addresses the points (a), (b), (c) raised in comment 66.

I appreciate the "least-resistance" logic of bug 722094 and it does mitigate a little bit. However, unfortunately, it's not that simple, because of the complexity of the underlying current wrong behaviour, as outlined to my best knowledge in comment 60 (from a POP3 perspective).  I don't know what you mean by "address", so FTR:

Bug 722094 with current propositions will NOT fix use cases of a), b) and c) raised in comment 66. Without more changes of behaviour, it will just add a useful warning for one or two special cases out of so many cases where the current behaviour is wrong. For a more detailed analysis, see bug 722094, comment 8.

Comment 69

[L A Walsh]

I'd like to mention that the worst possible outcome in this scenario (which has happened, to me), is that you drop the file into mozilla and think "it has it", then erase the original.  When it goes to send... no file..

Only happened to me once before I figured out the problem.


2nd prob -- if the file changes before you send it -- the original may not be 
recoverable -- again, mis-set expectations.

Why can't the file be copied when it is dropped ?

Comment 70

[Thomas D. (:thomas8)]

The dataloss scenario of comment 69 lends further support to the fact that the minimal fix for this bug needs to make "immediate snapshot copy when attaching" the default behaviour (notwithstanding later implementation of new feature: alternative behaviour "attach later when sending" of bug 722929). For details, see comment 60, 4). Comment 69 confirms my analysis as an illustration of comment 60, 4a).

Comment 71

[Phil Lacy]

Attaching a file and separating it from original would be ok but we should show a timestamp in the attachment. 

Editing an attachment should not be an option in an email program. User needs to drag attachmennt out and edit separately whatever type of file it is. So we need to make attachment read only. Or just simply encode and embed in message when inserted. Not sure if that is possible at compose state of message.

Comment 72

[Thomas D. (:thomas8)]

FTR: The proposed solution for this bug 378046 is to get (or even embed) immediate snapshot copy of anything (files, images, messages...) as soon as it is attached to current composition (instead of just referencing such external parts and not having any control whether they will still exist or be the same when sending).

Starting from Bug 817245 Comment 12, Magnus Melin, WADA, and I have once again established and confirmed that the solution proposed by this bug 378046 will solve a lot of other dataloss and privacy bugs including Bug 817245. Setting dependency for ease of tracking.

Comment 73

[L A Walsh]

I would agree with the immediate snapshot, since, pray tell, if I hold down
the shift key, and drag my attachment over to the attachment window, the default
action would be to *move* the attachment (and delete the old copy).

If Tbird doesn't do something with a attachment to save it at the time it is attached, the file can easily be gone.  

Data loss issues are right up there with privacy violations -- but I think creating
a tmp file to add to an attachment and deleting the tmp is far more likely with a
DnD(move) GUI interface...

Comment 74

[Thomas D. (:thomas8)]

Bug 771059 reports another inconsistency because we don't have unified attachment routines for Mapi and non-Mapi methods as requested in this bug 378046:

Bug 771059 - Emails with attachment created using simple-mapi (sendto) don't trigger the file link attachment (aka Bigfiles)