587475 - Reimplement UTF-7 in mailnews

Status: RESOLVED FIXED

(MailNews Core :: Internationalization, defect)

Description

[Simon Montagu :smontagu]

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

Comment 1

[Simon Montagu :smontagu]

Attachment: 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

Comment 2

[Mark Banner (:standard8)]

(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

Comment 3

[Simon Montagu :smontagu]

Attachment: Corrected patch

Fixed a bug in message display.

Comment 4

[Simon Montagu :smontagu]

(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.

Comment 5

[Simon Montagu :smontagu]

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.

Comment 6

[Simon Montagu :smontagu]

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.

Comment 7

[Simon Montagu :smontagu]

Attachment: 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.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 491532 [details] [diff] [review]
Use GetDecoderInternal for IMAP

This looks good. Sorry for the review delay.

r=Standard8

Comment 9

[Simon Montagu :smontagu]

Thanks for review, Mark. What is your opinion about whether we need to support UTF-7 encoded messages?

Comment 10

[Mark Banner (:standard8)]

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.

Comment 11

[Mark Banner (:standard8)]

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).

Comment 12

[[e-mail]]

Plus most of the patch is test fixes ;-)

Comment 13

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/4902ddd8ba76

Comment 14

[David :Bienvenu]

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

[David :Bienvenu]

(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.

Comment 16

[Simon Montagu :smontagu]

Hmm, I'll dig out my old patches and remind myself what I was talking about in that comment.