1814748 - Email attachments display empty/blank, apparently due to unexpected URI format

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect, P2)

Description

[gene smith]

With TB 111 self-built daily from comm-central, any attempt to open an attached email produces only a blank window instead of showing the email. This occurs for imap when the message is in offline store or in disk cache. It also occurs when the message is in Local Folders. It appears to be due to an unexpected URI string causing the message key to not be found.

For imap the following prints to the terminal console when the .eml attachment is clicked:

gds: uriStr = |imap://[e-mail]:993/fetch%3EUID%3E.INBOX%3E555?part=1.1.3&type=application/x-message-display&filename=postacert.eml|
[Parent 890880, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005 (NS_ERROR_FAILURE): file /home/gene/mozilla2/comm/mailnews/imap/src/nsImapService.cpp:804
[Parent 890880, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005 (NS_ERROR_FAILURE): file /home/gene/mozilla2/comm/mailnews/imap/src/nsImapService.cpp:3055
JavaScript error: chrome://messenger/content/aboutMessage.js, line 130: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgMessageService.messageURIToMsgHdr]

The first line comes from my addition to nsImapUtils.cpp:

diff --git a/mailnews/imap/src/nsImapUtils.cpp b/mailnews/imap/src/nsImapUtils.cpp
--- a/mailnews/imap/src/nsImapUtils.cpp
+++ b/mailnews/imap/src/nsImapUtils.cpp
@@ -34,18 +34,19 @@ nsresult nsImapURI2FullName(const char* 
   return NS_OK;
 }
 
 /* parses ImapMessageURI */
 nsresult nsParseImapMessageURI(const nsACString& uri, nsCString& folderURI,
                                uint32_t* key, char** part) {
   if (!key) return NS_ERROR_NULL_POINTER;
 
   const nsPromiseFlatCString& uriStr = PromiseFlatCString(uri);
+  printf("gds: uriStr = |%s|\n", uriStr.get());
   int32_t folderEnd = -1;

When I run with the same patch on 110 daily, there are no errors and it prints this URI:

imap-message://[e-mail]/INBOX#555?part=1.1.3&type=message/rfc822&filename=postacert.eml

compared to the unexpected URI from 111:

imap://[e-mail]:993/fetch%3EUID%3E.INBOX%3E555?part=1.1.3&type=application/x-message-display&filename=postacert.eml

The important difference is the "key separator" for 110 is "#" but for 111 it is "%3E".
There is a similar problem with the URI when the message and .eml attachment are opened from Local Folders.

Comment 1

[gene smith]

I could be totally wrong but maybe bug 1799764 is regressing this since it involves a stand alone window that displays the eml attachment.

Maybe off topic, but seems like you should be able to open an eml attachment into a tab, but it seems like it can only go to a new stand alone window.

Comment 2

[gene smith]

Actually not the regression I assumed above. Still may be a regression by something. It seems to be a threading issue maybe similar to bug 1801067.
Edit: I was wrong, he code below has no effect on this bug.
If I don't have this change in the mozilla code I see a one time assert crash and .eml attachments show as blank:

diff --git a/xpcom/base/nsISupportsImpl.cpp b/xpcom/base/nsISupportsImpl.cpp
--- a/xpcom/base/nsISupportsImpl.cpp
+++ b/xpcom/base/nsISupportsImpl.cpp
@@ -35,17 +35,19 @@ nsresult NS_FASTCALL NS_TableDrivenQI(vo
 
 #ifndef XPCOM_GLUE_AVOID_NSPR
 #  ifdef MOZ_THREAD_SAFETY_OWNERSHIP_CHECKS_SUPPORTED
 nsAutoOwningThread::nsAutoOwningThread() : mThread(PR_GetCurrentThread()) {}
 
 void nsAutoOwningThread::AssertCurrentThreadOwnsMe(const char* msg) const {
   if (MOZ_UNLIKELY(!IsCurrentThread())) {
     // `msg` is a string literal by construction.
+#if 0  // gds: stop crashing?
     MOZ_CRASH_UNSAFE(msg);
+#endif
   }
 }
 
 bool nsAutoOwningThread::IsCurrentThread() const {
   return mThread == PR_GetCurrentThread();
 }
 
 nsAutoOwningEventTarget::nsAutoOwningEventTarget()

Comment 3

[Geoff Lankow (:darktrojan)]

I'm aware of this. It's complicated, not least because there's no nsIMsgDBHdr for such messages, which makes some things impossible. Not currently high on my list of priorities, but I do have some potential ways to fix it.

Comment 4

[gene smith]

The diff in comment 2 has no effect on this bug I have since determined.

Comment 5

[Wayne Mery (:wsmwk)]

Do we know the regression bug# ?

Comment 6

[Geoff Lankow (:darktrojan)]

It is caused by bug 1799764 and some related stuff.

Comment 7

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1814748 - Display email attachments by saving to disk then opening. r=#thunderbird-reviewers

This is the same approach as we use to view encrypted parts of messages, so I think it's acceptable.

Comment 8

[Geoff Lankow (:darktrojan)]

I have abandoned my original over-complicated approach to this bug in favour of just saving the attachments to disk and then opening them as regular email files. This solves a lot of problems.

I'd rather not have used the disk, but to load things from a string in memory is much more complicated than it looks. Maybe I will come back to that one day.

Comment 9

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1814748 - Fix APIs for display of external messages. r=john.bieling

This reflects the fact that we use a real message header object for external messages now.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/13f0598b5f3c
Display email attachments by saving to disk then opening. r=mkmelin

Comment 11

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1814748 - Replace mistakenly removed linter imports. r=#thunderbird-reviewers

Also updates imports for globalOverlay.js, which I should've done in bug 1816341.

Comment 12

[Geoff Lankow (:darktrojan)]

Backed it out, too much of a mess:
https://hg.mozilla.org/comm-central/rev/ec1dddcdc867fe9c31aed5b377d5cc5eb255062c

Comment 13

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1814748 - Let messageDisplay API open external messages in tabs. r=john.bieling

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1b437e7aa80e
Display email attachments by saving to disk then opening. r=mkmelin

Comment 15

[Wayne Mery (:wsmwk)]

Comment on attachment 9319379 [details]
Bug 1814748 - Display email attachments by saving to disk then opening. r=#thunderbird-reviewers

[Triage Comment]
Approved for beta

Comment 16

[Wayne Mery (:wsmwk)]

NI John so we don't lose track of the API reviews

Comment 17

[Geoff Lankow (:darktrojan)]

Comment on attachment 9319379 [details]
Bug 1814748 - Display email attachments by saving to disk then opening. r=#thunderbird-reviewers

I'm scratching this one as it's tied to other patches we're not uplifting. It can wait until the merge next week.

Comment 18

[gene smith]

I guess this is still in work but I'm not sure what the final behavior is intended to be.

Right now, with current trunk, I can click on a "simple" email attachment (contains no other attachments) and it opens in a tab. To open in its own window I have to move the tab to its own window. Will there be a way to open the attached .eml in its own window initially?

But if I try to open a more "complex" attached email (that contains other .eml attachments and other attachments like .pdf or .png) I only see a blank new tab. I have to save the attached .eml to filesystem and then open it in "daily", then it appears in its own window, not in a tab.

Comment 19

[Geoff Lankow (:darktrojan)]

(In reply to gene smith from comment #18)

Right now, with current trunk, I can click on a "simple" email attachment (contains no other attachments) and it opens in a tab. To open in its own window I have to move the tab to its own window. Will there be a way to open the attached .eml in its own window initially?

Opening messages from files and attachments now follows the same behaviour as opening ordinary messages, because I fixed bug 499922.

But if I try to open a more "complex" attached email (that contains other .eml attachments and other attachments like .pdf or .png) I only see a blank new tab. I have to save the attached .eml to filesystem and then open it in "daily", then it appears in its own window, not in a tab.

Works for me, although I did have to go into the settings and change the default PDF handler back to Thunderbird (I don't recall changing it in the past but probably did). Opening a message attachment should now work exactly the same as opening a message from the file system, because that's exactly what we're doing.

Comment 20

[Geoff Lankow (:darktrojan)]

(In reply to gene smith from comment #18)

I have to save the attached .eml to filesystem and then open it in "daily", then it appears in its own window, not in a tab.

Oh right, I meant loading in the tab worked for me, and loading from the file system using the File menu. Opening from Windows Explorer or whatever will still open a window, because that runs through another bit of code altogether.

Comment 21

[gene smith]

I couldn't duplicate the problem with a simple "email in email in email" example. But I can duplicate it with this:
https://drive.google.com/file/d/1BTQg3B6V5S3O9RSlP9dZzasOgO3fa1PQ/view?usp=sharing Edit: link invalid, bug fixed at bug 1838347
It's a very large message in mbox format like I've used to test my recent disk caching and fetching changes for imap.

Edit: bug 1821205 is fixed so you can just drag the file into imap folder.
Recent builds have another bug where you can't just drop a .eml file into a folder (bug 1821205) so I had to create a new empty folder in Local Folders (in mbox format) called "test-big-one". Then put the file downloaded from the above temporary link (called test-big-one) in the Local Folder profile storage beside the file test-big-one.msf and replacing the existing 0 length "test-big-one". A folder repair and/or tb restart may be required to see the single email with subject "test again eml in eml" in the new local folder "test-big-one".

With "show attachments inline" you see the inner emails in the preview pane but if you click on either inner .eml attachment you see only a blank tab. If you then "move tab to a new window" the attachment appears in the new window.

With "don't show attachments inline" and after restart, an empty tab appears when the outer .eml attachment is clicked. (The inner inner .eml attachment doesn't appear in the list with attachments not inline which has always been the case.) When the tab is moved to a window, the attachments on that email don't show either.

Note: in both modes (show or don't show attachments inline) with earlier C-C snapshot based on changedset :76f62fd3a780 from 18-Jan the inner .eml attachments open properly into their own new window.
All the non-eml attachments (pdf etc.) display correctly in their tab or in external apps. So the problems is only with the .eml attachments.

Comment 22

[John Bieling (:TbSync)]

I am back and I started to review D170731 and D171304.

Comment 23

[Geoff Lankow (:darktrojan)]

The test looks like it might fail intermittently, but let's ship it and find out how often that happens.

Try run

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a4d34654d207
Fix APIs for display of external messages. r=john.bieling
https://hg.mozilla.org/comm-central/rev/35a645eaa042
Let messageDisplay API open external messages in tabs. r=john.bieling

Comment 25

[Geoff Lankow (:darktrojan)]

Comment on attachment 9319416 [details]
Bug 1814748 - Fix APIs for display of external messages. r=john.bieling

[Approval Request Comment]
Regression caused by (bug #): supernova
User impact if declined: broken extension API

Comment 26

[Wayne Mery (:wsmwk)]

Comment on attachment 9320427 [details]
Bug 1814748 - Let messageDisplay API open external messages in tabs. r=john.bieling

[Triage Comment]
Approved for beta

Comment 27

[Wayne Mery (:wsmwk)]

Comment on attachment 9319416 [details]
Bug 1814748 - Fix APIs for display of external messages. r=john.bieling

[Triage Comment]
Approved for beta

Comment 28

[gene smith]

Still seeing the problem described in comment 21. Noticed it while testing issues in bug 1819916.
The main issue is that if attached .eml is opened into a tab it displays blank. To see the attached .eml I have to move the tab to a new window.
Apparently this is only a problem for very large emails and emails containing other emails. I tried it again with a smaller email but with the same nested structure and it works fine (inner .eml opens directly into tab).

One thing I just noticed is that if I open the email again, this time it works. But if I move to another email and come back and try to open the .eml attachment into a tab, again it fails once and then works the 2nd time and it keeps working OK as long as I stay selected on the email.

I included a link above to a test email but you have to have access to an imap server that allows large messages (almost 100M in this case).

FWIW, I see these errors when I try to open the inner .eml's into a tab:

[Parent 2888436, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80550006: file /home/gene/mozilla2/comm/mailnews/base/src/nsMsgDBFolder.cpp:4744
[Parent 2888436, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80550006: file /home/gene/mozilla2/comm/mailnews/local/src/nsMailboxService.cpp:557

When it fails to open I see just the above two lines. But when it opens OK in a tab, I see a long series of those lines repeated.

Comment 29

[gene smith]

Looks like its not so much the structure of message as the total size. I just kept the structure like this:

Outer email
  text body
  Inner .eml attachment
    text body
    inner-inner .eml attachment
      text body
      png attachment (12MB)
    text attachment
  pdf attachment
  tar.gz attachment
  zip attachment
  ... etc.

With the initial size of about 92.5 MB the problem occurs (have to open inner or inner-inner .eml attachment twice to appear in tab).
When I reduce the outer attachments (pdf, tar.gz etc) down so the total message size is 68.3 MB, both .eml attachments open to a tab on first try.
If I add outer attachment so the total message reaches 74.7MB, the problem occurs. So the problem occurs somewhere between these two total sizes.