376105 - Dynamic attributes aren't quoted correctly

Status: RESOLVED WORKSFORME

(Rhino Graveyard :: E4X, defect)

Description

[Stefan Matthias Aust]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Rhino CVS HEAD as of March, 31th 2007

Dynamic attributes aren't quoted if the attribute value evaluates to a non-empty string. I think, this is an error. I'd added steps to reproduce this with the current CVS version of Rhino and also included a patch to fix the problem.

Reproducible: Always

Steps to Reproduce:
Eval var x='foo'; <bar a={x}/>.toXMLString()
Actual Results:  
Error message about a missing "

Expected Results:  
<bar a="foo"/>

Here's a patch:

### Eclipse Workspace Patch 1.0
#P rhino
Index: xmlimplsrc/org/mozilla/javascript/xmlimpl/XmlProcessor.java
===================================================================
RCS file: /cvsroot/mozilla/js/rhino/xmlimplsrc/org/mozilla/javascript/xmlimpl/XmlProcessor.java,v
retrieving revision 1.4
diff -u -r1.4 XmlProcessor.java
--- xmlimplsrc/org/mozilla/javascript/xmlimpl/XmlProcessor.java	7 Feb 2007 15:22:10 -0000	1.4
+++ xmlimplsrc/org/mozilla/javascript/xmlimpl/XmlProcessor.java	31 Mar 2007 16:39:09 -0000
@@ -278,7 +278,7 @@
 		String elementText = toString(e);
 		int begin = elementText.indexOf('"');
 		int end = elementText.lastIndexOf('"');
-		return elementText.substring(begin+1,end);
+		return elementText.substring(begin, end + 1);
 	}
 
 	String escapeTextValue(Object value) {

Comment 1

[David Caldwell]

I can't reproduce this.  Can you help?  Here's what I get (it doesn't look like there have been any changes since 31-March):

$ java -jar js.jar
Rhino 1.6 release 6 Pre 2007 04 06
js> var x = 'foo'; <bar a={x}/>.toXMLString()
<bar a="foo"/>

Comment 2

[David Caldwell]

Is there something strange about your classpath, maybe?  The quoting semantics of escapeAttributeValue did change between releases.  I can reproduce the exact *opposite* of your problem on HEAD (see bug 378580), which would seem to indicate that you are somehow picking up an implementation with the old semantics (maybe a version of Rhino compiled prior to the change combined with an E4X implementation compiled after the change).

Comment 3

[David Caldwell]

Bug 378580 has been fixed.  I suspect that a clean checkout would fix your problem, or that updating would now make it never work, not even for empty strings. :)

Comment 4

[Norris Boyd]

Resolving as WORKSFORME.