If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[FIX]Form.elements not updated if form is not part of a document

RESOLVED FIXED in mozilla1.8beta2

Status

()

Core
DOM: Core & HTML
P1
normal
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: howard, Assigned: bz)

Tracking

Trunk
mozilla1.8beta2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040615 Firefox/0.9
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040615 Firefox/0.9

The following piece of code yields different results in Mozilla
and IE:

  f = document.createElement('form');
  ip1 = document.createElement('input');  ip1.type = 'text';
  f.appendChild(ip1);
  alert("1. Elements length=" + f.elements.length);
  document.body.appendChild(f);
  alert("2. Elements length=" + f.elements.length);
  ip2 = document.createElement('input');  ip2.type = 'text';
  f.appendChild(ip2);
  alert("3. Elements length=" + f.elements.length);
  document.body.removeChild(f);
  alert("4. Elements length=" + f.elements.length);
  f.removeChild(ip1);
  alert("5. Elements length=" + f.elements.length);
  
In IE (and Opera), the alerts read:

    1. Elements length=1
    2. Elements length=1
    3. Elements length=2
    4. Elements length=2
    5. Elements length=1
    
In Mozilla, they read:

    1. Elements length=0
    2. Elements length=1
    3. Elements length=2
    4. Elements length=2
    5. Elements length=1
    
Mozilla fails to update a form's elements collection as elements 
are added to the form, unless the form is part of a/the document.
However, it -does- update the collection as elements are removed
from the form, whether or not the form is part of the document.

According to bzbarsky:

    Yep.  That's exactly what we do, and it's a bug.
    
    ... the problem is in nsGenericHTMLFormElement::SetParent (the
    "non-null parent" branch does an IsInDocument() check that it 
    really doesn't need to do).


Reproducible: Always
Steps to Reproduce:
1. In a webpage script, enter the following

  f = document.createElement('form');
  ip1 = document.createElement('input');  ip1.type = 'text';
  f.appendChild(ip1);
  alert("1. Elements length=" + f.elements.length);
  document.body.appendChild(f);
  alert("2. Elements length=" + f.elements.length);
  ip2 = document.createElement('input');  ip2.type = 'text';
  f.appendChild(ip2);
  alert("3. Elements length=" + f.elements.length);
  document.body.removeChild(f);
  alert("4. Elements length=" + f.elements.length);
  f.removeChild(ip1);
  alert("5. Elements length=" + f.elements.length);

Actual Results:  
Five alert boxes:

    Elements length=0
    Elements length=1
    Elements length=2
    Elements length=2
    Elements length=1


Expected Results:  
    Elements length=1
    Elements length=1
    Elements length=2
    Elements length=2
    Elements length=1
Confirming; taking.  Patch coming up.
Assignee: general → bzbarsky
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P1
Target Milestone: --- → mozilla1.8alpha4
Created attachment 157828 [details] [diff] [review]
Possible patch
Comment on attachment 157828 [details] [diff] [review]
Possible patch

This helps the immediate problem, but doesn't help the case when we append the
form control to a div that we append to the form...  To solve that one, we'd
probably need BindToTree (which would recurse down the tree even in the cases
when we're not ending up in a document, because it would need to set
bindingParent correctly).
Attachment #157828 - Flags: superreview?(jst)
Attachment #157828 - Flags: review?(jst)
Comment on attachment 157828 [details] [diff] [review]
Possible patch

Looks good. r+sr=jst
Attachment #157828 - Flags: superreview?(jst)
Attachment #157828 - Flags: superreview+
Attachment #157828 - Flags: review?(jst)
Attachment #157828 - Flags: review+
Note: the BindToTree bug is bug 211128
Summary: Form.elements not updated if form is not part of a document → [FIXr]Form.elements not updated if form is not part of a document
Fix checked in for 1.8a4
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

13 years ago
I'm looking at Mozilla 1.8b (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b)
Gecko/20050217) ... this bug appears to be unfixed.

Here's a (more descriptive?) test case:

<html><head><script type="text/javascript">
function fcounts() {
    var p = function(s) {
        var d = document.createElement('div');
        d.appendChild(document.createTextNode(s));
        document.body.appendChild(d);
    }
    
    var ip1 = document.createElement('input'); ip1.type = 'text'; ip1.name='ip1';
    var ip2 = document.createElement('input'); ip2.type = 'text'; ip2.name='ip2';
    var ip3 = document.createElement('input'); ip3.type = 'text'; ip3.name='ip3';
    var f = document.createElement('form');
    
    p("1. Elements.length(should be 0) = " + f.elements.length + "; innerHTML='"
+ f.innerHTML + "'");
    f.appendChild(ip1);
    p("2. Elements length(should be 1) = " + f.elements.length + "; innerHTML='"
+ f.innerHTML + "'");
    f.appendChild(ip2);
    p("3. Elements length(should be 2) = " + f.elements.length + "; innerHTML='"
+ f.innerHTML + "'");
    
    p(" --- appending form to document");
    document.body.insertBefore(f,document.body.firstChild);
    
    p("4. Elements length(should be 2) = " + f.elements.length + "; innerHTML='"
+ f.innerHTML + "'");
    f.appendChild(ip3);
    p("5. Elements length(should be 3) = " + f.elements.length + "; innerHTML='"
+ f.innerHTML + "'");

    p(" --- removing form from document");
    document.body.removeChild(f);
    p("6. Elements length(should be 3) = " + f.elements.length + "; innerHTML='"
+ f.innerHTML + "'");
    
    f.removeChild(ip1);
    p("7. Elements length(should be 2) = " + f.elements.length + "; innerHTML='"
+ f.innerHTML + "'");
    f.appendChild(ip1);
    p("8. Elements length(should be 3) = " + f.elements.length + "; innerHTML='"
+ f.innerHTML + "'");
}
</script></head><body onload="fcounts()"></body></html>
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 176102 [details]
First testcase for this bug
Created attachment 176103 [details]
Second testcase for this bug
Created attachment 176104 [details] [diff] [review]
Patch to really fix it

Howard, thank you for keeping me honest... I was sure I tested the patch.  :(

jst, the comment up front should explain what's going on here, I think.
Attachment #157828 - Attachment is obsolete: true
Attachment #176104 - Flags: superreview?(jst)
Attachment #176104 - Flags: review?(jst)
Summary: [FIXr]Form.elements not updated if form is not part of a document → [FIX]Form.elements not updated if form is not part of a document
Target Milestone: mozilla1.8alpha4 → mozilla1.8beta2
Comment on attachment 176104 [details] [diff] [review]
Patch to really fix it

Yeah, that makes a lot of sense :) r+sr=jst
Attachment #176104 - Flags: superreview?(jst)
Attachment #176104 - Flags: superreview+
Attachment #176104 - Flags: review?(jst)
Attachment #176104 - Flags: review+
Fixed, for real this time.
Status: REOPENED → RESOLVED
Last Resolved: 13 years ago13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.