javascript removeNode only affects the view, NOT the DOM

RESOLVED FIXED

Status

()

RESOLVED FIXED
13 years ago
10 years ago

People

(Reporter: nicolas, Assigned: peterv)

Tracking

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

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

I'm using javascript to populate an html table using the DOM functions
createElement, appendChild, etc. But there is a problem when I try to remove the
rows:
  I first loop in the childNodes of the tbody and remove some of the rows. The
result is immediately visible in the page.
  Then I reloop in the childNodes of the tbody to remove some other rows.
Surprisingly, the rows I removed first are still there (from a javascript point
of view) and if I try to remove one of those ghosts, I get the following error:

Error: uncaught exception: [Exception... "Node was not found"  code: "8"
nsresult: "0x80530008 (NS_ERROR_DOM_NOT_FOUND_ERR)"  location:
"http://localhost/functions.js Line: 58"]

If I go to the DOMInspector, the rows don't appear in the DOM tree, but are
still shown in the childNodes of the tbody in the Javascript Object view.

Here is the tree :
table
  tbody
    tr
      td
        input type=checkbox (checked: the row is to be removed)
      td
        text and image
    tr (same as above)
    tr (same as above)
    tr (same as above)

Reproducible: Always

Steps to Reproduce:
1. create a new file and paste this code:
<html>
<head>
<script type="text/javascript">
function removeFromList(tBody){
  n_tbody = document.getElementById(tBody);
  n_trs = n_tbody.childNodes;
  n_toRemove = Array();
  for(i_tr in n_trs){
    /* skip the non-tr nodes */
    if(n_trs[i_tr].nodeName==undefined ||
       n_trs[i_tr].nodeName.toLowerCase()!="tr") continue;
    /* get the checkbox directly (quick and dirty, without checks) */
    n_input = n_trs[i_tr].firstChild.firstChild;
    if(n_input.checked==true){
      n_toRemove.push(n_trs[i_tr]);
    }
  }
  /* remove the nodes */
  while(n_tr = n_toRemove.pop()) n_tbody.removeChild(n_tr); /* <-- error here */
}
</script>
</head>
<body>
<form>
<table>
<tbody id="foo">
<tr><td><input type="checkbox"/></td><td>cell1</td></tr>
<tr><td><input type="checkbox"/></td><td>cell 1</td></tr>
<tr><td><input type="checkbox"/></td><td>cell 2</td></tr>
<tr><td><input type="checkbox"/></td><td>cell 3</td></tr>
<tr><td><input type="checkbox"/></td><td>cell 4</td></tr>
</tbody>
</table>
<input type="button" value="remove" onclick="removeFromList('foo')"/>
</form>
</body>
</html>
2. Open the page with firefox, select AT LEAST TWO checkboxes and click "remove"
3. Select another checkbox and click "remove"

Actual Results:  
The first time, the rows are removed
The second time, I get a javascript error
--> the DOM view shows 3 lines, the Javascript object shows 4!

BUT: when only one checkbox is selected at a time, everything goes well.

Expected Results:  
The rows should be removed, the Javascript object and dom tree sould be synchronized

Comment 1

13 years ago
Created attachment 185029 [details]
test case

This is the proposed code that causes the error as a testcase. I do get an
error: 

Error: uncaught exception: [Exception... "Node was not found"  code: "8"
nsresult: "0x80530008 (NS_ERROR_DOM_NOT_FOUND_ERR)"  location:
"file:///C:/Documents%20and%20Settings/Adam%20Guthrie/Desktop/testcase.html
Line: 19"]

but I'm not sure whether this has to do with just bad JavaScript of Firefox's
implementation of it.
(Reporter)

Comment 2

13 years ago
I've checked and double-checked the javascript code, but maybe I missed something.
But the thing that really strikes me is that the Javascript Object (childNodes
of the tbody object) does not match the DOM tree.

Comment 3

13 years ago
Created attachment 185139 [details]
Reduced Testcase

The initial code does not do what the author intended since it uses the in
operator with an array and thus loops through both the numeric array properties
and the length and item properties.

It does seem to reveal something strange though, see the testcase, try running
it with the for loop commented out to see the difference in behaviour.

->Core: DOM Core

Updated

13 years ago
Assignee: nobody → general
Component: General → DOM: Core
Product: Firefox → Core
QA Contact: general → ian
Version: unspecified → Trunk
(Assignee)

Comment 4

13 years ago
Created attachment 196017 [details] [diff] [review]
v1
Assignee: general → peterv
Status: UNCONFIRMED → ASSIGNED
Attachment #196017 - Flags: superreview?(jst)
Attachment #196017 - Flags: review?(jst)
(Assignee)

Comment 5

13 years ago
Comment on attachment 196017 [details] [diff] [review]
v1

Ugh, attached wrong patch, pretend |*vp = nsnull| is actually |*vp =
JSVAL_NULL|
Comment on attachment 196017 [details] [diff] [review]
v1

Looks good, but I'd say we should probably use JSVAL_VOID here, not JSVAL_NULL.
If you today do alert(nodelist[500000]), you'll get undefined, not null (except
if you're perverted enough to have that many nodes in the list, or course), so
accessing an entry by an index that used to exist but doesn't exist any more,
you should get back the same value as you did when the entry never did exist.

r+sr=jst
Attachment #196017 - Flags: superreview?(jst)
Attachment #196017 - Flags: superreview+
Attachment #196017 - Flags: review?(jst)
Attachment #196017 - Flags: review+
(Assignee)

Updated

13 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
No longer depends on: 100499
Resolution: --- → FIXED
Depends on: 310927

Updated

13 years ago
Depends on: 332238

Updated

10 years ago
Component: DOM: Core → DOM: Core & HTML
QA Contact: ian → general
You need to log in before you can comment on or make changes to this bug.