241212 - Message window for .EML file needs envelope panel

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Mike Cowperthwaite]

Thanks to the patch for bug 239555, it is now possible to directly open .EML 
files from Mail/News.  However, when the window opens, it does not render like 
normal message window; altho it has the message window's toolbar and menu, it 
lacks the 'envelope panel' which shows the headers and the attachment list, if 
any.

Instead, the message displays exactly as if the file had been opened in the 
browser.  Headers are shown in an unexpandable, abbreviated form with some 
styling to set them apart from the message body; but without the special XUL 
rendering and menu for addresses.

Worse, attachments in the mail are not usefully represented.  The display of 
them is poor (subject to bug 174692), and they do not have the standard 
attachment context menus (nor the File|Attachments menu, but that will be in a 
separate bug).  Image attachments (shown broken, per 174692) will generate the 
'Image' context menu -- and the View item works for this -- but the Save item 
works on the message source, not on the image.  (The Block/Unblock menu item 
makes no sense here, but that is bug 237858.)

Comment 1

[Robert Accettura [:raccettura]]

cc--> bienvenu, the master.

This means making MsgOpenNewWindowForMessage smarter as I mentioned in bug 236637.

Comment 2

[Mike Cowperthwaite]

Additional note: without the envelope, the title of the window is always "Mail & 
Newsgroups" instead of the subject line of the message.

Comment 3

[Dennis Gesker]

There seem to be quite a few bugs relating to eml files not behaving as
expected. TB is the default email client on both WindowsXP and Windows2K PCs and
it does not seem possible to associate .eml files with TB so that when one
double clicks on the file the file opens in a ThunderBird window.

--drg

Comment 4

[Mike Cowperthwaite]

