WebCore:
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 1 Jan 2008 01:17:22 +0000 (01:17 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 1 Jan 2008 01:17:22 +0000 (01:17 +0000)
        Reviewed by Mitz.

        - fix http://bugs.webkit.org/show_bug.cgi?id=16641
          Acid3 reveals HTMLFormElement.elements fails to update when element name changes

        Test: fast/dom/HTMLFormElement/elements-not-in-document.html

        This was a bug specific to forms that are not in the document tree.
        The fix was to change the code to increment the document version number to match
        up with other document change tracking. Maybe at some point we can clean these up
        so we don't have so many competing change notification systems.

        * dom/ContainerNode.cpp:
        (WebCore::ContainerNode::replaceChild): Removed bogus comment.
        (WebCore::ContainerNode::addChild): Added an explicit incDOMTreeVersion
        call here, since this code path bypasses the subtree-modified event code.

        * dom/Element.cpp:
        (WebCore::Element::setAttribute): Remove the inDocument() check -- not all HTML
        collections are for things in the document.
        (WebCore::Element::setAttributeMap): Ditto.

        * dom/EventTargetNode.cpp:
        (WebCore::EventTargetNode::dispatchSubtreeModifiedEvent): Added a call to
        incDOMTreeVersion here; covers most cases of tree structure changes.

        * dom/Node.cpp:
        (WebCore::Node::attach): Remove call to incDOMTreeVersion -- creating a renderer
        has nothing to do with changes to the DOM tree!
        (WebCore::Node::detach): Ditto.

        * html/HTMLFormElement.cpp:
        (WebCore::HTMLFormElement::registerFormElement): Remove call to incDOMTreeVersion.
        This is handled at a lower level and doesn't need to be here.
        (WebCore::HTMLFormElement::removeFormElement): Ditto.

LayoutTests:

        Reviewed by Mitz.

        - test for http://bugs.webkit.org/show_bug.cgi?id=16641
          Acid3 reveals HTMLFormElement.elements fails to update when element name changes

        * fast/dom/HTMLFormElement: Added.
        * fast/dom/HTMLFormElement/elements-not-in-document-expected.txt: Added.
        * fast/dom/HTMLFormElement/elements-not-in-document.html: Added.
        * fast/dom/HTMLFormElement/resources: Added.
        * fast/dom/HTMLFormElement/resources/TEMPLATE.html: Added.
        * fast/dom/HTMLFormElement/resources/elements-not-in-document.js: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@29053 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLFormElement/elements-not-in-document-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/HTMLFormElement/elements-not-in-document.html [new file with mode: 0644]
LayoutTests/fast/dom/HTMLFormElement/resources/TEMPLATE.html [new file with mode: 0644]
LayoutTests/fast/dom/HTMLFormElement/resources/elements-not-in-document.js [new file with mode: 0644]
WebCore/ChangeLog
WebCore/dom/ContainerNode.cpp
WebCore/dom/Element.cpp
WebCore/dom/EventTargetNode.cpp
WebCore/dom/Node.cpp
WebCore/html/HTMLFormElement.cpp

index 6586cdc99f3ef0771a257f5e0c9393d2fb62afa9..74560fc34c951f4a81c9c2b91898339cf7101460 100644 (file)
@@ -1,3 +1,17 @@
+2007-12-31  Darin Adler  <darin@apple.com>
+
+        Reviewed by Mitz.
+
+        - test for http://bugs.webkit.org/show_bug.cgi?id=16641
+          Acid3 reveals HTMLFormElement.elements fails to update when element name changes
+
+        * fast/dom/HTMLFormElement: Added.
+        * fast/dom/HTMLFormElement/elements-not-in-document-expected.txt: Added.
+        * fast/dom/HTMLFormElement/elements-not-in-document.html: Added.
+        * fast/dom/HTMLFormElement/resources: Added.
+        * fast/dom/HTMLFormElement/resources/TEMPLATE.html: Added.
+        * fast/dom/HTMLFormElement/resources/elements-not-in-document.js: Added.
+
 2007-12-31  Henry Mason  <hmason@mac.com>
 
         Reviewed by Darin.
diff --git a/LayoutTests/fast/dom/HTMLFormElement/elements-not-in-document-expected.txt b/LayoutTests/fast/dom/HTMLFormElement/elements-not-in-document-expected.txt
new file mode 100644 (file)
index 0000000..f786b61
--- /dev/null
@@ -0,0 +1,27 @@
+Test the elements collection when the form is not a descendant of the document. This test case failed in an early version of Acid3.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS i.getAttribute('name') is 'first'
+PASS i.name is 'first'
+PASS i.getAttribute('type') is 'text'
+PASS i.type is 'text'
+PASS i.value is 'test'
+PASS f.elements.length is 1
+PASS f.elements[0] is i
+PASS f.elements.first is i
+FAIL f.elements.second should be null (of type object). Was undefined (of type undefined).
+PASS i.getAttribute('name') is 'second'
+PASS i.name is 'second'
+PASS i.getAttribute('type') is 'password'
+PASS i.type is 'password'
+PASS i.value is 'TEST'
+PASS f.elements.length is 1
+PASS f.elements[0] is i
+PASS f.elements.second is i
+FAIL f.elements.first should be null (of type object). Was undefined (of type undefined).
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/HTMLFormElement/elements-not-in-document.html b/LayoutTests/fast/dom/HTMLFormElement/elements-not-in-document.html
new file mode 100644 (file)
index 0000000..f471de6
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../js/resources/js-test-style.css">
+<script src="../../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="resources/elements-not-in-document.js"></script>
+<script src="../../js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/HTMLFormElement/resources/TEMPLATE.html b/LayoutTests/fast/dom/HTMLFormElement/resources/TEMPLATE.html
new file mode 100644 (file)
index 0000000..1951c43
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../js/resources/js-test-style.css">
+<script src="../../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="YOUR_JS_FILE_HERE"></script>
+<script src="../../js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/HTMLFormElement/resources/elements-not-in-document.js b/LayoutTests/fast/dom/HTMLFormElement/resources/elements-not-in-document.js
new file mode 100644 (file)
index 0000000..da1ea80
--- /dev/null
@@ -0,0 +1,37 @@
+description('Test the elements collection when the form is not a descendant of the document. This test case failed in an early version of Acid3.');
+
+var f = document.createElement('form');
+var i = document.createElement('input');
+i.name = 'first';
+i.type = 'text';
+i.value = 'test';
+f.appendChild(i);
+
+shouldBe("i.getAttribute('name')", "'first'");
+shouldBe("i.name", "'first'");
+shouldBe("i.getAttribute('type')", "'text'");
+shouldBe("i.type", "'text'");
+shouldBe("i.value", "'test'");
+shouldBe("f.elements.length", "1");
+shouldBe("f.elements[0]", "i");
+shouldBe("f.elements.first", "i");
+
+f.elements.second;
+i.name = 'second';
+i.type = 'password';
+i.value = 'TEST';
+
+// This has to be the first expression tested, because reporting the result will fix the bug.
+shouldBe("f.elements.second", "i");
+
+shouldBe("i.getAttribute('name')", "'second'");
+shouldBe("i.name", "'second'");
+shouldBe("i.getAttribute('type')", "'password'");
+shouldBe("i.type", "'password'");
+shouldBe("i.value", "'TEST'");
+shouldBe("f.elements.length", "1");
+shouldBe("f.elements[0]", "i");
+shouldBe("f.elements.first", "undefined");
+shouldBe("f.elements.second", "i");
+
+var successfullyParsed = true;
index ac6ccf97da627a7580452cbfc533dc682796abe0..342534ebc43572b1acd96a2ac8e710063ad00a78 100644 (file)
@@ -1,3 +1,41 @@
+2007-12-31  Darin Adler  <darin@apple.com>
+
+        Reviewed by Mitz.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=16641
+          Acid3 reveals HTMLFormElement.elements fails to update when element name changes
+
+        Test: fast/dom/HTMLFormElement/elements-not-in-document.html
+
+        This was a bug specific to forms that are not in the document tree.
+        The fix was to change the code to increment the document version number to match
+        up with other document change tracking. Maybe at some point we can clean these up
+        so we don't have so many competing change notification systems.
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::replaceChild): Removed bogus comment.
+        (WebCore::ContainerNode::addChild): Added an explicit incDOMTreeVersion
+        call here, since this code path bypasses the subtree-modified event code.
+
+        * dom/Element.cpp:
+        (WebCore::Element::setAttribute): Remove the inDocument() check -- not all HTML
+        collections are for things in the document.
+        (WebCore::Element::setAttributeMap): Ditto.
+
+        * dom/EventTargetNode.cpp:
+        (WebCore::EventTargetNode::dispatchSubtreeModifiedEvent): Added a call to
+        incDOMTreeVersion here; covers most cases of tree structure changes.
+
+        * dom/Node.cpp:
+        (WebCore::Node::attach): Remove call to incDOMTreeVersion -- creating a renderer
+        has nothing to do with changes to the DOM tree!
+        (WebCore::Node::detach): Ditto.
+
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::registerFormElement): Remove call to incDOMTreeVersion.
+        This is handled at a lower level and doesn't need to be here.
+        (WebCore::HTMLFormElement::removeFormElement): Ditto.
+
 2007-12-31  Henry Mason  <hmason@mac.com>
 
         Reviewed by Darin.
