1899337 - When a tab that opens a PDF attached to a mail in the maildir folder is restored, the tab will display a calendar panel etc. instead of the PDF if the mail could not be loaded due to an error

Status: UNCONFIRMED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[EarlgreyTea]

Attachment: calendar panel appears in PDF tab.mp4

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:115.0) Gecko/20100101 Thunderbird/115.11.0

Steps to reproduce:

1. Set up a POP account with message storage type "File per message (maildir)".
2. Receive a mail with a PDF attached.
3. Open the PDF attached to the mail in a tab.
4. Delete the mail.
5. Restart Thunderbird.

Actual results:

The title of the tab that was displaying the PDF became "Problem Loading Page" and the tab content became blank.
Actually, "calendarTabPanel" is displayed, and once I displayed the calendar, the same calendar was displayed on the tab that was displaying the PDF.

Expected results:

The tab should display an error message.

Addtional Information:

If an error occurs while restoring six tabs of a PDF, each tab will display the following.

• 1st tab: "mail3PaneTab1"
• 2nd tab: "calendarTabPanel"
• 3rd tab: "calControlBarMenuPopup"
• 4th tab: "calendarItemPanel"
• 5th tab: "mail3PaneTab1"
• 6th tab: "contentTabWrapper0"
• 7th tab: "contentTabWrapper0"

"contentTabWrapper0" displays an error message.
The same problem occurs with Daily, and a similar situation occurs with Ver.91.
This bug was detected in bug 1858157.

Comment 1

[EarlgreyTea]

Attachment: console-export-2024-5-29_0-5-32.txt

Console log when error occurs.

Comment 2

[EarlgreyTea]

Add the following to expected results:
Tabs where no errors occurred should be restored.

When trying to restore a PDF tab with several UIs tabbed open after the PDF tab, the contents of the "Account Settings" tab were replaced with an error page.

tab	title(before)	id(before)	title(after)	id(after)
1	pop (maildir)	mail3PaneTab1	pop (maildir)	mail3PaneTab1
2	A.pdf	contentTabWrapper1	Problem loading page	calendarTabPanel
3	B.pdf	contentTabWrapper2	Problem loading page	calControlBarMenuPopup
4	C.pdf	contentTabWrapper3	Problem loading page	calendarItemPanel
5	D.pdf	contentTabWrapper4	Problem loading page	mail3PaneTab1
6	E.pdf	contentTabWrapper5	Problem loading page	contentTabWrapper0
7	F.pdf	contentTabWrapper6	Problem loading page	contentTabWrapper0
8	Address Book	addressBookTabWrapper0	Address Book	addressBookTabWrapper0
9	Calendar	calendarTabPanel	Calendar	calendarTabPanel
10	Tasks	calendarTabPanel	Tasks	calendarTabPanel
11	Chat	chatTabPanel	Chat	chatTabPanel
12	Settings	preferencesTabWrapper0	Settings	preferencesTabWrapper0
13	Account Settings	contentTabWrapper0	Account Settings	contentTabWrapper0
14	Add-ons Manager	contentTabWrapper7	Add-ons Manager	contentTabWrapper1
15	Saved Files	contentTabWrapper8	Saved Files	contentTabWrapper2
16	Troubleshooting Infomation	contentTabWrapper9	Troubleshooting Infomation	contentTabWrapper3

Comment 3

[EarlgreyTea]

Attachment: account. settings tab displays error.jpg

Comment 4

[gene smith]

Attachment: mailboxProtocol-fix--mbox-only.diff

Note: this comment and the attachment were intended for bug 1858157, not this bug.
This partially fixes the bug, but ONLY for mbox. When PDFs are in tabs, on restart the whole mbox file was getting read and the PDF "part" was getting accessed at the wrong file offset and contained the wrong data. Since the whole mbox files was read, the "mime part", e.g., part=1.2, was getting extracted incorrectly at the start of the file instead in in the message farther down the file (at the proper offset into the mbox file) where the PDF "part" resides. This caused blank or incorrect tabs containing PDFs to appear at startup.

This change now just obtains and uses the "offset" into the mbox file to access the PDF data when there is no "folder" available. The offset and length are used in the call to OpenFileSocket.

The normal method used during runtime when a message or PDF is opened into a tab is to call CreateInputTransport but that doesn't seem to have a way to have an offset specified to begin reading the file.

OpenFileSocket, according to the comment, seems to be intended only for reading in .eml files (I assume via the -file commandline option). When an .eml is opened, there is no header pointer so in that case I use the default parameters (aURL, 0, -1) and don't pass in a explicit message offset and message length.

