407323 - Javascript errors on http://rishida.net/scripts/uniview/conversion

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Simon Montagu :smontagu]

Steps to reproduce:

Go to http://rishida.net/scripts/uniview/conversion
Enter something in one of the textboxes
Click "Convert"

Expected results: the input is converted and displayed in the other textboxes.
Actual results: nothing happens.

An error appears in the error console, e.g. for the "Characters" textbox:

Error: chars is undefined
Source File: http://rishida.net/scripts/uniview/conversion
Line: 1

This is a regression from the last day or two.

Comment 1

[Igor Bukanov]

The page contains:

var XML = document.getElementById('XML');

This no longer works as XML is defined as const since the patch from the bug 376957 landed. I guess we should remove JSCLASS_FIXED_BINDING from Namespace, QName and XML classes as pretty much nobody is aware/cares about E4X.

Comment 2

[Brendan Eich [:brendan]]

Agreed, sigh. Jeff, what do you say?

/be

Comment 3

[Brendan Eich [:brendan]]

A better idea: make the E4X "type" bindings fixed iff JS_HAS_XML_OPTION(cx). This requires runtime logic beyong the JSCLASS_FIXED_BINDING flag. Jeff, can you take this bug, or are you in finals hell?

/be

Comment 4

[Jeff Walden [:Waldo]]

(In reply to comment #3)
> A better idea: make the E4X "type" bindings fixed iff JS_HAS_XML_OPTION(cx).
> This requires runtime logic beyong the JSCLASS_FIXED_BINDING flag.

Rebinding XML doesn't do anything interesting -- unlike []/{} nothing uses the XML binding.  To me this makes it much less interesting as something to lock down to make the language more predictable.  I wouldn't bother with pain for something that basically isn't used all that much, and I'd just remove the JSCLASS_FIXED_BINDING for each.

> Jeff, can you take this bug, or are you in finals hell?

Probably best if I don't, to be honest.

Comment 5

[Brendan Eich [:brendan]]

Attachment: fix as proposed, except Namespace is still immutably bound

Namespace is coming in ES4/JS2, and I would like to put a stake in the ground.

/be

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 292192 [details] [diff] [review]
fix as proposed, except Namespace is still immutably bound

Based on quick regression report, asking for b2 approval.

/be

Comment 7

[Mike Schroepfer]

Comment on attachment 292192 [details] [diff] [review]
fix as proposed, except Namespace is still immutably bound

K - go for it - note monday is the drop-dead for changes so if this causes regressions we'll need to act quick.

Comment 8

[Mike Schroepfer]

also once you land can you clobber so we get a new nightly update?  I'm pretty sure this is the last change in.

Comment 9

[Brendan Eich [:brendan]]

Fixed:

js/src/jsxml.c 3.181

Reed is kindly handling the clobber.

/be

Comment 10

[Bob Clary [:bc] (inactive)]

Checking in e4x/Regress/regress-407323.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-407323.js,v  <--  regress-407323.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/js/tests/js1_5/Regress/regress-407323.js,v
done
Checking in js1_5/Regress/regress-407323.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-407323.js,v  <--  regress-407323.js
initial revision: 1.1
done

Comment 11

[Bob Clary [:bc] (inactive)]

tests updated due to bug 409252

Checking in e4x/Regress/regress-407323.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-407323.js,v  <--  regress-407323.js
new revision: 1.2; previous revision: 1.1
done
Checking in js1_5/Regress/regress-407323.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-407323.js,v  <--  regress-407323.js
new revision: 1.2; previous revision: 1.1
done

Comment 12

[Bob Clary [:bc] (inactive)]

verified fixed 1.9.0