450649 - The link "click'here' to get the rest of truncated mail" is not working. (when truncated POP3 mail is moved/copied to other account's folder, including "Global Inbox" case)

Status: VERIFIED FIXED

(Thunderbird :: Mail Window Front End, defect, P1)

Description

[Henry N.]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.1.6) Gecko/20070730 SUSE/2.0.0.6-25 Firefox/2.0.0.6
Build Identifier: 2008073000, 20080813030431

The Link "Click 'here'" for getting the rest of truncated pop3 mail is not working for accounts with "Global inbox".


Reproducible: Always

Steps to Reproduce:
1.Have a pop3 account with Disk Space limit 50KB, enable "Global Inbox"
2.Get a mail that exceeded the limit, for example 150KB
4.Click the Link "click 'here' to get the rest of truncated mail"
Actual Results:  
Nothing.


Expected Results:  
Load the rest of mail.


This is a cleaner description from bug 450486.

The mail header has theses entries:
X-Account-Key: account1
X-UIDL: UID5412-1173099605

File popstate.dat in this account has entries like this:
*pop username
b UID5412-1173099605 1218700081

The non working Link from "Global inbox" in TB3:
mailbox://nobody@local%20folders/Inbox?number=1480191&messageid=48A3E309.4020405%40username.arcor.de&uidl=UID5412-1173099605

For accounts with non "Global Inbox", Link are working in TB3.
Here is such Link:
mailbox://[e-mail]/Inbox?number=194558&messageid=48A3E684.3000103%40username.arcor.de&uidl=UID56-1195863407

Same profile directory (same data) used with older Thunderbird Version 2.0.0.6 the Link from "Global inbox" is working as:
pop://username@pop:995/?uidl=UID5412-1173099605

Comment 1

[WADA:World Anti-bad-Duping Agency]

When a truncate mail in Inbox of non-Global-Inbox account is copied to other
account's Inbox, "internal path name of account" field of the link's URI was changed to one for account who owns mail folder. (Tested with tb trunk 2008/8/09 build)

>(In original account's Inbox)
>  mailbox://<user_id_1>@<servernme_1>/Inbox?number=nn&messageid=mm&uidl=uuuu...
>(Copied to Local Folders's Inbox)
>  mailbox://nobody@local%20folders/Inbox?number=nn&messageid=mm&uidl=uuuu...
>(Copied to other non-Global-Inbox account's Inbox)
>  mailbox://<user_id_2>@<servernme_2>/Inbox?number=nn&messageid=mm&uidl=uuuu...

Mail can be moved to other account's mail folder(Global Inbox is one of such cases). X-Account-Key: header should be used if exists, upon generation of link URI for additional download of truncated mail.

Confirmig.

Comment 2

[David :Bienvenu]

I think this should block since global inbox is the default

Comment 3

[Henry N.]

Muttley, the problem exist also, if pop3 account receives truncated mails directly into the global inbox.

Move/copy such mail is an additional effect for same problem.

Comment 4

[David :Bienvenu]

putting in b1 milestone, and blocking b1

Comment 5

[David Ascher (:davida)]

Switching for b1 flags to target milestones, to avoid flag churn.

Comment 6

[David :Bienvenu]

the file uri for the link url is wrong - it's just file:///Inbox, which isn't sufficient. The spec looks OK when you hover over the link, however.

Comment 7

[David :Bienvenu]

It's not the file uri that's the issue - that looks the same in the success case, and the failure case (the failure case being the global inbox case, which is the default). In the failure case, nsPop3Service::NewURI is getting an orphan folder back from the uri (no parent), and in the success case, it's getting a real folder. The URIs look OK in both cases, but it must not be quite right.

Comment 8

[David :Bienvenu]

the problem is that the partial download url is "local%20folders" and the folder uri is "Local%20Folders" so someone has lower-cased it...

Comment 9

[David :Bienvenu]

we're probably going through some uri conversion that ensures that host names are lower case. In this case, "Local Folders" is the host name, in our uri scheme.

Comment 10

[David :Bienvenu]

nsStandardURL::BuildNormalizedSpec is calling NormalizeIDN, which is lower-casing our host name.

We have a lot of uri's in prefs that have "mailbox://Local%20Folders..." so we can't just lower case the host name when creating the uri's for the local folder accounts, since uri's are case-sensitive.  Maybe we can use the original uri...

Comment 11

[David :Bienvenu]

this just gets more and more complicated - I think the file:///Inbox uri really is the issue, or at least part of it. In 2.0, where this works, the file uri points to the actual mailbox file. This allows us to scan the message, and figure out the account key, which tells us which pop3 server the message is actually on. It would be very helpful to get a regression range for this. A lot of this code changed when we got rid of nsFileSpec but I'm not at all sure that's the cause. When and how gecko creates mailbox urls for displayed messages seems to have changed quite a bit between 2.0 and 3.0 so it's difficult to compare the working code with the code that's failing, but the uri's we're getting from the doc shell are completely different; one is of the account form (nobody@local%20folders//Inbox), the one that works is the actual path to the mail folder on disk.

Comment 12

[David :Bienvenu]

In 2.0, where this works, mousing over the "get the rest" link correctly shows the url for the pop3 server the message was delivered to in the status bar.

Comment 13

[David :Bienvenu]

Attachment: possible fix

this fixes the problem - the change to nsPop3Service to get the msg hdr from the folder instead of from the message url is the thing that got me over the hump. I don't know for sure if the originalspec changes are required. 

I believe GetMessageHdr is only failing for the partial download url, not mailbox urls in general, though I need to verify that. If that's the case, then the change not to use it in the partial download case is OK.

Comment 14

[David :Bienvenu]

Attachment: proposed fix

turns out this is sufficient.

Comment 15

[[e-mail]]

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

>       nsCOMPtr<nsIMsgMessageUrl> msgUrl = do_QueryInterface(mailboxUrl, &rv);
>       NS_ENSURE_SUCCESS(rv,rv);
>-      msgUrl->GetMessageHeader(getter_AddRefs(msgHdr));
Nit: This was the only reference to msgUrl so you can remove all of this.

Comment 16

[Mark Banner (:standard8)]

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

r=me with the change that Neil commented on.

Comment 17

[David :Bienvenu]

fixed with Neil's nit addressed, changeset 247:8d26696691ae

Comment 19

[Henry N.]

Thanks, is working now in nightly build 20080904030144 (Linux/de).

Comment 20

[Joshua Cranmer [:jcranmer]]

Verified by comment 19.