Closed Bug 468740 Opened 13 years ago Closed 13 years ago

Text in compose window is deleted when attachment is openend

Categories

(SeaMonkey :: MailNews: Composition, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mick22, Assigned: bugzilla)

References

Details

(Keywords: regression)

Attachments

(1 file, 3 obsolete files)

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.
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-seamonkey2?
Version: unspecified → Trunk
Duplicate of this bug: 470206
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.
OS: Windows Vista → All
Hardware: x86 → All
I think we may be able to resolve this by copying the openURL function from toolkit's contentAreaUtils.js and calling that instead.
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?
Sure, I will look into this and have a patch ready soon.
Assignee: nobody → neuos
Status: NEW → ASSIGNED
Attached patch Use a different method (obsolete) — Splinter Review
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.
Attachment #354035 - Flags: review?(neil)
Attachment #354035 - Flags: review?(neil) → review-
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.
Attached patch Remove the observer (obsolete) — Splinter Review
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.
Attachment #354035 - Attachment is obsolete: true
Attachment #354132 - Flags: review?(neil)
(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)
Attachment #354132 - Attachment description: Use a different method v2 → Remove the observer
Attachment #354132 - Attachment is obsolete: true
Attachment #354132 - Flags: review?(neil)
Attached patch Remove the observer v2 (obsolete) — Splinter Review
(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).
Attachment #355077 - Flags: review?(neil)
(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.
Attachment #355077 - Flags: review?(neil) → review-
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).
(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.
Attachment #355077 - Attachment is obsolete: true
Attachment #355208 - Flags: superreview?(neil)
Attachment #355208 - Flags: review?(neil)
Attachment #355208 - Flags: superreview?(neil)
Attachment #355208 - Flags: superreview+
Attachment #355208 - Flags: review?(neil)
Attachment #355208 - Flags: review+
Keywords: checkin-needed
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...
(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).
Keywords: regression
(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".
(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.
Whiteboard: [patch has r+sr, needs checkin]
Pushed changeset 6c0865b7fe42 to comm-central.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [patch has r+sr, needs checkin]
Flags: blocking-seamonkey2?
You need to log in before you can comment on or make changes to this bug.