209220 - Gecko & friends should store/access charsets as ASCII

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Alec Flett]

now that bug 206379 is fixed, i18n APIs take ASCII charset names directly. This
means that aside from frozen DOM interfaces, we have no reason to have unicode
charset names... 

So I started by tweaking nsIDocument, which has Get/SetDocumentCharacterSet()
and changed them to take nsACString, and then watched the effects ripple out
from there. I ended up changing a ton of interfaces all over the parser, layout,
docshell, intl, etc.. but the effects were fantastic - huge reductions in calls
to NS_LossyConvertUCS2toASCII, elimination of temporary variables,
AssignWithConversion, you name it...

In general, I switched most interfaces over to nsACString, but a few of them
stayed as |const char*| for simplicity of changes... 

Patch forthcoming... it touches over 70 files, but may of the changes are just
+1/-1 so the review should be pretty quick.

Comment 1

[Alec Flett]

Attachment: use ASCII charset names

and here's the patch..

I'd like an r=someone good (jkeiser, bsmedberg, jshin, are all good candidates)
and sr=jst

the patch looks big but it is FAR simpler than it appears - much easier to
review than bug 206379

Comment 2

[Alec Flett]

Comment on attachment 125526 [details] [diff] [review]
use ASCII charset names

requesting r=jkeiser, sr=jst but as mentioned above, I'll take an r= from other
folks as well... this is really straight forward stuff.

Comment 3

[John Keiser (jkeiser)]

Comment on attachment 125526 [details] [diff] [review]
use ASCII charset names

This is sweet, and totally worth the few places where you have to convert back
to Unicode.  Only one thing:

Index: dom/src/base/nsLocation.cpp
@@ -97,12 +97,8 @@ static +  nsCAutoString charset;
+  rv = doc->GetDocumentCharacterSet(aCharset);
   return rv;
 }

The declaration of charset is no longer necessary.

Comment 4

[Alec Flett]

(good catch. I've removed it in my tree)

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 125526 [details] [diff] [review]
use ASCII charset names

Nice, sr=jst!

Comment 6

[Jungshik Shin]

Wow, great. 
These three lines caught my eyes. Perhaps, a result of global replacement :-)

> Index: layout/html/base/src/nsImageFrame.h
> @@ -2746,11 +2746,11 @@ NS_IMETHODIMP nsPluginInstanceOwner::Get
> +      !nsCRT::strncmp(charset.get(), NS_LITERAL_CSTRING("UTF").get(), 3)) {

> Index: mailnews/base/src/nsMessenger.cpp

> @@ -555,7 +555,7 @@ nsMessenger::OpenURL(const char *aURL)
> +  SetDisplayCharset(NS_LITERAL_CSTRING("UTF-8").get());
> @@ -594,7 +594,7 @@ nsMessenger::LoadURL(nsIDOMWindowInterna
> +  SetDisplayCharset(NS_LITERAL_CSTRING("UTF-8").get());

Comment 7

[Alec Flett]

good catch! I'll have those lines as raw strings when I go to check in.

Comment 8

[Alec Flett]

fix is in! thanks everyone