241739 - nsXULDocument seems unable to deal with non-ascii scripts

Status: RESOLVED FIXED

(Core :: XUL, defect, P1)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

in the OnStreamComplete impl:
3133         nsString stringStr; stringStr.AssignWithConversion(string, stringLen);

it seems somewhat unlikely that this will do the intended thing for non-ascii
(or, non-latin1) scripts.

Comment 1

[Jungshik Shin]

http://lxr.mozilla.org/seamonkey/source/content/base/src/nsScriptLoader.cpp#735

Can we do something similar to what's done in nsScriptLoader.cpp? 

Comment 2

[Boris Zbarsky [:bzbarsky]]

That's exactly what we want to do, in fact...  It just sucks to duplicate code.
  Any way to hook this crud up to the normal scriptloader?

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

nsIScriptLoader:
 89   void processScriptElement(in nsIDOMHTMLScriptElement aElement,
 90                             in nsIScriptLoaderObserver aObserver);

I suspect the type of aElement would have to change, at least.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

it could possibly use
http://lxr.mozilla.org/seamonkey/source/content/html/content/public/nsIScriptElement.h
although it may need a way to query the URL

Comment 5

[Boris Zbarsky [:bzbarsky]]

See bug 235826.  That changes the nsIScriptLoader api already.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

unfortunatly i suspect it's not as easy in XUL due to the
implement-all-elements-using-a-single-class design :(

Comment 7

[Boris Zbarsky [:bzbarsky]]

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

Comment 8

[Boris Zbarsky [:bzbarsky]]

The other option is to move a lot of the scritloader OnStreamComplete guts (at
least the part handling the encoding conversion) out into a static helper or an
nsContentUtils helper and have nsXULDocument reuse it...

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

crazy idea: could we maybe just create an HTMLScriptElement for <xul:script/> nodes?

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

You probably wouldn't be able to take advantage of the prototype stuff for
script elements then. IIRC for scripts we put the actual script in the prototype
to avoid recompiles.

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: Testcase

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: implements comment 8's suggestion

(this patch leaves the method on nsScriptLoader, instead of putting it onto
nsContentUtils)

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 185903 [details] [diff] [review]
implements comment 8's suggestion

>Index: base/src/nsScriptLoader.cpp
>+nsScriptLoader::ConvertToUTF16(nsIChannel* aChannel, const PRUint8* aData,

>+  if (NS_FAILED(rv) || characterSet.IsEmpty()) {
>+    if (!aHintCharset.IsEmpty()) {

Why not use && instead of a separate if?

>+      nsString tempStr;

Why bother?  Why not just write to aString directly?

r+sr=bzbarsky with those nits picked.

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

comments addressed, transferring bz's r+sr

This patch allows non-ASCII characters in XUL <script src> scripts, would be
nice to have in 1.8 - it just reuses the code for HTML/SVG <script>.

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in content/base/src/nsScriptLoader.cpp;
/cvsroot/mozilla/content/base/src/nsScriptLoader.cpp,v  <--  nsScriptLoader.cpp
new revision: 1.72; previous revision: 1.71
done
Checking in content/base/src/nsScriptLoader.h;
/cvsroot/mozilla/content/base/src/nsScriptLoader.h,v  <--  nsScriptLoader.h
new revision: 1.10; previous revision: 1.9
done
Checking in content/xul/document/src/nsXULDocument.cpp;
/cvsroot/mozilla/content/xul/document/src/nsXULDocument.cpp,v  <-- 
nsXULDocument.cpp
new revision: 1.659; previous revision: 1.658
done

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

documented at
http://developer.mozilla.org/en/docs/International_characters_in_XUL_JavaScript