Closed Bug 358183 Opened 13 years ago Closed 13 years ago

XML equality does not compare all attributes

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: igor, Assigned: igor)

References

Details

(Keywords: verified1.8.0.9, verified1.8.1.1)

Attachments

(2 files)

The current implementation of XML equality in jsxml.c, XMLEquality function, can compares as equals XML elements with different set of attributes as the following example demonstrates:

@callisto~/m/trunk/mozilla/js/src> cat ~/m/files/y.js
var x = <a b="1"/>;
var y = <a b="1" c="2"/>;
if (x == y)
  throw "XML equality does not compare all attributes."

@callisto~/m/trunk/mozilla/js/src> ./Linux_All_DBG.OBJ/js ~/m/files/y.js
uncaught exception: XML equality does not compare all attributes.
Scheduling fix for JS1.7. 
Blocks: js1.7src
Flags: blocking1.8.1.1?
Summary: XML eqaulity does not compare all attributes → XML equality does not compare all attributes
Attached patch Fix v1Splinter Review
Attachment #243719 - Flags: review?(brendan)
Attachment #243719 - Flags: approval1.8.1.1?
Comment on attachment 243719 [details] [diff] [review]
Fix v1

D'oh, my blunder -- nice fix, thanks.

/be
Attachment #243719 - Flags: review?(brendan) → review+
I committed the patch from comment 2 to the trunk:

Checking in jsxml.c;
/cvsroot/mozilla/js/src/jsxml.c,v  <--  jsxml.c
new revision: 3.134; previous revision: 3.133
done
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I suggest to apply this to 1.8.0 branch as well.
Flags: blocking1.8.0.9?
Checking in 9.1.1.9.js;
/cvsroot/mozilla/js/tests/e4x/Types/9.1.1.9.js,v  <--  9.1.1.9.js
new revisio
Flags: in-testsuite+
verified fixed 1.9 20061028 windows/linux
Status: RESOLVED → VERIFIED
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1+
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.9+
Comment on attachment 243719 [details] [diff] [review]
Fix v1

approved for 1.8/1.8.0 branches, a=dveditz for drivers
Attachment #243719 - Flags: approval1.8.1.1?
Attachment #243719 - Flags: approval1.8.1.1+
Attachment #243719 - Flags: approval1.8.0.9+
I committed the patch from comment 2 to MOZILLA_1_8_BRANCH:

Checking in jsxml.c;
/cvsroot/mozilla/js/src/jsxml.c,v  <--  jsxml.c
new revision: 3.50.2.56; previous revision: 3.50.2.55
done
Keywords: fixed1.8.1.1
Attached patch Fix v1 for 1.8.0Splinter Review
The trunk patch does not applies as is due to the lack of js_EqualString on the branch that the trunk code uses hence a trivial port. 

Here is a diff with v1:

 diff  /home/igor/s/XMLEquality.patch /home/igor/s/XMLEquality-1.8.0.patch
4,8c4,8
< retrieving revision 3.133
< diff -U7 -p -r3.133 jsxml.c
< --- jsxml.c	26 Oct 2006 13:36:16 -0000	3.133
< +++ jsxml.c	26 Oct 2006 22:36:41 -0000
< @@ -3660,29 +3660,27 @@ retry:
---
> retrieving revision 3.50.2.15.2.26
> diff -U7 -p -r3.50.2.15.2.26 jsxml.c
> --- jsxml.c	7 Nov 2006 20:28:23 -0000	3.50.2.15.2.26
> +++ jsxml.c	9 Nov 2006 20:13:45 -0000
> @@ -3634,29 +3634,27 @@ retry:
29c29
<                  *bp = js_EqualStrings(attr->xml_value, vattr->xml_value);
---
>                  *bp = !js_CompareStrings(attr->xml_value, vattr->xml_value);

Diff finished at Thu Nov  9 21:14:09
Attachment #245131 - Flags: approval1.8.1.1?
Attachment #245131 - Flags: approval1.8.1.1? → approval1.8.0.9?
Attachment #243719 - Flags: approval1.8.0.9+
Comment on attachment 245131 [details] [diff] [review]
Fix v1 for 1.8.0

approved 1.8.0 branch, a=dveditz
Attachment #245131 - Flags: approval1.8.0.9? → approval1.8.0.9+
verified fixed 1.8.1.1 20061109 windows/linux/mac*
I committed the patch from comment 10 to MOZILLA_1_8_0_BRANCH:

Checking in jsxml.c;
/cvsroot/mozilla/js/src/jsxml.c,v  <--  jsxml.c
new revision: 3.50.2.15.2.27; previous revision: 3.50.2.15.2.26
done
Keywords: fixed1.8.0.9
verified fixed 1.8.0.9 20061111 windows/linux/mac* 
You need to log in before you can comment on or make changes to this bug.