293392 - E4X Support does not work in <xul:script>

Status: VERIFIED FIXED

(Core :: XUL, defect, P1)

Description

[Peter Wilson]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050507 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050507 Firefox/1.0+

Access to the E4X Javascript extensions does not seem to work for chrome url's. 
In XUL source:
<script type="application/x-javascript; e4x=1" src="XXX.js"/>

In XXX.js:
var elem = <AnElement></AnElement>;
gives a syntax error.

This works fine for an html url.

Also What is E4X status of JS loaded using mozIJSSubSriptLoader or 
E4X code which is eval'd from script which is not E4X blessed?

Reproducible: Always

Comment 1

[Boris Zbarsky [:bzbarsky]]

Does this work if you use an <html:script> (in chrome) instead of a <xul:script>?

Comment 2

[Peter Wilson]

Using the code from the IBM example. I coded the following into global level of
js script:
var x = <people>
   <person gender="male">
      <name>Ant</name>
      <hair>Shaggy</hair>
      <eyes>Blue</eyes>
      <height measure="metric">176</height>
   </person>
   <person gender="male">
      <name>Paul</name>
      <hair>Spiky</hair>
      <eyes>Grey</eyes>
      <height measure="metric">178</height>
   </person>
</people>;

alert(x.person[0].eyes);
blue

alert(x.person[1].hair);
spikey

html:script seems to work. I still get the syntax error in the JS Console - but
the element is created and contents are displayed using alert() - the program
continues to load and run correctly. Switching back to xul:script I get the same
syntax error - but that error stops the form from running. 

(Displaying the alert/dialog box is currently horked in latest Deer Park
nightly. I assume those problems are not yours.)

Error: syntax error
Source File: chrome://xulapp/content/xulapp.js
Line: 45, Column: 8
Source Code:
var x = <people>

Comment 3

[Boris Zbarsky [:bzbarsky]]

So the issue is that xul:script doesn't use nsScriptLoader, which is where the
";e4x=1" thing is implemented.  Instead it does various XUL prototype stuff... 
Sounds like we need to teach all that code about e4x or make it use nsScriptLoader.

Comment 4

[Tennessee Leeuwenburg]

This bug also applies to inline scripts. Here is my minimal example:

<?xml version="1.0"?>
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>

<window
    id="findfile-window"
    title="Bureau of Meteorology Data Viewer"
    orient="horizontal"

xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
    align="start"


    <vbox style="margin: 45px;">
                    <button id="xmlbutton" label="Play with XML"
accesskey="x" width="200" flex="0" />
    </vbox>

    <script>

        var btnXML = document.getElementById("xmlbutton");
        btnXML.addEventListener('command', playXML, true);

        function playXML() {
            var x =
                <people>
                   <person gender="male">
                      <name>Ant</name>
                      <hair>Shaggy</hair>
                      <eyes>Blue</eyes>
                      <height measure="metric">176</height>
                   </person>
                   <person gender="male">
                      <name>Paul</name>
                      <hair>Spiky</hair>
                      <eyes>Grey</eyes>
                      <height measure="metric">178</height>
                   </person>
                </people>;

            alert(x);

        }

    </script>

</window> 

Comment 5

[Brendan Eich [:brendan]]

Attachment: fixed version of testcase from comment 4

Comment 6

[Brendan Eich [:brendan]]

Attachment: proposed fix

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 187906 [details] [diff] [review]
proposed fix

sr=jst

Comment 8

[Peter Van der Beken [:peterv]]

Comment on attachment 187906 [details] [diff] [review]
proposed fix

>Index: content/xul/content/src/nsXULElement.cpp
>===================================================================

>+
>+    // XXXbe violate nsIScriptContext layering because its version parameter
>+    // is mis-typed as const char * -- if it were uint32, we could more easily
>+    // extend version to include compile-time option, as the JS engine does.
>+    // It'd also be more efficient than converting to and from a C string.

Is there a bug filed about this?

Comment 9

[Brendan Eich [:brendan]]

(In reply to comment #8)
> (From update of attachment 187906 [details] [diff] [review] [edit])
> >Index: content/xul/content/src/nsXULElement.cpp
> >===================================================================
> 
> >+
> >+    // XXXbe violate nsIScriptContext layering because its version parameter
> >+    // is mis-typed as const char * -- if it were uint32, we could more easily
> >+    // extend version to include compile-time option, as the JS engine does.
> >+    // It'd also be more efficient than converting to and from a C string.
> 
> Is there a bug filed about this?

Not specifically, because lots of other places violate layering, and the
nsIScriptContext interface itself implicitly uses JS-only types such as jsvals.
 See bug 255942.

/be

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 187906 [details] [diff] [review]
proposed fix

Approving for 1.8b3.

/be

Comment 11

[Brendan Eich [:brendan]]

Fixed.  Note that E4X is enabled by default both in chrome windows, and in XUL
script tags -- the latter is necessary for remote XUL parity.  To disable, clear
the e4x MIME type parameter:

<script type="application/javascript; e4x=0">...</script>

/be

Comment 12

[Ed Lee :Mardak]

Attachment: testcase

It seems like this bug isn't completely fixed because it only checks the type
that is explicitly provided in the type (application/javascript; e4x=1). If
that is omitted, but the content-type says it's e4x enabled, it still doesn't
work.

The testcase loads two scripts. The first is a javascript e4x=1 file that
should alert 'Hello'. The second is the same but alerts 'World' and the script
has an attribute explicitly saying it's e4x.

Comment 13

[Boris Zbarsky [:bzbarsky]]

What does that testcase have to do with XUL?  It's an HTML testcase...  Sounds
like you want to file a separate bug on that issue.

Comment 14

[Bob Clary [:bc] (inactive)]

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

Comment 15

[Brendan Eich [:brendan]]

(In reply to comment #12)
> The testcase loads two scripts. The first is a javascript e4x=1 file that
> should alert 'Hello'. The second is the same but alerts 'World' and the script
> has an attribute explicitly saying it's e4x.

This worksforme in 1.5.0.6.

/be