110512 - Always displays a blank page as ISO-8859-1 on startup

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P1)

Description

[Hansoo Kim]

nsCOMPtr<nsIContentViewer> cv is always NULL in 
nsHTMLDocument::StartDocumentLoad()when a new browser window is open.

As a result, nsCOMPtr<nsIMarkupDocumentViewer> muCV is NULL, also.

It's bad b/c it prevents the new browser window from getting a right default 
characterset.

nsAutoString charset; charset.AssignWithConversion("ISO-8859-1"); // fallback 
value in case webShell return error

...

nsCOMPtr<nsIMarkupDocumentViewer> muCV;
nsCOMPtr<nsIContentViewer> cv;
docShell->GetContentViewer(getter_AddRefs(cv));
if (cv) {      =================================> First time loading this is 
0x00000000
   muCV = do_QueryInterface(cv);            ====> So as muCV  b/c of cv !!!
} else 

...

  if(kCharsetFromUserDefault > charsetSource) 
  {
    PRUnichar* defaultCharsetFromWebShell = NULL;
    if (muCV) {                                             ===> fot the first 
time, No execution of this !!
  		rv = muCV->GetDefaultCharacterSet(&defaultCharsetFromWebShell);
      if(NS_SUCCEEDED(rv)) {
        charset = defaultCharsetFromWebShell;
        Recycle(defaultCharsetFromWebShell);
        charsetSource = kCharsetFromUserDefault;
      }
    }
  }

...

// ======>  !!!  So charset is always defaults to ("ISO-8859-1")
  rv = this->SetDocumentCharacterSet(charset);

Comment 1

[Hansoo Kim]

This is necessary so that a new browser window gets a right default 
characterset with a fix made by Roy for 105113.

Roy, would you be able to test this case in mozilla.exe too, which
is not embedding case ?

Thanks.

/HK

Comment 2

[Roy Yokoyama]

Attachment: Read fallback charset from Pref

Comment 3

[Roy Yokoyama]

If we set a Pref in Moz, we can reproduce this problem.
Goto Pref dlgbox, set navigator to display a blank page when start up.
( "Blank page" Radio button under "When Navigator starts up, display" )

The blank page is always ISO-8889-1 at the startup.

Comment 4

[Roy Yokoyama]

ftang/jbetak: can you review the patch?

Comment 5

[[e-mail] (away - not reading bugmail)]

Comment on attachment 58158 [details] [diff] [review]
Read fallback charset from Pref

r=jbetak

Comment 6

[[e-mail] (away - not reading bugmail)]

Roy, 

the only thing that bugged me about this was the fact that we'll *always* spend 
a few cycles on this - whether we'll actually use the default or not. 

I looked into it and it seems like it might not be a good idea to change it 
now. I'd still mention the idea to you: perhaps the code could be changed to 
read the default at the end, only when it's needed. We would need to change 
error handling in a few places from "return(rv)" to something else to make sure 
we fetch the default.

I'm not sure how big a win this would be - it would need some profiling - but 
probably not big, so it's probably not worth to bother. Just an idea - perhaps 
there will be a better time.

Comment 7

[Roy Yokoyama]

Comment on attachment 58158 [details] [diff] [review]
Read fallback charset from Pref

/r=jbetak

Comment 8

[Roy Yokoyama]

jbetak: thank for your input and you are right about
the charset determination as a whole.  As you can see
in nsHTMLDocument::StartDocumentLoad() we have
number of calls just to find out which charset we use
for the document.
- default
- HTTP header
- HTML Meta Tag
- Forced/User Override
- Auto-Detector
- Cache
- Inherit from Parent
- Bookmark
I believe I saw a bug to improve the charset detection 
code (not Auto-detect).  I believe we should deal about this
in later time.

brendan: can your super review?

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 58158 [details] [diff] [review]
Read fallback charset from Pref

Roy, can you record the bug number for that followup/future-improvement work?

sr=[e-mail].

/be

Comment 10

[Teruko Kobayashi]

*** Bug 111217 has been marked as a duplicate of this bug. ***

Comment 11

[Roy Yokoyama]

Checked into the trunk. 
Bug 111336 is filed to investigate for charset detection.

teruko: please verify. thanks

---- However, leaving the bug open until we checkin to 0_9_4_BRANCH

Comment 12

[Roy Yokoyama]

ylong: can you verify this on the trunk?  Teruko is on vacation.

Comment 13

[Yuying Long]

Verified it on 11-26 trunk build(both windows and Mac):
In preferences, when change the start up new window from home page to blank
page, and change the default language charset to something other than
iso-8859-1, the charset in new blank page window is followed the changed default
charset.
While 11-19 trunk build is always set to iso-8859-1 no matter how you change the
default language setting.

So I can see the check-in is working on 11-26 trunk build.
 

Comment 14

[Michael Dunn]

Permission to check in to branch

Comment 15

[Roy Yokoyama]

checked into 0_9_4_BRANCH

Comment 16

[Teruko Kobayashi]

Verified as fixed in 2001-11-28-0.9.4ec Win32 build.

Comment 17

[Judson Valeski]

adding fixed0.9.4 keyword