index bb1e2a6483a1b34753d9362be840bd41c3b09f00..c890e897cc423be9a5cc41a09ccf47ad46618a2b 100644 (file)
@@ -323,7 +323,6 @@ bool ContainerNode::replaceChild(PassRefPtr<Node> newChild, Node* oldChild, Exce
         child = nextChild.release();
     }
 
-    // ### set style in case it's attached
     document()->setDocumentChanged(true);
     dispatchSubtreeModifiedEvent();
     return true;
@@ -577,6 +576,7 @@ ContainerNode* ContainerNode::addChild(PassRefPtr<Node> newChild)
     m_lastChild = newChild.get();
     allowEventDispatch();
 
+    document()->incDOMTreeVersion();
     if (inDocument())
         newChild->insertedIntoDocument();
     if (document()->hasNodeLists())
index cb52e597a6ecd99d87f4bc67d312f6734765b891..1ab3f318af07eca73212e160c535f9f583e52884 100644 (file)
@@ -461,8 +461,7 @@ void Element::setAttribute(const String& name, const String& value, ExceptionCod
         return;
     }
     
-    if (inDocument())
-        document()->incDOMTreeVersion();
+    document()->incDOMTreeVersion();
 
     if (localName == idAttr.localName())
         updateId(old ? old->value() : nullAtom, value);
