The default bug view has changed. See this FAQ.

nsXULDocument seems unable to deal with non-ascii scripts

RESOLVED FIXED in mozilla1.8beta3

Status

()

Core
XUL
P1
normal
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: Biesinger, Assigned: Biesinger)

Tracking

({intl})

Trunk
mozilla1.8beta3
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

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.
Keywords: helpwanted

Comment 1

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

Can we do something similar to what's done in nsScriptLoader.cpp? 
Keywords: intl
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?
nsIScriptLoader:
 89   void processScriptElement(in nsIDOMHTMLScriptElement aElement,
 90                             in nsIScriptLoaderObserver aObserver);

I suspect the type of aElement would have to change, at least.
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
See bug 235826.  That changes the nsIScriptLoader api already.
unfortunatly i suspect it's not as easy in XUL due to the
implement-all-elements-using-a-single-class design :(
*** Bug 264014 has been marked as a duplicate of this bug. ***
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...
Assignee: nobody → cbiesinger
crazy idea: could we maybe just create an HTMLScriptElement for <xul:script/> nodes?
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.
Created attachment 185895 [details]
Testcase
Created attachment 185903 [details] [diff] [review]
implements comment 8's suggestion

(this patch leaves the method on nsScriptLoader, instead of putting it onto
nsContentUtils)
Attachment #185903 - Flags: review?(bzbarsky)
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta3
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.
Attachment #185903 - Flags: superreview+
Attachment #185903 - Flags: review?(bzbarsky)
Attachment #185903 - Flags: review+
Created attachment 185953 [details] [diff] [review]
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>.
Attachment #185903 - Attachment is obsolete: true
Attachment #185953 - Flags: superreview+
Attachment #185953 - Flags: review+
Attachment #185953 - Flags: approval1.8b3?

Updated

12 years ago
Attachment #185953 - Flags: approval1.8b3? → approval1.8b3+
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
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Keywords: helpwanted
documented at
http://developer.mozilla.org/en/docs/International_characters_in_XUL_JavaScript

Updated

9 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.