468740 - Text in compose window is deleted when attachment is openend

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Composition, defect)

Description

[mick22]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20081129 SeaMonkey/2.0a2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20081129 SeaMonkey/2.0a2pre

1. Write some text into a new email in "Mail & Newsgroup"
2. Attach a file to the new email
3. Right-click on the attachment to open it
4. The text written in step 1 is deleted and you have to write it again

I don't understand why the email window is cleared when you open an attachment. Checking attachment before actually sending the message is completely normal. Nobody would expect that this action will also delete the message you just have written.

Reproducible: Always

Steps to Reproduce:
1. Write some text into a new email in "Mail & Newsgroup"
2. Attach a file to the new email
3. Right-click on the attachment to open it
4. The text written in step 1 is deleted and you have to write it again
Actual Results:  
Attachment is openend but the message text is deleted.

Expected Results:  
Attachment is opened and the message remains.

I rated this a major bug because this behaviour is completely unexpected to most users and the text is really gone. You cannot get it back with Undo.

Comment 1

[lenochod]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081216 SeaMonkey/2.0a3pre

Confirming, on trunk, Win XP SP3.

Comment 3

[Karsten Düsterloh]

I can confirm this for current Linux and Mac trunk:
- In the HTML editor, both the entire addressing widget and the mail body text
get cleared
-In the plaintext editor, only the entire addressing widget gets cleared, the
mail body remains untouched
The subject stays untouched in both editors.

Comment 4

[[e-mail]]

I think we may be able to resolve this by copying the openURL function from toolkit's contentAreaUtils.js and calling that instead.

Comment 5

[Karsten Düsterloh]

The problem seems to be in OpenSelectedAttachment(), where we use the current editor element to open the attachment, hence the gMsgEditorCreationObserver gets notified of the attachment document load and clears the editor fields...

Neuos, are you interested in taking this?

Comment 6

[Krang]

Sure, I will look into this and have a patch ready soon.

Comment 7

[Krang]

Attachment: Use a different method

Here is a patch to use a different method to open the attachment that doesn't have the side effect of clearing the compose window.  As Neil suggested in comment #4, I based this off openURL.

Comment 8

[[e-mail]]

Comment on attachment 354035 [details] [diff] [review]
Use a different method

>+            return loadgroup;
You don't define this.

Based on Mnyromyr's comment there is in fact a better way: simply remove the editor creation observer once it's observed the editor's creation.

Comment 9

[Krang]

Attachment: Remove the observer

I tried removing the command observer right after the editor is created, and it fixes this bug, but it has the strange side effect of removing some of the blue grid lines from the To/CC/BCC pane in the compose window (on WinXP, at least).

Here is a patch that attempts to remove it right before loading the attachment, which doesn't have the side effect.  Not sure if it's acceptable to keep trying to remove the observer, though.

Comment 10

[[e-mail]]

(In reply to comment #9)
> I tried removing the command observer right after the editor is created, and it
> fixes this bug, but it has the strange side effect of removing some of the blue
> grid lines from the To/CC/BCC pane in the compose window (on WinXP, at least).
Have you tried using a timeout? (A bit hacky, I guess)

Comment 11

[Krang]

Attachment: Remove the observer v2

(In reply to comment #10)
> Have you tried using a timeout? (A bit hacky, I guess)

Good idea.  Here is a patch that removes the observer after a one-second timeout (I originally tried 100ms, but it only worked sporadically, so I thought 1000ms would be safer).

Comment 12

[Krang]

(In reply to comment #11)
> I originally tried 100ms, but it only worked sporadically

BTW, by "worked," I mean that it correctly created the dummy rows in the compose window.

Comment 13

[[e-mail]]

Comment on attachment 355077 [details] [diff] [review]
Remove the observer v2

That's not going to work, you need to remove the command manager when the "obs_documentCreated" event occurs (i.e. in the observer itself).

Comment 14

[Krang]

Attachment: Remove the observer v3

(In reply to comment #13)
> (From update of attachment 355077 [details] [diff] [review])
> That's not going to work, you need to remove the command manager when the
> "obs_documentCreated" event occurs (i.e. in the observer itself).

This updated patch moves the observer removal to the correct place.  No problems this time with the dummy rows, so no timeout was needed.

Comment 15

[Serge Gautherie (:sgautherie)]

Assignee, may I suggest you to file your name or at least an alias in your Bugzilla preferences ?

Fwiw, it looks like TB file may need the same fix...

Comment 16

[Krang]

(In reply to comment #15)
> Assignee, may I suggest you to file your name or at least an alias in your
> Bugzilla preferences ?

I had previously used an alias in that field, but judging by what was said in IRC, it seems that SeaMonkey devs generally dislike that.  Please use just my email address for checkin.

> Fwiw, it looks like TB file may need the same fix...

I don't think TB is affected by this bug since it uses different code for opening attachments (see bug 182121).

Comment 17

[[e-mail]]

(In reply to comment #16)
> (In reply to comment #15)
> > Assignee, may I suggest you to file your name or at least an alias in your
> > Bugzilla preferences ?
> I had previously used an alias in that field, but judging by what was said in
> IRC, it seems that SeaMonkey devs generally dislike that.  Please use just my
> email address for checkin.
Actually that's worse, since Mercurial then extracts your "name" from your email address, which in this case will result in "Bugzilla".

Comment 18

[Krang]

(In reply to comment #17)
> Actually that's worse, since Mercurial then extracts your "name" from your
> email address, which in this case will result in "Bugzilla".

Oh, that's true.  In that case, it's back to using an alias for me, although I'll use my more traditional one instead of going back to Neuos.

Comment 19

[[e-mail]]

Pushed changeset 6c0865b7fe42 to comm-central.