shouldn't try to fetch audio parts inline

RESOLVED FIXED

Status

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

People

(Reporter: Bienvenu, Assigned: Bienvenu)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

859 bytes, patch
neil@parkwaycc.co.uk
: review+
neil@parkwaycc.co.uk
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
Created attachment 462946 [details] [diff] [review]
proposed fix

When doing mime parts on demand, we should leave audio parts (e.g., mp3 files) on the imap server. There are other parts/types we should leave on the server, but application/* catches most of them, and audio catches an other source of big files.
Attachment #462946 - Flags: superreview?(neil)
Attachment #462946 - Flags: review?(neil)

Updated

7 years ago
Attachment #462946 - Flags: superreview?(neil)
Attachment #462946 - Flags: superreview+
Attachment #462946 - Flags: review?(neil)
Attachment #462946 - Flags: review+

Comment 1

7 years ago
Comment on attachment 462946 [details] [diff] [review]
proposed fix

>+    if (!PL_strcasecmp(m_bodyType, "AUDIO"))
>+      return PR_FALSE;
>     // Here's where we can add some more intelligence -- let's leave out
>     // any other parts that we know we can't display inline.

shouldn't the added intelligence be after this comment, not under the 'application' comment
(Assignee)

Comment 2

7 years ago
(In reply to comment #1)

> >+    if (!PL_strcasecmp(m_bodyType, "AUDIO"))
> >+      return PR_FALSE;
> >     // Here's where we can add some more intelligence -- let's leave out
> >     // any other parts that we know we can't display inline.
> 
> shouldn't the added intelligence be after this comment, not under the
> 'application' comment

No, I read the comment to mean "this is where you'd add other types of parts we can't display inline, in addition to what the code already handles."
(Assignee)

Comment 3

7 years ago
fixed on trunk. This won't affect users who have offline set on their folders, but I ran across this while testing some other stuff and it's a trivial fix.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.