Caching parts doesn't work on reply when query string is ?header=quotebody&part=1.2

RESOLVED FIXED in Thunderbird 52.0

Status

MailNews Core
Networking: IMAP
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: Jorg K (GMT+2), Assigned: Jorg K (GMT+2))

Tracking

({regression})

Trunk
Thunderbird 52.0
regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
The bug is pretty clear. We try to extract ?part= but that isn't there.
Instead there is: ?header=quotebody&part=1.2
(Assignee)

Comment 1

7 months ago
Created attachment 8801546 [details] [diff] [review]
1310446.patch

Sorry, a little regression from our part caching bug.

If you want to test that, create yourself a message with an embedded image in an IMAP folder without local storage.

Reply to that, you will see a broken image in the reply. Inspect the HTML in the reply. I use ThunderHTMLedit for that which I have installed permanently. You see:

<img src="imap://... ?header=quotebody&amp;part=1.2&amp;filename=....">.

The renderer requests the part, but we don't notice that a part is requested since there is no ?part= in the request. Consequently we don't set up part extraction and hand back the entire message.

Small fix to also detect the &part=. I need to change this to ?part= since a ? in the cache key signifies a cached part.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8801546 - Flags: review?(rkent)

Comment 2

7 months ago
Comment on attachment 8801546 [details] [diff] [review]
1310446.patch

Review of attachment 8801546 [details] [diff] [review]:
-----------------------------------------------------------------

This bug is another good example why we should not be doing things like trying parse URLs ourselves with string methods, or using something like ?part= to represent a query flag (as it combines an attribute like "part" with syntax markers like ? and =). I have not been able to find decent, supported URL query parsing code though in Gecko.

So let's do this, but do try in the future to move our code toward use of standard parsers.
Attachment #8801546 - Flags: review?(rkent) → review+
(Assignee)

Updated

7 months ago
Keywords: checkin-needed

Comment 3

7 months ago
https://hg.mozilla.org/comm-central/rev/2bf4d3a7b660b4014c2b21258c502a4fcf86eb52
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0
You need to log in before you can comment on or make changes to this bug.