Closed
Bug 314529
Opened 20 years ago
Closed 19 years ago
E4X: delete operator misses some descendants - e4x/Types/9.1.1.3.js
Categories
(Core :: JavaScript Engine, defect, P2)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.9alpha1
People
(Reporter: Seno.Aiko, Assigned: jerfa)
Details
(Keywords: testcase, verified1.8.1)
Attachments
(2 files)
|
383 bytes,
text/html
|
Details | |
|
2.32 KB,
patch
|
brendan
:
review+
brendan
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
delete sometimes doesn't delete everything it should. See the attached testcase for an example. Replacing |delete ecma357..HR| with |delete ecma357.HR| works, but that's doesn't help in deeper nested documents.
Build id: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051031 Firefox/1.6a1
|
||
Comment 2•20 years ago
|
||
Checking in Types/9.1.1.3.js;
/cvsroot/mozilla/js/tests/e4x/Types/9.1.1.3.js,v <-- 9.1.1.3.js
new revision: 1.5; previous revision: 1.4
done
Flags: testcase+
|
|
||
Updated•20 years ago
|
Summary: E4X: delete operator misses some descendants → E4X: delete operator misses some descendants - e4x/Types/9.1.1.3.js
|
||
Updated•20 years ago
|
Assignee: general → brendan
Priority: -- → P2
Target Milestone: --- → mozilla1.9alpha
|
Assignee | |
Comment 3•19 years ago
|
||
This code tries to hold two different indices in one variable.
Attachment #214908 -
Flags: review?(brendan)
|
|
||
Comment 4•19 years ago
|
||
Erik, if you would like CVS commit access, please file a bug in the mozilla.org product request it -- I'll vouch.
Looking at patch in a minute.
/be
|
|
||
Comment 5•19 years ago
|
||
Comment on attachment 214908 [details] [diff] [review]
don't reuse index
Great -- thanks for another fix. You really should get CVS access ;-).
/be
Attachment #214908 -
Flags: review?(brendan) → review+
|
|
Assignee | |
Comment 6•19 years ago
|
||
Thank you for the offer, I feel honored. :-) It would be a bit pointless to ask for CVS access, though. Now that I've survived the flu I wont spend the whole day in bed staring at the debugger because there's nothing better to do. No more patches in the foreseeable future...
Comment on attachment 214908 [details] [diff] [review]
don't reuse index
I'd like to see this fixed in Firefox 2.
Attachment #214908 -
Flags: approval-branch-1.8.1?
|
||
Updated•19 years ago
|
Assignee: brendan → jerfa
|
|
||
Updated•19 years ago
|
Attachment #214908 -
Flags: approval-branch-1.8.1? → approval-branch-1.8.1?(brendan)
|
|
||
Comment 8•19 years ago
|
||
mozilla/js/src/jsxml.c 3.99
You really should get CVS access :)
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
|
|
||
Updated•19 years ago
|
Attachment #214908 -
Flags: approval-branch-1.8.1?(brendan) → approval-branch-1.8.1+
|
|
||
Comment 9•19 years ago
|
||
Checked in on the 1.8 branch.
mozilla/js/src/jsxml.c 3.50.2.32
Keywords: fixed1.8.1
|
|
Assignee | |
Comment 10•19 years ago
|
||
I believe delete is now working as expected but e4x/Types/9.1.1.3.js is still failing. Dreaded whitespace handling, I'm not sure whether it's another bug in the engine or the testcase's fault. What is the expected value of the following expression if XML.ignoreWhitespace is true?
<body>ECMA-357</body> == <body>
ECMA-357</body>
If this should be true, then the bug is in the engine, otherwise the testcase should be fixed.
|
|
||
Comment 11•19 years ago
|
||
#0: function TEST(section=integer:6, expected=XML:{0}, actual=XML:{0}) in <http://test.mozilla.com/tests/mozilla.org/js/e4x/shell.js> line 133
133: if (expected != actual) {
0001: expected == actual
$[0] = [boolean] false
0001: expected.toString()
$[1] = [string] "\n ECMA-357\n"
0001: actual.toString()
$[2] = [string] "ECMA-357"
0001: expected.toXMLString()
$[3] = [string] "<BODY>ECMA-357</BODY>"
0001: actual.toXMLString()
$[4] = [string] "<BODY>ECMA-357</BODY>"
They aren't identical, but are equivalent. I think changing the test to compare the toXMLString values would be sufficient. Ok?
|
|
Assignee | |
Comment 12•19 years ago
|
||
I've now checked the Java engine and it returns true for the expression in comment 10, the opposite of SpiderMonkey. One of the two must be wrong unless the spec is so ambigous to allow both results.
But section 9.1.1.3 isn't about whitespace handling, so I guess it's ok to change the testcase.
|
|
||
Comment 13•19 years ago
|
||
Checking in 9.1.1.3.js;
/cvsroot/mozilla/js/tests/e4x/Types/9.1.1.3.js,v <-- 9.1.1.3.js
new revision: 1.6; previous revision: 1.5
|
|
||
Comment 14•19 years ago
|
||
verified fixed 1.8.1/1.9a1 win/mac/linux 20060415
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•