But with this fix, PDF tab restores on startup for maildir still come in blank (and there are other issue that EarlGT has pointed out with maildir). And I really have no idea how to fix this.

Ben, do you have any ideas on this or how to fix it so it will work for mbox AND maildir? I know that messages in maildir don't have a true "offset" since each message is in its own file. I know there is a "storetoken" items that is the offset for mbox and somehow points to the proper maildir file, but I don't know how it works (or if it's relevant to this bug).

Comment 5

[Ben Campbell]

(In reply to gene smith from comment #4)
[snip]

OpenFileSocket, according to the comment, seems to be intended only for reading in .eml files (I assume via the -file commandline option). When an .eml is opened, there is no header pointer so in that case I use the default parameters (aURL, 0, -1) and don't pass in a explicit message offset and message length.

As a slightly tangential issue:
This is the only place OpenFileSocket() is called. I'd be inclined to either inline it, or at least move it from nsMsgProtocol into nsMailboxProtocol. Also, it shouldn't use the slicedstream at all, and doesn't need offset or length params - it should just stream the whole file, which is the case it's used for for .eml files.

Ben, do you have any ideas on this or how to fix it so it will work for mbox AND maildir? I know that messages in maildir don't have a true "offset" since each message is in its own file. I know there is a "storetoken" items that is the offset for mbox and somehow points to the proper maildir file, but I don't know how it works (or if it's relevant to this bug).

General local-message-access technique:

The correct way to access raw message stream is shown here:
https://searchfox.org/comm-central/source/mailnews/local/src/nsMailboxProtocol.cpp#92

            nsCOMPtr<nsIInputStream> stream;
            rv = folder->GetLocalMsgStream(msgHdr, getter_AddRefs(stream));

GetLocalMsgStream() basically just gets the 'storeToken' from the msghdr and passes it to nsIMsgPluggableStore.getMsgInputStream(storeToken) to open the message for reading. It's a little more complicated than that because GetLocalMsgStream() for IMAP folders has that gmail hack to support messages being in other folders. Sigh. But one day it'll be nice and simple!
This is the only way to safely access local messages.
The stream contains just the message you asked for - from the callers point of view there's no notion of it being part of a mbox file.
The stream returned for the mbox store handles unescaping "From " lines in the body of the message, so if you're just reading from the raw mbox file you may just get different data back!
Probably unlikely to happen in a pdf attachment, but stranger things have happened, and it'd be a helluva bug to track down!

(see also Bug 1720047, in which I state my intention to kill GetMessageOffset() :-)

Note also that any additional message store implementations might not even use raw files directly (eg an sqlite-backed store is a popular candidate).

...Anyway....

That doesn't help us here, because we don't appear to have a folder available - the database doesn't have the folder set during startup!
Your patch extends the original code to use OpenFileSocket when the folder is not known. But that should never be the case - a msgHdr from the database should always have a folder.

So this is the real problem: Why is the database not connected to a folder and how can we fix that?
I suspect this is going to require a deep dive into the horrors of folder creation...

I'm going to flip it back to you, but please do feel free to just NI it back to me and I'll look into it!

Comment 6

[EarlgreyTea]

Thanks to gene and Ben for starting to work on resolving the bug.
But that solution seems to be for bug 1858157. I think the misunderstanding occurred because of my poor English.
I would like to clarify the points of the two bugs.

bug 1858157:

• Thunderbird tries to load data from the wrong position when restoring tabs that have PDFs open.
• This results in the following.
  • For mbox folder, incorrect identical or blank tabs are displayed.
  • For maildir folder, the tab doesn't display the PDF because of the "NS_ERROR_FILE_ACCESS_DENIED" error.
  • In either case the number of tabs remains the same.

bug 1899337:

• If an error occurs when reloading or restoring a tab with a PDF open in a maildir folder, the tab doesn't display an error message.
• The details will vary depending on the number of tabs, their order, etc., but the results will generally be as follows.
  • A pattern where "calendarTabPanel" or "calControlBarMenuPopup" or other items are displayed in the tab where the PDF was displayed.
  • A pattern where an error message is displayed on the tab that was originally displayed using "contentTabWrapper0". (Account Settings)
  • If there are many tabs, the later tabs will use "contentTabWrapper0" and an error message will be displayed.
• Please note that in the reproduction steps I deliberately cause an error by deleting the mails.

Comment 7

[gene smith]

(In reply to EarlgreyTea from comment #6)

Thanks to gene and Ben for starting to work on resolving the bug.
But that solution seems to be for bug 1858157. I think the misunderstanding occurred because of my poor English.

Your English is great! I attached the .diff to the wrong bug. Sorry, I meant to attach it to bug 1858157.