javascript removeNode only affects the view, NOT the DOM




13 years ago
10 years ago


(Reporter: nicolas, Assigned: peterv)


Windows XP
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(3 attachments)



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
  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 :
        input type=checkbox (checked: the row is to be removed)
        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:
<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;
  /* remove the nodes */
  while(n_tr = n_toRemove.pop()) n_tbody.removeChild(n_tr); /* <-- error here */
<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>
<input type="button" value="remove" onclick="removeFromList('foo')"/>
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: uncaught exception: [Exception... "Node was not found"  code: "8"
nsresult: "0x80530008 (NS_ERROR_DOM_NOT_FOUND_ERR)"  location:
Line: 19"]

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

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


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

Comment 4

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

Comment 5

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

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

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.

Attachment #196017 - Flags: superreview?(jst)
Attachment #196017 - Flags: superreview+
Attachment #196017 - Flags: review?(jst)
Attachment #196017 - Flags: review+


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


13 years ago
Depends on: 332238


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.