Last Comment Bug 665313 - Opening message attachment does not work
: Opening message attachment does not work
Status: RESOLVED FIXED
: regression
Product: SeaMonkey
Classification: Client Software
Component: MailNews: General (show other bugs)
: SeaMonkey 2.4 Branch
: All All
: -- normal (vote)
: seamonkey2.4
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-18 12:31 PDT by Stanimir Stamenkov
Modified: 2012-06-27 06:07 PDT (History)
8 users (show)
iann_bugzilla: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
Sample message (1.50 KB, message/rfc822)
2011-06-18 12:33 PDT, Stanimir Stamenkov
no flags Details
Fix opening attached messages from the compose window (956 bytes, patch)
2011-06-22 16:51 PDT, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Special-case attached messages (1.13 KB, patch)
2011-06-23 04:24 PDT, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Alternate approach [Checked in: Comment 20 and Comment 21] (1.42 KB, patch)
2011-06-23 04:40 PDT, neil@parkwaycc.co.uk
mnyromyr: review+
iann_bugzilla: approval‑comm‑aurora+
iann_bugzilla: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Stanimir Stamenkov 2011-06-18 12:31:04 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20110608 SeaMonkey/2.1
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20110608 SeaMonkey/2.1

Opening a message attachment doesn't seem to work since SeaMonkey 2.1 (used to work fine with SeaMonkey 2.0).  Opening simple message attachment causes Bug 665310.  Opening message attachment which has a ZIP attachment on itself makes a browser window opened with what it seems to be the raw dump of the ZIP content - starts with "PK".


Reproducible: Always

Steps to Reproduce:
1. Create a new message and attach a ZIP file to it, then save as draft;
2. Create another new message and attach the previous saved draft message to it, and save as draft (or send to self);
3. Try opening the attached message from the Attachments box from the preview of the second message.


Actual Results:  
A browser window showing the raw content of the ZIP is opened.


Expected Results:  
A mail message window showing the message attachment should be opened.
Comment 1 Stanimir Stamenkov 2011-06-18 12:33:15 PDT
Created attachment 540261 [details]
Sample message

A message with a message attachment, with a ZIP attachment on its own.
Comment 2 Stanimir Stamenkov 2011-06-18 12:36:44 PDT
The same issue is observed with current SeaMonkey 2.2, 2.3a2 and 2.4a1 builds, also.
Comment 3 Jens Hatlak (:InvisibleSmiley) 2011-06-18 15:56:25 PDT
Confirming with SM 2.1 and trunk on Windows and Linux. Worked with SM 2.0.

I could not reproduce this with Thunderbird (Miramar, Aurora, trunk), so this seems to be SM-only.

It would be interesting to know when this broke; we need a regression window. So far I checked:
Last known good: SM 2.1a3 (Gecko/20100815)
First bad: SM 2.1b1 (Gecko/20101008)
Since TB is not affected, I assume what broke it was an SM-only change or one that fixed a part only for TB. In both cases the culprit would probably be found on the comm-central hg log.

Alternatively one could try to start from the code that is responsible for deciding whether to open a MailNews standalone window or browser window (wherever that code is).
Comment 4 Jens Hatlak (:InvisibleSmiley) 2011-06-18 16:32:48 PDT
Last known good: 2010-09-29
First bad: 2010-09-30

http://hg.mozilla.org/comm-central/pushloghtml?startdate=2010-09-28+00%3A00%3A00&enddate=2010-09-30+23%3A59%3A59

In between lies http://hg.mozilla.org/comm-central/rev/e19b069946a4 (bug 599119) which sounds suspicious to me because the check-in comment mentions message/rfc822. Touches only files under mailnews/ so I wonder why the problem didn't appear in TB. Because we have a browser? :-/

Could also be one of the others in the above time range, like http://hg.mozilla.org/comm-central/rev/098388acf16b (bug 410613).