@@ -479,8 +478,7 @@ void Element::setAttribute(const String& name, const String& value, ExceptionCod
 
 void Element::setAttribute(const QualifiedName& name, StringImpl* value, ExceptionCode& ec)
 {
-    if (inDocument())
-        document()->incDOMTreeVersion();
+    document()->incDOMTreeVersion();
 
     // allocate attributemap if necessary
     Attribute* old = attributes(false)->getAttributeItem(name);
@@ -511,8 +509,7 @@ Attribute* Element::createAttribute(const QualifiedName& name, StringImpl* value
 
 void Element::setAttributeMap(NamedAttrMap* list)
 {
-    if (inDocument())
-        document()->incDOMTreeVersion();
+    document()->incDOMTreeVersion();
 
     // If setting the whole map changes the id attribute, we need to call updateId.
 
index f44f3a48c3ae05f458d66be672476cf4b1675aa1..84f3ad532e1bac2028a6f2ce0828caa66200ce97 100644 (file)
@@ -118,6 +118,8 @@ bool EventTargetNode::dispatchSubtreeModifiedEvent(bool sendChildrenChanged)
 {
     ASSERT(!eventDispatchForbidden());
     
+    document()->incDOMTreeVersion();
+
     // FIXME: Pull this whole if clause out of this function.
     if (sendChildrenChanged) {
         notifyNodeListsChildrenChanged();
index d18a5c69b928943a7a037e2c62907a388b5751e1..698de9c8d53a0d8f383a5ffcd9d8c88eefa6f760 100644 (file)
@@ -797,7 +797,6 @@ void Node::attach()
 {
     ASSERT(!attached());
     ASSERT(!renderer() || (renderer()->style() && renderer()->parent()));
-    document()->incDOMTreeVersion();
     m_attached = true;
 }
 
@@ -818,7 +817,6 @@ void Node::detach()
         doc->hoveredNodeDetached(this);
     if (m_inActiveChain)
         doc->activeChainNodeDetached(this);
-    doc->incDOMTreeVersion();
 
     m_active = false;
     m_hovered = false;
index b2d8fa32d5625a0c7885079f030b882afb6038d7..c2d7b15b2880bf97197b2d49fd4e561d5f49eb74 100644 (file)
@@ -582,18 +582,15 @@ unsigned HTMLFormElement::formElementIndex(HTMLGenericFormElement *e)
 
 void HTMLFormElement::registerFormElement(HTMLGenericFormElement* e)
 {
-    Document* doc = document();
-    doc->checkedRadioButtons().removeButton(e);
+    document()->checkedRadioButtons().removeButton(e);
     m_checkedRadioButtons.addButton(e);
     formElements.insert(formElementIndex(e), e);
-    doc->incDOMTreeVersion();
 }
 
 void HTMLFormElement::removeFormElement(HTMLGenericFormElement* e)
 {
     m_checkedRadioButtons.removeButton(e);
     removeFromVector(formElements, e);
-    document()->incDOMTreeVersion();
 }
 
 bool HTMLFormElement::isURLAttribute(Attribute *attr) const