Reimplement UTF-7 in mailnews

RESOLVED FIXED in Thunderbird 3.3a2

Status

MailNews Core
Internationalization
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: smontagu, Assigned: smontagu)

Tracking

Trunk
Thunderbird 3.3a2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

7 years ago
Bug 414064 will remove UTF-7 from intl/uconv. We need to reimplement UTF-7 and x-imap4-modified-utf7 in mailnews.
(Assignee)

Comment 1

7 years ago
Created attachment 466142 [details] [diff] [review]
Patch

This builds locally for me and passes xpcshell-tests, but for some reason it is failing on all tryserver builds:
http://tinderbox.mozilla.org/showlog.cgi?log=ThunderbirdTry/1281879755.1281885688.24828.gz
http://tinderbox.mozilla.org/showlog.cgi?log=ThunderbirdTry/1281879750.1281882815.13518.gz
http://tinderbox.mozilla.org/showlog.cgi?log=ThunderbirdTry/1281879757.1281883810.17938.gz
http://tinderbox.mozilla.org/showlog.cgi?log=ThunderbirdTry/1281879758.1281885781.25035.gz
Attachment #466142 - Flags: feedback?(bugzilla)
(In reply to comment #1)
> Created attachment 466142 [details] [diff] [review]
> Patch
> 
> This builds locally for me and passes xpcshell-tests, but for some reason it is
> failing on all tryserver builds:

According to the try graph [1], you pushed from an older version of comm-central which is busted with respect to current mozilla-central.

If you update to latest comm-central and push again, then it should work fine.

[1] http://hg.mozilla.org/try-comm-central/graph
(Assignee)

Comment 3

7 years ago
Created attachment 466599 [details] [diff] [review]
Corrected patch

Fixed a bug in message display.
Attachment #466599 - Flags: review?(bugzilla)
(Assignee)

Updated

7 years ago
Attachment #466142 - Flags: feedback?(bugzilla)
(Assignee)

Comment 4

7 years ago
(In reply to comment #2)
> According to the try graph [1], you pushed from an older version of
> comm-central which is busted with respect to current mozilla-central.
> 
> If you update to latest comm-central and push again, then it should work fine.

Thanks, Mark, that worked like a dream.
(Assignee)

Comment 5

7 years ago
Unfortunately View Source of a UTF-7 message is broken with this approach. I
haven't tested viewing an .eml file in browser, but I expect it will be broken
also.
(Assignee)

Comment 6

7 years ago
We can do this a lot more simply by adopting the mechanism from bug 601429 for marking the charset as "XSSVulnerable", rather than removing the UTF-7 decoder altogether from code code, but we need to make a decision first about what level of utf-7 support we want to retain in mailnews.

If we only want to support IMAP Mod utf-7, life will be very simple, and we will need only a one-line patch in mailnews.

Supporting messages encoded in utf-7 is harder. I have been working on it, and I have solutions of various degrees of hackery for most of the issues, but I'm not sure if it's worth the investment.
(Assignee)

Comment 7

7 years ago
Created attachment 491532 [details] [diff] [review]
Use GetDecoderInternal for IMAP

OK, 3 one-line patches ;-)

This passes all the IMAP xpcshell tests with attachment 491486 [details] [diff] [review] from bug 414064 applied.
Attachment #466142 - Attachment is obsolete: true
Attachment #466599 - Attachment is obsolete: true
Attachment #491532 - Flags: review?(bugzilla)
Attachment #466599 - Flags: review?(bugzilla)
Comment on attachment 491532 [details] [diff] [review]
Use GetDecoderInternal for IMAP

This looks good. Sorry for the review delay.

r=Standard8
Attachment #491532 - Flags: review?(bugzilla) → review+
(Assignee)

Comment 9

7 years ago
Thanks for review, Mark. What is your opinion about whether we need to support UTF-7 encoded messages?
I don't think we have enough information to make a call either way at the moment.

I'm also not sure how we'd get that information.
(Assignee)

Updated

7 years ago
Attachment #491532 - Flags: superreview?(neil)
Comment on attachment 491532 [details] [diff] [review]
Use GetDecoderInternal for IMAP

There's no sr needed on the patch as you're not changing apis... https://developer.mozilla.org/en/Mailnews_and_Mail_code_review_requirements

(unless you really want to).
Attachment #491532 - Flags: superreview?(neil)
Plus most of the patch is test fixes ;-)
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Status: NEW → ASSIGNED
Checked in: http://hg.mozilla.org/comm-central/rev/4902ddd8ba76
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a2
Depends on: 677111

Comment 14

6 years ago
Simon, I think we're interested in supporting utf-7 encoded messages - see bug 677111 - what do you think this is going to entail?

Comment 15

6 years ago
(In reply to Simon Montagu from comment #6)

> Supporting messages encoded in utf-7 is harder. I have been working on it,
> and I have solutions of various degrees of hackery for most of the issues,
> but I'm not sure if it's worth the investment.

Simon, any more info about this? From bug 677111, it seems that we do want to support utf-7 encoded messages.
(Assignee)

Comment 16

6 years ago
Hmm, I'll dig out my old patches and remind myself what I was talking about in that comment.
You need to log in before you can comment on or make changes to this bug.