Karsten, Neil, ideas?
Comment 5 neil@parkwaycc.co.uk 2011-06-19 03:24:50 PDT
URL of attached file inside attached message:
mailbox:///C|/Users/Neil/AppData/Roaming/Mozilla/SeaMonkey/Profiles/salted.default/Mail/Local%20Folders/Unsent%20Messages?number=31089&part=1.2.1.2&filename=logo.gif
The MIME type of this URL appears to be image/gif as expected.

URL of attached message:
mailbox:///C|/Users/Neil/AppData/Roaming/Mozilla/SeaMonkey/Profiles/salted.default/Mail/Local%20Folders/Unsent%20Messages?number=31089&part=1.2&filename=about%3Alogo.eml
The MIME type of this URL appears to be text/html and the content appears to be the raw attachment. The source of this URL is the correct message source.

If I append "&type=message/rfc822" to the URL then the message opens correctly.
Comment 6 neil@parkwaycc.co.uk 2011-06-19 03:56:18 PDT
Also, the url of the message itself,
mailbox:///C|/Users/Neil/AppData/Roaming/Mozilla/SeaMonkey/Profiles/salted.default/Mail/Local%20Folders/Unsent%20Messages?number=31089
opens correctly in the browser.

Note that we can work around the problem by forcing a message attachment to open in a standalone message window. (Currently the mail content viewer claims to be able to open message/rfc822 in the browser which is why we try.)
Comment 7 neil@parkwaycc.co.uk 2011-06-20 02:48:24 PDT
In 2.0, isTypeSupported("message/rfc822") returns UNSUPPORTED...
Comment 8 Jens Hatlak (:InvisibleSmiley) 2011-06-20 03:57:05 PDT
(In reply to comment #7)
> In 2.0, isTypeSupported("message/rfc822") returns UNSUPPORTED...

Then it probably really was bug 599119 that changed it. From MailNewsDLF.h:

"This factory is a thin wrapper around the text/html loader factory. All it
does is convert message/rfc822 to text/html and delegate the rest of the
work to the text/html factory."
Comment 9 neil@parkwaycc.co.uk 2011-06-20 06:11:10 PDT
So, we would seem to have two options:
1. Find all the places in the UI that might open messages and add code to check the content type and open a message window if appropriate
2. #ifdef the mail news document viewer factory
I don't know what effect the plugin scan has on SeaMonkey.
Comment 10 Jens Hatlak (:InvisibleSmiley) 2011-06-22 16:03:45 PDT
Relnoted, see bug 656719 comment 32 and bug 665682 comment 9.
Comment 11 neil@parkwaycc.co.uk 2011-06-22 16:51:59 PDT
Created attachment 541228 [details] [diff] [review]
Fix opening attached messages from the compose window

I looked to see what the compose window did and it special-cases message attachments. Sadly the code gets it wrong and the message doesn't display.
Comment 12 Jens Hatlak (:InvisibleSmiley) 2011-06-23 02:09:08 PDT
(In reply to comment #11)
> Created attachment 541228 [details] [diff] [review] [review]
> Fix opening attached messages from the compose window
> 
> I looked to see what the compose window did and it special-cases message
> attachments. Sadly the code gets it wrong and the message doesn't display.

I take it this is only for the compose window, so what are the STR for this? If I just open a compose window and d&d a message with a ZIP attachment to its Attachments area, I can open the attached message and the ZIP file attached to it with or without this patch. :-/

[Note: You didn't target your feedback request.]
Comment 13 neil@parkwaycc.co.uk 2011-06-23 04:16:11 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > Created attachment 541228 [details] [diff] [review]
> > I looked to see what the compose window did and it special-cases message
> > attachments. Sadly the code gets it wrong and the message doesn't display.
> I take it this is only for the compose window, so what are the STR for this?
> If I just open a compose window and d&d a message with a ZIP attachment to
> its Attachments area, I can open the attached message and the ZIP file
> attached to it with or without this patch. :-/
Doesn't work when I try it. Doesn't work for forwarding messages either. (The forwarded/dropped message doesn't need to have an attachment.)
Comment 14 neil@parkwaycc.co.uk 2011-06-23 04:24:11 PDT
Created attachment 541335 [details] [diff] [review]
Special-case attached messages

This puts as back to the 2.0 behaviour where we pass messages off to the messenger to open them in a message window.
Comment 15 neil@parkwaycc.co.uk 2011-06-23 04:40:59 PDT
Created attachment 541338 [details] [diff] [review]
Alternate approach [Checked in: Comment 20 and Comment 21]

This makes it clearer that we're opening an attached message.
Comment 16 Jens Hatlak (:InvisibleSmiley) 2011-06-23 04:47:24 PDT
(In reply to comment #13)
> Doesn't work when I try it.

I tried on the Drafts folder of an IMAP server, maybe that's relevant.
Comment 17 neil@parkwaycc.co.uk 2011-06-23 06:23:48 PDT
Could be; I was using a local folder, so the URL code is different...
Comment 18 Karsten Düsterloh 2011-06-25 17:45:58 PDT
Comment on attachment 541228 [details] [diff] [review]
Fix opening attached messages from the compose window

>diff --git a/suite/mailnews/compose/MsgComposeCommands.js b/suite/mailnews/compose/MsgComposeCommands.js
>-        var folderUri = msgHdr.folder.folderURL;
>+        var folderUri = msgHdr.folder.URI;

We only get here when forwarding.
If I take Stan's attachment, turn it into a mailbox file and "Message→Edit as New" that message, the attachment url will be "file:///tmp/nsmail-2.eml" and I don't get here at all. Maybe OpenSelectedAttachment() could just use gMessenger.openAttachment() as well instead of its own URL voodoo?
Comment 19 Karsten Düsterloh 2011-06-25 18:00:16 PDT
Comment on attachment 541338 [details] [diff] [review]
Alternate approach [Checked in: Comment 20 and Comment 21]

I'm in favour of this approach.

>diff --git a/suite/mailnews/msgHdrViewOverlay.js b/suite/mailnews/msgHdrViewOverlay.js
>+  switch (this.contentType)
>+  {
>+    // As of bug 599119, isTypeSupported returns true for messages, but
>+    // attached messages don't open reliably in the browser, so pretend
>+    // they're not supported and open a message window for them instead.

I would have phrased this along the lines of "the messenger is more qualified to show messages". *shrug*

>+    case "message/rfc822":
>+      var url = this.url + "&type=application/x-message-display";
>+      window.openDialog("chrome://messenger/content/messageWindow.xul",
>+                        "_blank", "all,dialog=no",
>+                        Services.io.newURI(url, null, null));

Please add a return statement here, lest someone wonders later if the fall-through was intentional.
Comment 20 neil@parkwaycc.co.uk 2011-06-26 12:37:20 PDT
Comment on attachment 541338 [details] [diff] [review]
Alternate approach [Checked in: Comment 20 and Comment 21]

Pushed changeset f25b9ef3514b to comm-central.

If anyone approves this, please transplant it, I don't have any a/b trees.
Comment 22 Philip Chee 2011-06-26 21:38:58 PDT
IanN:
> http://hg.mozilla.org/releases/comm-beta/rev/1167a08ced50
This didn't seem to have made it to the SEAMONKEY_2_2b2_RELEASE release branch.
Comment 23 Justin Wood (:Callek) (Away until Aug 29) 2011-07-26 16:07:32 PDT
This landed on comm-release in time for SeaMonkey 2.2 final

http://hg.mozilla.org/releases/comm-release/rev/2d4177a29472?revcount=120
Comment 24 Stéphane Grégoire (yamo) 2011-08-08 01:19:54 PDT
Hi!
I've seen that Seamonkey 2.3 fixe this bug does it solve bug #621665 ?
Comment 25 Thomas Oskk 2012-06-21 10:50:08 PDT
On version 2.10.1 it has recurred, as it seems. The previous version worked fine, but now I can open attachments only by selecting "open" from the right-click menu. Doubleclicking does not work any more.
Comment 26 seamon2 2012-06-27 06:07:22 PDT
Confirm that it happens again.
(Build-Identifikator: Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20120604 Firefox/13.0 SeaMonkey/2.10)

Note You need to log in before you can comment on or make changes to this bug.