(In reply to comment #3)
> it does not seem possible to associate .eml files with TB so that when one
> double clicks on the file the file opens in a ThunderBird window.

Bug 242959 and bug 261559.

Comment 5

[Manuel Haim]

There's a workaround:

You'll find the "TB Attachment Tools" at
http://www.supportware.net/mozilla/#ext9

Install them and you'll have a new "Import" function in Thunderbird's "File"
menu. You can use it to import an .eml file to a mail folder. From within there,
you can view the .eml and even access the attachments.

Comment 6

[Mike Cowperthwaite]

(In reply to comment #5)
> There's a workaround:  [...]  "TB Attachment Tools"

I haven't installed it, but from the description, it appears to conflict with 
other changes for current trunk builds -- e.g., the "Delete All Attachments" 
feature is now part of TB.

Comment 7

[David :Bienvenu]

The first cause of this is that when opening a .eml file, we're running a file:
url, instead of a mailbox url. This prevents us from associating a msg window
with the msg url, which prevents mime from finding the header sink. So either we
need to cons up a mailbox url from the file url, or we need to be able to get
and set the msg window on the file url. We might want to have a mailbox url for
other reasons, but it might have tricky side effects too. I'll investigate.

Comment 8

[David :Bienvenu]

Attachment: proposed fix

this makes the header area display correctly by switching the file: url to a
mailbox url, and asking for the message at pos 0 in eml file. I also had to
change nsMailboxProtocol::SetupMessageExtraction to ask the mailbox url for the
message size directly first, and only if it doesn't know, get the size from the
msg hdr (we don't have a msg hdr in the .eml file case). And when running the
url, we set the message size based on the .eml file size.

What doesn't work with this patch is opening or saving attachments in .eml
files, for the same reason as above - we don't have a msg hdr with the message
size. To get that to work, I'm going to either need a msg header, or figure out
a way to play the same trick that we do when opening the original .eml file.

Comment 9

[David :Bienvenu]

*** Bug 253216 has been marked as a duplicate of this bug. ***

Comment 10

[David :Bienvenu]

Attachment: proposed fix

this patch uses a dummy header in msgHdrViewOverlay.js to store the message
size. With this patch, we can open .eml attachments/files in a stand-alone
message window, and open/save attachments within that message.

If I have time, I'll try to make reply/reply all work by populating the dummy
header from headers process in msgHdrViewOverlay.js, and using the dummy header
at reply time. I'd also like to get copy to a folder working.

Comment 11

[Scott MacGregor]

Comment on attachment 184692 [details] [diff] [review]
proposed fix

can we call is a DummyMsgHeader? 

this comment sounds like you didn't finish it:

// need to strip out ?type=x-message-display because it confuses

Comment 12

[David :Bienvenu]

I'll call it DummyMsgHeader and remove the "need to" part from the comment.

Comment 13

[David :Bienvenu]

Attachment: forgot these files...

Comment 14

[Scott MacGregor]

Comment on attachment 184704 [details] [diff] [review]
forgot these files...

bump the interface ID on nsIMimeMiscStatus

Comment 15

[chris hofmann]

Comment on attachment 184692 [details] [diff] [review]
proposed fix

a=chofmann

Comment 16

[chris hofmann]

Comment on attachment 184704 [details] [diff] [review]
forgot these files...

a=chofmann

Comment 17

[[e-mail]]

Attachment: Suite port

Comment 18

[David :Bienvenu]

Comment on attachment 185150 [details] [diff] [review]
Suite port

mDummyHeader is going to grow a bit to handle other stuff, if I have time :-)

Comment 19

[Mike Cowperthwaite]

verified fixed with TB 1.0+0603.  Thanks so much, David!  I'm looking forward to 
the fixes that will be possible now that this is in place.

I'd mark this bug verified except I haven't had a chance to check a suite build 
yet.

Comment 20

[[e-mail]]

Suite port checked in.

Comment 21

[Mike Cowperthwaite]

(In reply to comment #20)
> Suite port checked in.

I just checked Moz 1.8b2-0621, and using File|Open to select a .EML file results 
in a window with the 3-pane menus and toolbar, and otherwise blank -- definitely 
not the desired result.

Comment 22

[Mike Cowperthwaite]

(In reply to comment #21)
> (In reply to comment #20)
> > Suite port checked in.
> 
> I just checked Moz 1.8b2-0621, and using File|Open to select a .EML file
> results in a window with the 3-pane menus and toolbar, and otherwise blank --
> definitely not the desired result.

Still the case with SeaMonkey/1.0a-0806, Win2K -- and I'm also seeing the same 
thing with TB 1.0+0806, when running on Windows 98.  It works fine with TB under 
Win2K.

Comment 23

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #22)
> > I just checked Moz 1.8b2-0621, and using File|Open to select a .EML file
> > results in a window with the 3-pane menus and toolbar, and otherwise blank --
> > definitely not the desired result.
> 
> Still the case with SeaMonkey/1.0a-0806, Win2K -- and I'm also seeing the same 
> thing with TB 1.0+0806, when running on Windows 98.  It works fine with TB under 
> Win2K.

Error: uncaught exception: [Exception... "Component returned failure code:
0x80550006 [nsIMsgFolder.getMsgDatabase]"  nsresult: "0x80550006 (<unknown>)" 
location: "JS frame :: chrome://messenger/content/messageWindow.js :: CreateView
:: line 358" data: no]

Just pasting Mike's JS exception so we know _what's_ happening, but not _why_, yet.

Comment 24

[[e-mail]]

I've figured out that I can reproduce Mike's bug by loading Account Central for
Local Folders (or for any account except IMAP or news).
What I don't know is what the correct fix is.
a) GetMsgDatabase should always return NS_OK as per remote accounts
b) GetMsgDatabase returns special success codes instead of failure codes
c) Some JS callers have to be wrapped in try/catch just in case

Comment 25

[Mike Cowperthwaite]

(In reply to comment #24)
> I've figured out that I can reproduce Mike's bug by loading Account Central

Good catch, Neil -- once I selected an account, the message window opened fine. 
And the trunk version of TB is showing the same symptom -- I'd never tried 
opening a message with AC showing.

Does this problem need a new bug?

Comment 26

[David :Bienvenu]

I'm going to re-open this. It seems to have regressed for me, both on the 1.8
branch and the trunk. I can save an attachment, but can't open one.

Comment 27

[David :Bienvenu]

Attachment: enable opening of attachments from .eml files opened in stand-along window

use url instead of message uri.

Comment 28

[David :Bienvenu]

Attachment: fix case when account central is selected

Comment 29

[[e-mail]]

Comment on attachment 193489 [details] [diff] [review]
enable opening of attachments from .eml files opened in stand-along window

>+  PRInt32 typeIndex = urlString.Find("?type=application/x-message-display");
>+  if (typeIndex != kNotFound)
>+  {
>+    urlString.Cut(typeIndex, sizeof("?type=application/x-message-display") - 1);
>+    // we also need to replace the next '&' with '?'
>+    PRInt32 firstPartIndex = urlString.FindChar('&');
>+    if (firstPartIndex != kNotFound)
>+      urlString.SetCharAt('?', firstPartIndex);
If there is a character following "display" then it should be the & that you
seek...

>+    return docShell->LoadURI(URL, loadInfo, nsIWebNavigation::LOAD_FLAGS_NONE, PR_FALSE);
nsIWebNavigation::LOAD_FLAGS_IS_LINK perhaps?

Comment 30

[Scott MacGregor]

Comment on attachment 193489 [details] [diff] [review]
enable opening of attachments from .eml files opened in stand-along window

I don't know what the answer is to Neil's comment about using LOAD_FLAGS_LINK.
I know everywhere else where we are opening similar type urls, we pass in
LOAD_FLAGS_NONE in the mailnews.

Comment 31

[[e-mail]]

Comment on attachment 193490 [details] [diff] [review]
fix case when account central is selected

>     // this is a hack to make opening a stand-alone msg window on a 
>@@ -392,6 +393,12 @@
>     // of not having a folder.
Since you didn't use enough -u I shall have to quote the entire comment:
// this is a hack to make opening a stand-alone msg window on a 
// .eml file work. We use a search view since its much more tolerant
// of not having a folder.
Maybe tweak MsgOpenFromFile to actually trigger this hack?

Comment 32

[David :Bienvenu]

Attachment: fix inspired by Neil.

pass in null for the folder uri by calling window.openDialog directly.

Comment 33

[David :Bienvenu]

Comment on attachment 193509 [details] [diff] [review]
fix inspired by Neil.

I think Neil is going to prefer this patch to the other patch, Scott, so I
think this is the one I'd like sr'ed and checked into the trunk for tbird.

Comment 34

[[e-mail]]

Comment on attachment 193509 [details] [diff] [review]
fix inspired by Neil.

Do you still need to pass gDBView? Also, when I ported the fix to Suite I also
had to make three other changes which Thunderbird might or might not have.

Comment 35

[[e-mail]]

Attachment: Suite port (checked in trunk/branch 1.8 & 1.8.0)

Comment 36

[David :Bienvenu]

Comment on attachment 193593 [details] [diff] [review]
Suite port (checked in trunk/branch 1.8 & 1.8.0)

there is no nsMsgViewType.eSearch - it's eShowSearch, at least in my tree.
Tbird already has that code (modulo eSearch->eShowSearch)

the tbird OnLoadMessageWindowDelayed  starts like this:

  if (loadCustomMessage)
  {
    gDBView.suppressMsgDisplay = false;

and I don't know about the third change, I didn't need to do that. Re passing
in the view, passing in null is ok, and may be better, I don't know - if you
pass in a view, we'll clone it, which is pretty fast, but creating a search
view that you don't use is probably faster.

Comment 37

[David :Bienvenu]

Comment on attachment 193489 [details] [diff] [review]
enable opening of attachments from .eml files opened in stand-along window

this fix is not without its terrors since attachment handling is always fun.
The effect should be limited to opening attachments from local mail messages,
and I've tested it for a while. My fear is that if we don't take this fix for
1.5, we'll get a ton of bugs filed on this once people discover 1.5's improved
.eml file handling.

Comment 38

[David :Bienvenu]

reclosing, fixes checked in.

Comment 39

[Scott MacGregor]

Comment on attachment 193509 [details] [diff] [review]
fix inspired by Neil.

approving for the 1.8b4 branch. Low risk change.

Comment 40

[Scott MacGregor]

Comment on attachment 193489 [details] [diff] [review]
enable opening of attachments from .eml files opened in stand-along window

make sure bz's mime type change for application/x-message-display is on the
branch before checking this in. It looks to me like it is in the 1.8 branch.

Comment 41

[Mike Cowperthwaite]

Altho the message window is opening correctly, the JavaScript console is full of 
errors: there are multiple instances (exact number appears to depend on the 
message, i.e. whether it has an attachment) of:
===
Error: [Exception... "'JavaScript component does not have a method named: 
"getStringProperty"' when calling method: [nsIMsgDBHdr::getStringProperty]"  
nsresult: "0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)"  location: 
"<unknown>"  data: no]
===
followed by one each of:
===
Error: [Exception... "Component returned failure code: 0x80004005 
(NS_ERROR_FAILURE) [nsIMsgMailNewsUrl.folder]"  nsresult: "0x80004005 
(NS_ERROR_FAILURE)"  location: "JS frame :: chrome://messenger/content/
mailWindowOverlay.js :: OnMsgLoaded :: line 2243"  data: no]
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 2243

Error: [Exception... "Component returned failure code: 0x80004005 
(NS_ERROR_FAILURE) [nsIMsgMailNewsUrl.folder]"  nsresult: "0x80004005 
(NS_ERROR_FAILURE)"  location: "JS frame :: chrome://messenger/content/
phishingDetector.js :: isMsgEmailScam :: line 24"  data: no]
Source File: chrome://messenger/content/phishingDetector.js
Line: 24
===

TB 1.5b1-0904, TB 1.6a1-0830, Win2K

David, do you want to reopen this, or should I open a new bug?

Comment 42

[David :Bienvenu]

were those warnings fixed by another bug?

Comment 43

[Mike Cowperthwaite]

(In reply to comment #42)
> were those warnings fixed by another bug?

Are you saying you don't see the errors?  (They're not warnings.)  If so, which 
build are you using?

(In reply to comment #41)
> TB 1.5b1-0904, TB 1.6a1-0830, Win2K

I can update this to: 1.6a1-0904 shows the errors.  I haven't gotten a new build 
since 9/4 because there have been hardly any mail-related patches checked in 
since 9/4, none of any interest to me.  (Getting a new build is nontrivial for 
those of us on dialup.)

Comment 44

[David :Bienvenu]

I do see these warnings, even after updating. It might depend on the message. I
wasn't seeing so many of them on other messages.

Comment 45

[Karsten Düsterloh]

Comment on attachment 193593 [details] [diff] [review]
Suite port (checked in trunk/branch 1.8 & 1.8.0)

Neil, is this patch still needed? Opened .eml files are shown as expected with
my current trunk build, even without the patch.

Regardless of the patch being applied or not, I can see the warnings mentioned
in comment 41, but only if a folder (not an account) is selected; switching
View->Headers->* doesn't work then.
If an account is selected, the message windows open, but doesn't display the
message...

Comment 46

[Mike Cowperthwaite]

(In reply to comment #45)
> Regardless of the patch being applied or not, I can see the warnings mentioned
> in comment 41, but only if a folder (not an account) is selected; switching
> View->Headers->* doesn't work then.

With TB 1.5b1-0904, I don't see any difference between opening with an account 
selected and with a folder selected -- many error messages displayed.  (David 
supplied a patch at bug 268746 to correct for the missing-getStringProperty-
method errors, checked in today.)

Most of the View menu doesn't work, and hasn't since opening a .EML file was 
first implemented; see bug 241213.

Comment 47

[Mike Cowperthwaite]

(In reply to comment #46)
> (David supplied a patch at bug 268746 to correct for the missing-
> getStringProperty- method errors, checked in today.)

And it's working, in today's nightly TB build; but the other two errors from 
comment 41 still exist.

Comment 48

[Karsten Düsterloh]

Comment on attachment 193593 [details] [diff] [review]
Suite port (checked in trunk/branch 1.8 & 1.8.0)

Okay, the patch *does* make a difference insofar as you can't open .emls
without it if just an accout is selected...
(And don't forget to change the eSearchs into eShowSearch on checkin! ;-) )

Comment 49

[Mike Cowperthwaite]

*** Bug 239685 has been marked as a duplicate of this bug. ***

Comment 50

[Mike Cowperthwaite]

I'm a little confused as to the state of the patches, but: Seamonkey 1.0a-0910 
(the most recent 1.0a I could find on ftp.mozilla.org) still has the problem of 
not displaying the message if an account, rather than a folder, is selected in 
the Folder pane.

Comment 51

[[e-mail]]

(In reply to comment #50)
>I'm a little confused as to the state of the patches, but: Seamonkey 1.0a-0910 
>(the most recent 1.0a I could find on ftp.mozilla.org) still has the problem of 
>not displaying the message if an account, rather than a folder, is selected in 
>the Folder pane.
That's because the branch doesn't have the Suite port.

Comment 52

[Ian Neal]

Comment on attachment 193593 [details] [diff] [review]
Suite port (checked in trunk/branch 1.8 & 1.8.0)

Only TB version has been checked into branch, not port version

Comment 53

[Ian Neal]

Comment on attachment 193593 [details] [diff] [review]
Suite port (checked in trunk/branch 1.8 & 1.8.0)

Checking in (branch 1.8)
commandglue.js;
new revision: 1.258.4.3; previous revision: 1.258.4.2
mailWindowOverlay.js;
new revision: 1.222.2.5; previous revision: 1.222.2.4
messageWindow.js;
new revision: 1.105.2.3; previous revision: 1.105.2.2
done
Checking in (branch 1.8.0)
commandglue.js;
new revision: 1.258.10.2; previous revision: 1.258.10.1
mailWindowOverlay.js;
new revision: 1.222.2.3.4.2; previous revision: 1.222.2.3.4.1
messageWindow.js;
new revision: 1.105.2.1.4.2; previous revision: 1.105.2.1.4.1
done

Comment 54

[Mike Cowperthwaite]

Regarding the errors cited in comment 41: the 'getStringProperty()' errors have been fixed [bug 268746]; I've opened bug 338536 for the other two.

Comment 55

[Simon Paquet [:sipaq]]

Bug was fixed in SM 1.0