Bug 16641

Summary: Acid3 reveals HTMLFormElement.elements fails to update when element name changes
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: DOMAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, ap, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
patch mitz: review+

Description Eric Seidel (no email) 2007-12-28 02:21:10 PST
Acid3 reveals HTMlFormElement.elements fails to update when element name changes

From this test:

    function () {
      // test 68: changing a dynamic <input>
      var ok = true;
      var f = document.createElement('form');
      var i = document.createElement('input');
      i.name = 'first';
      i.type = 'text';
      i.value = 'test';
      f.appendChild(i);
      if (i.getAttribute('name') != 'first' ||
          i.name != 'first' ||
          i.getAttribute('type') != 'text' ||
          i.type != 'text' ||
          i.value != 'test' ||
          f.elements.length != 1 ||
          f.elements[0] != i ||
          f.elements.first != i ||
          f.elements.second != null)
        ok = false;
      i.name = 'second';
      i.type = 'password';
      i.value = 'TEST';
      if (i.getAttribute('name') != 'second' ||
          i.name != 'second' ||
          i.getAttribute('type') != 'password' ||
          i.type != 'password' ||
          i.value != 'TEST' ||
          f.elements.length != 1 ||
          f.elements[0] != i ||
          f.elements.second != i ||
          f.elements.first != null)
        ok = false;
      if (ok)
        return 5;
    },

We only fail this check "f.elements.second != i".
Comment 1 Eric Seidel (no email) 2007-12-30 23:13:45 PST
The problem is JSNamedNodesCollection.  It caches the nodes at time of creation, and then it's not gone from the DOM cache until a GC.  Meaning if any of the nodes change, it's stale.  We'll probably move to something more like a NodeList.
Comment 2 Darin Adler 2007-12-31 16:29:22 PST
(In reply to comment #1)
> The problem is JSNamedNodesCollection.  It caches the nodes at time of
> creation, and then it's not gone from the DOM cache until a GC.  Meaning if any
> of the nodes change, it's stale.  We'll probably move to something more like a
> NodeList.

I don't think that analysis is correct. Each call to "second" will create a unique JSNamedNodesCollection.

The bug here is presumably something in HTMLCollection, not the DOM binding. Perhaps nameCache is not getting invalidated. Perhaps the DOM tree version isn't getting updated when the name is changed.
Comment 3 Darin Adler 2007-12-31 16:35:27 PST
This bug is specific to an HTMLFormElement that is not currently in its owner document.

The bug is in the document's DOMTreeVersion mechanism. It only increments if the node is in the document. But the form collection works even for forms that are not "in" the document. The best fix for this is to remove the inDocument() checks at all the call sites for incDOMTreeVersion.
Comment 4 Darin Adler 2007-12-31 17:03:51 PST
Created attachment 18220 [details]
patch
Comment 5 mitz 2007-12-31 17:08:57 PST
Comment on attachment 18220 [details]
patch

r=me
Comment 6 Darin Adler 2007-12-31 17:17:40 PST
Committed revision 29053.