The default bug view has changed. See this FAQ.

Local file as mail start page doesn't show up

VERIFIED FIXED in mozilla16

Status

()

Core
Internationalization
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Matthias Bockelkamp, Assigned: emk)

Tracking

Trunk
mozilla16
x86
Windows XP
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
In recent trunk builds, a local mail start page (file:///C:/Folder/page.htm) does not show up any more. Using a remote URL (http://www.web.de) works as expected.

Regression range is somewhere between 2012-04-22 and 2012-06-03.
(Reporter)

Comment 1

5 years ago
I tried to find the exact regression date:

2012-05-24-00-30-03-comm-central-trunk is ok
2012-05-25-00-30-03-comm-central-trunk is broken

Comment 2

5 years ago
http://hg.mozilla.org/comm-central/annotate/64605d5789cc/suite/mailnews/mailWindow.js#l490
Commenting out the following line in function loadStartPage() makes file urls work:

        messenger.setDisplayCharset("");

This line was added in Bug 59787 - Messenger Startup page in Mail viewing window displays incorrectly.

Thunderbird removed this line from their mailWindow.js in Bug 258447 - Mail Start Page ignores character setting. Do you think it's OK for us to do the same?
(Reporter)

Comment 3

5 years ago
Philip, please tell me how you found that checkin. 

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2012-05-23+23%3A00&enddate=2012-05-25+00%3A00
http://hg.mozilla.org/comm-central/pushloghtml?startdate=2012-05-23+23%3A00&enddate=2012-05-25+00%3A00

The checkin you named is not listed there.

Comment 4

5 years ago
A recent checkin just exposed a pre-existing bug. Thunderbird fixed their equivalent code in Bug 258447. I haven't identified the mozilla-central bug that changed the behaviour yet but it's probably one of the 118 changesets @
2552b0541f87	Ed Morley — Merge last PGO-green changeset of mozilla-inbound to mozilla-central.

Comment 5

5 years ago
Further investigation shows:
It fails if the local file doesn't contain a <meta ... charset>
If the file contains something like: <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />, then the local file works.
If you load a non working local file in the browser the following shows up in the error console:

Wed Jun 13 2012 19:12:01
Error: The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must to be declared in the document or in the transfer protocol.
Source file: file:///C:/..../TestCases/foobar.html

Comment 6

5 years ago
(In reply to Philip Chee from comment #2)
> Commenting out the following line in function loadStartPage() makes file
> urls work:
> 
>         messenger.setDisplayCharset("");
> 
> This line was added in Bug 59787 - Messenger Startup page in Mail viewing
> window displays incorrectly.
> 
> Thunderbird removed this line from their mailWindow.js in Bug 258447 - Mail
> Start Page ignores character setting. Do you think it's OK for us to do the
> same?

Previously SetDisplayCharset set the forced character set, which is why we had to unset it in order for the start page to detect its character set correctly. Bug 253807 changed this to set the hint character set, which resets itself on every page load, so doesn't need to be unset. I couldn't work out which code now relies on the character set not being empty; there is an assertion for unsupported character set strings but it fails to fire for the empty string.

Comment 7

5 years ago
So, the code that now fails when the character set is empty is nsHtml5StreamParser::SetupDecodingAndWriteSniffingBufferAndCurrentSegment, which tries to get the unicode decoder for the specified character set, but falls back to windows-1252 if the charset converter manager returns NS_ERROR_UCONV_NOCONV. Unfortunately due to bug 710693 it no longer does so for an empty character set, instead it now returns NS_ERROR_NULL_POINTER, which confuses the HTML5 parser into thinking that the document is broken.

Updated

5 years ago
Blocks: 710693
(Assignee)

Comment 8

5 years ago
Created attachment 632794 [details] [diff] [review]
Don't propagate an error code from GetCharsetAlias

Indeed XSS vulnerable encodings should never be detectable from callers of GetUnicodeDecoder.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #632794 - Flags: review?(smontagu)
Comment on attachment 632794 [details] [diff] [review]
Don't propagate an error code from GetCharsetAlias

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

::: intl/uconv/src/nsCharsetConverterManager.cpp
@@ +178,5 @@
>  
>    // fully qualify to possibly avoid vtable call
>    nsresult rv = nsCharsetConverterManager::GetCharsetAlias(aSrc, charset);
>    if (NS_FAILED(rv))
> +    return NS_ERROR_UCONV_NOCONV;

You don't really need rv at all any more, you could just do 
if (NS_FAILED( ... 
  return NS_ERROR_UCONV_NOCONV;

but r=me either way.
Attachment #632794 - Flags: review?(smontagu) → review+
(Assignee)

Comment 10

5 years ago
Created attachment 633087 [details] [diff] [review]
Don't propagate an error code from GetCharsetAlias. r=smontagu

patch for checkin
Attachment #632794 - Attachment is obsolete: true
Attachment #633087 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Component: MailNews: General → Internationalization
Product: SeaMonkey → Core
QA Contact: mail → i18n
https://hg.mozilla.org/integration/mozilla-inbound/rev/0de74d9fd081
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/0de74d9fd081
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

5 years ago
Verified with Build ID 20120615003002
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.