Title: [217524] trunk
Revision
217524
Author
[email protected]
Date
2017-05-27 13:15:02 -0700 (Sat, 27 May 2017)

Log Message

imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute.html is crashing
https://bugs.webkit.org/show_bug.cgi?id=172472
<rdar://problem/32334831>

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

* web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute-expected.txt:
Rebaseline test now that more checks are passing. We were previously wrongly resetting the input form owner
to null when removing the form from the document and the input had a form attribute set and was a descendant
of the form.

Source/WebCore:

Fix assertion hit when running imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute.html.

When the form was removed from the document, A descendant would try to find a new form owner in the document. If the descendant had
a form content attribute and there was another form in the document with this ID, then we would erroneously associate the descendant with
that other form, even though that descendant is being disconnected. This is because when the form with the given id is removed, we
notify the IdTargetObservers of the change. In this case, the form control is an IdTargetObserver and gets notified after
removedFrom() has been called on the form but *before* removedFrom() has been called on its descendant form control. As a result, the
form control still thinks it is in the tree (i.e. isConnected() wrongly returns true) and we make the wrong decision and try to
associate it with another form in the document.

To address the problem, we leverage the fact that when a form element is being removed, it already notifies its associated form
controls that it is being removed. When it does, we make sure to clear the control's id observer if the form is its ancestor.
The ID observer is no longer needed beyond this point since the control is now disconnected from the document, and the ID observer
callback would erroneously associate it with another form element in the document of the same ID because isConnected() still returns
true at that point.
As a result, the control's form owner is kept unchanged, which is the right thing to do here, since it is its ancestor, even
though both are detached.

Test: fast/dom/HTMLFormElement/form-removal-duplicate-id-crash.html

* dom/ContainerNode.h:
(WebCore::Node::rootNode):
Inline rootNode to avoid an extra function call in the fast path case. For the slow path, we now
call traverseToRootNode() to avoid duolicating logic.

* dom/Node.cpp:
(WebCore::Node::traverseToRootNode):
Add a traverseToRootNode() method which gets the root node by traversing the ancestors. This logic was duplicated in 3 places:
- Slow path in Node::rootNode()
- computeRootNode() in FormAssociatedElement.cpp
- findRoot() in HTMLFormElement.cpp
They are now consolidated in a single place to avoid duplication.

* dom/Node.h:
* html/FormAssociatedElement.cpp:
(WebCore::FormAssociatedElement::removedFrom):
Just simplify the logic a bit:
- Clear the id observer (i.e. m_formAttributeTargetObserver) no matter what. Since the element is no longer part of the document,
  it is no longer needed. We would previously have checks that would basically avoid resetting m_formAttributeTargetObserver to
  null if it is already null. Settign m_formAttributeTargetObserver to null is cheap so there is no reason for those checks. Those
  checks were also confusing because they made it look like we would sometimes keep on id observer after being removed from the
  document.
- Use new traverseToRootNode() utility function (no behavior change)
- Drop unnecessary |element| local variable

(WebCore::FormAssociatedElement::formOwnerRemovedFromTree):
- Rename to formOwnerRemovedFromTree() to make it clear that it is the element's form owner that is removed, and not just any form.
- As we traverse the tree up to find the root, also check if we find the form owner. If we do, clear the id observer since we are
  effectively detached from the document and return early since there is no need to reset our form owner in this case.

* html/FormAssociatedElement.h:
* html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::removedFrom):
- Use new traverseToRootNode() utility function (no behavior change)

LayoutTests:

* TestExpectations:
Unskip test that is no longer crashing in Debug builds.

* fast/dom/HTMLFormElement/form-removal-duplicate-id-crash-expected.txt: Added.
* fast/dom/HTMLFormElement/form-removal-duplicate-id-crash.html: Added.
Add reduced test case reproducing the crash.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (217523 => 217524)


--- trunk/LayoutTests/ChangeLog	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/LayoutTests/ChangeLog	2017-05-27 20:15:02 UTC (rev 217524)
@@ -1,3 +1,18 @@
+2017-05-27  Chris Dumez  <[email protected]>
+
+        imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute.html is crashing
+        https://bugs.webkit.org/show_bug.cgi?id=172472
+        <rdar://problem/32334831>
+
+        Reviewed by Ryosuke Niwa.
+
+        * TestExpectations:
+        Unskip test that is no longer crashing in Debug builds.
+
+        * fast/dom/HTMLFormElement/form-removal-duplicate-id-crash-expected.txt: Added.
+        * fast/dom/HTMLFormElement/form-removal-duplicate-id-crash.html: Added.
+        Add reduced test case reproducing the crash.
+
 2017-05-27  Simon Fraser  <[email protected]>
 
         getComputedStyle returns percentage values for left / right / top / bottom

Modified: trunk/LayoutTests/TestExpectations (217523 => 217524)


--- trunk/LayoutTests/TestExpectations	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/LayoutTests/TestExpectations	2017-05-27 20:15:02 UTC (rev 217524)
@@ -802,8 +802,6 @@
 
 imported/w3c/web-platform-tests/html/browsers/history/the-history-interface/history_go_zero.html [ Pass Failure ]
 
-[ Debug ] imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute.html [ Skip ]
-
 # FIXME: The following failures need individual bugs.
 webkit.org/b/148805 imported/w3c/css/css-multicol-1/multicol-inherit-003.xht [ ImageOnlyFailure ]
 webkit.org/b/148805 imported/w3c/css/css-multicol-1/multicol-inherit-004.xht [ ImageOnlyFailure ]

Added: trunk/LayoutTests/fast/dom/HTMLFormElement/form-removal-duplicate-id-crash-expected.txt (0 => 217524)


--- trunk/LayoutTests/fast/dom/HTMLFormElement/form-removal-duplicate-id-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/HTMLFormElement/form-removal-duplicate-id-crash-expected.txt	2017-05-27 20:15:02 UTC (rev 217524)
@@ -0,0 +1,12 @@
+Tests that we do not crash when removing a form with an associated child when there is another form with the same id in the document.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS control.form is form1
+PASS control.form is form1
+PASS control.form is form1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/dom/HTMLFormElement/form-removal-duplicate-id-crash.html (0 => 217524)


--- trunk/LayoutTests/fast/dom/HTMLFormElement/form-removal-duplicate-id-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/HTMLFormElement/form-removal-duplicate-id-crash.html	2017-05-27 20:15:02 UTC (rev 217524)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<div id="placeholder"></div>
+<script>
+description("Tests that we do not crash when removing a form with an associated child when there is another form with the same id in the document.");
+jsTestIsAsync = true;
+
+_onload_ = function() {
+    var placeholder = document.getElementById("placeholder");
+
+    form1 = document.createElement("form");
+    form2 = document.createElement("form");
+    form1.id = "form1";
+    form2.id = "form2";
+    placeholder.appendChild(form1);
+    placeholder.appendChild(form2);
+
+    control = document.createElement("button");
+    control.setAttribute("form", "form1");
+    form1.appendChild(control);
+    shouldBe("control.form", "form1");
+
+    form2.id = "form1";
+    shouldBe("control.form", "form1");
+
+    form1.parentNode.appendChild(form2);
+    shouldBe("control.form", "form1");
+
+    placeholder.removeChild(placeholder.firstChild);
+    finishJSTest();
+}
+</script>
+</body>
+</html>

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (217523 => 217524)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2017-05-27 20:15:02 UTC (rev 217524)
@@ -1,3 +1,16 @@
+2017-05-27  Chris Dumez  <[email protected]>
+
+        imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute.html is crashing
+        https://bugs.webkit.org/show_bug.cgi?id=172472
+        <rdar://problem/32334831>
+
+        Reviewed by Ryosuke Niwa.
+
+        * web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute-expected.txt:
+        Rebaseline test now that more checks are passing. We were previously wrongly resetting the input form owner
+        to null when removing the form from the document and the input had a form attribute set and was a descendant
+        of the form.
+
 2017-05-27  Simon Fraser  <[email protected]>
 
         getComputedStyle returns percentage values for left / right / top / bottom

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute-expected.txt (217523 => 217524)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute-expected.txt	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute-expected.txt	2017-05-27 20:15:02 UTC (rev 217524)
@@ -15,7 +15,7 @@
 PASS [BUTTON] When the id of a non-ancestor form changes from not being a match for the form attribute to being a match, the control's form owner is reset 
 PASS [BUTTON] When form element with same ID as the control's form attribute is inserted earlier in tree order, the form owner is changed to the inserted form 
 PASS [BUTTON] When non-form element with same ID as the control's form attribute is inserted earlier in tree order, the control does not have a form owner 
-FAIL [BUTTON] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists assert_equals: expected Element node <form id="form1"><button form="form1"></button></form> but got null
+PASS [BUTTON] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists 
 PASS [FIELDSET] Basic form association - control with no form attribute is associated with ancestor 
 PASS [FIELDSET] Form owner is reset to null when control's form attribute is set to an ID that does not exist in the document 
 PASS [FIELDSET] Control whose form attribute is an empty string has no form owner 
@@ -29,7 +29,7 @@
 PASS [FIELDSET] When the id of a non-ancestor form changes from not being a match for the form attribute to being a match, the control's form owner is reset 
 PASS [FIELDSET] When form element with same ID as the control's form attribute is inserted earlier in tree order, the form owner is changed to the inserted form 
 PASS [FIELDSET] When non-form element with same ID as the control's form attribute is inserted earlier in tree order, the control does not have a form owner 
-FAIL [FIELDSET] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists assert_equals: expected Element node <form id="form1"><fieldset form="form1"></fieldset></form> but got null
+PASS [FIELDSET] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists 
 PASS [INPUT] Basic form association - control with no form attribute is associated with ancestor 
 PASS [INPUT] Form owner is reset to null when control's form attribute is set to an ID that does not exist in the document 
 PASS [INPUT] Control whose form attribute is an empty string has no form owner 
@@ -43,7 +43,7 @@
 PASS [INPUT] When the id of a non-ancestor form changes from not being a match for the form attribute to being a match, the control's form owner is reset 
 PASS [INPUT] When form element with same ID as the control's form attribute is inserted earlier in tree order, the form owner is changed to the inserted form 
 PASS [INPUT] When non-form element with same ID as the control's form attribute is inserted earlier in tree order, the control does not have a form owner 
-FAIL [INPUT] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists assert_equals: expected Element node <form id="form1"><input form="form1"></form> but got null
+PASS [INPUT] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists 
 PASS [OBJECT] Basic form association - control with no form attribute is associated with ancestor 
 PASS [OBJECT] Form owner is reset to null when control's form attribute is set to an ID that does not exist in the document 
 PASS [OBJECT] Control whose form attribute is an empty string has no form owner 
@@ -57,7 +57,7 @@
 PASS [OBJECT] When the id of a non-ancestor form changes from not being a match for the form attribute to being a match, the control's form owner is reset 
 PASS [OBJECT] When form element with same ID as the control's form attribute is inserted earlier in tree order, the form owner is changed to the inserted form 
 PASS [OBJECT] When non-form element with same ID as the control's form attribute is inserted earlier in tree order, the control does not have a form owner 
-FAIL [OBJECT] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists assert_equals: expected Element node <form id="form1"><object form="form1"></object></form> but got null
+PASS [OBJECT] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists 
 PASS [OUTPUT] Basic form association - control with no form attribute is associated with ancestor 
 PASS [OUTPUT] Form owner is reset to null when control's form attribute is set to an ID that does not exist in the document 
 PASS [OUTPUT] Control whose form attribute is an empty string has no form owner 
@@ -71,7 +71,7 @@
 PASS [OUTPUT] When the id of a non-ancestor form changes from not being a match for the form attribute to being a match, the control's form owner is reset 
 PASS [OUTPUT] When form element with same ID as the control's form attribute is inserted earlier in tree order, the form owner is changed to the inserted form 
 PASS [OUTPUT] When non-form element with same ID as the control's form attribute is inserted earlier in tree order, the control does not have a form owner 
-FAIL [OUTPUT] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists assert_equals: expected Element node <form id="form1"><output form="form1"></output></form> but got null
+PASS [OUTPUT] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists 
 PASS [SELECT] Basic form association - control with no form attribute is associated with ancestor 
 PASS [SELECT] Form owner is reset to null when control's form attribute is set to an ID that does not exist in the document 
 PASS [SELECT] Control whose form attribute is an empty string has no form owner 
@@ -85,7 +85,7 @@
 PASS [SELECT] When the id of a non-ancestor form changes from not being a match for the form attribute to being a match, the control's form owner is reset 
 PASS [SELECT] When form element with same ID as the control's form attribute is inserted earlier in tree order, the form owner is changed to the inserted form 
 PASS [SELECT] When non-form element with same ID as the control's form attribute is inserted earlier in tree order, the control does not have a form owner 
-FAIL [SELECT] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists assert_equals: expected Element node <form id="form1"><select form="form1"></select></form> but got null
+PASS [SELECT] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists 
 PASS [TEXTAREA] Basic form association - control with no form attribute is associated with ancestor 
 PASS [TEXTAREA] Form owner is reset to null when control's form attribute is set to an ID that does not exist in the document 
 PASS [TEXTAREA] Control whose form attribute is an empty string has no form owner 
@@ -99,5 +99,5 @@
 PASS [TEXTAREA] When the id of a non-ancestor form changes from not being a match for the form attribute to being a match, the control's form owner is reset 
 PASS [TEXTAREA] When form element with same ID as the control's form attribute is inserted earlier in tree order, the form owner is changed to the inserted form 
 PASS [TEXTAREA] When non-form element with same ID as the control's form attribute is inserted earlier in tree order, the control does not have a form owner 
-FAIL [TEXTAREA] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists assert_equals: expected Element node <form id="form1"><textarea form="form1"></textarea></form> but got null
+PASS [TEXTAREA] A control that is not in the document but has the form attribute set is associated with the nearest ancestor form if one exists 
 

Modified: trunk/Source/WebCore/ChangeLog (217523 => 217524)


--- trunk/Source/WebCore/ChangeLog	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/Source/WebCore/ChangeLog	2017-05-27 20:15:02 UTC (rev 217524)
@@ -1,3 +1,66 @@
+2017-05-27  Chris Dumez  <[email protected]>
+
+        imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute.html is crashing
+        https://bugs.webkit.org/show_bug.cgi?id=172472
+        <rdar://problem/32334831>
+
+        Reviewed by Ryosuke Niwa.
+
+        Fix assertion hit when running imported/w3c/web-platform-tests/html/semantics/forms/form-control-infrastructure/form_attribute.html.
+
+        When the form was removed from the document, A descendant would try to find a new form owner in the document. If the descendant had 
+        a form content attribute and there was another form in the document with this ID, then we would erroneously associate the descendant with
+        that other form, even though that descendant is being disconnected. This is because when the form with the given id is removed, we
+        notify the IdTargetObservers of the change. In this case, the form control is an IdTargetObserver and gets notified after
+        removedFrom() has been called on the form but *before* removedFrom() has been called on its descendant form control. As a result, the
+        form control still thinks it is in the tree (i.e. isConnected() wrongly returns true) and we make the wrong decision and try to
+        associate it with another form in the document.
+
+        To address the problem, we leverage the fact that when a form element is being removed, it already notifies its associated form
+        controls that it is being removed. When it does, we make sure to clear the control's id observer if the form is its ancestor.
+        The ID observer is no longer needed beyond this point since the control is now disconnected from the document, and the ID observer
+        callback would erroneously associate it with another form element in the document of the same ID because isConnected() still returns
+        true at that point.
+        As a result, the control's form owner is kept unchanged, which is the right thing to do here, since it is its ancestor, even
+        though both are detached.
+
+        Test: fast/dom/HTMLFormElement/form-removal-duplicate-id-crash.html
+
+        * dom/ContainerNode.h:
+        (WebCore::Node::rootNode):
+        Inline rootNode to avoid an extra function call in the fast path case. For the slow path, we now
+        call traverseToRootNode() to avoid duolicating logic.
+
+        * dom/Node.cpp:
+        (WebCore::Node::traverseToRootNode):
+        Add a traverseToRootNode() method which gets the root node by traversing the ancestors. This logic was duplicated in 3 places:
+        - Slow path in Node::rootNode()
+        - computeRootNode() in FormAssociatedElement.cpp
+        - findRoot() in HTMLFormElement.cpp
+        They are now consolidated in a single place to avoid duplication.
+
+        * dom/Node.h:
+        * html/FormAssociatedElement.cpp:
+        (WebCore::FormAssociatedElement::removedFrom):
+        Just simplify the logic a bit:
+        - Clear the id observer (i.e. m_formAttributeTargetObserver) no matter what. Since the element is no longer part of the document,
+          it is no longer needed. We would previously have checks that would basically avoid resetting m_formAttributeTargetObserver to
+          null if it is already null. Settign m_formAttributeTargetObserver to null is cheap so there is no reason for those checks. Those
+          checks were also confusing because they made it look like we would sometimes keep on id observer after being removed from the
+          document.
+        - Use new traverseToRootNode() utility function (no behavior change)
+        - Drop unnecessary |element| local variable
+
+        (WebCore::FormAssociatedElement::formOwnerRemovedFromTree):
+        - Rename to formOwnerRemovedFromTree() to make it clear that it is the element's form owner that is removed, and not just any form.
+        - As we traverse the tree up to find the root, also check if we find the form owner. If we do, clear the id observer since we are
+          effectively detached from the document and return early since there is no need to reset our form owner in this case.
+
+        * html/FormAssociatedElement.h:
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::removedFrom):
+        - Use new traverseToRootNode() utility function (no behavior change)
+
 2017-05-27  Yusuke Suzuki  <[email protected]>
 
         [DOMJIT] Move DOMJIT patchpoint infrastructure out of domjit

Modified: trunk/Source/WebCore/dom/ContainerNode.h (217523 => 217524)


--- trunk/Source/WebCore/dom/ContainerNode.h	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/Source/WebCore/dom/ContainerNode.h	2017-05-27 20:15:02 UTC (rev 217524)
@@ -194,6 +194,13 @@
     return &treeScope().rootNode() == this;
 }
 
+inline Node& Node::rootNode() const
+{
+    if (isInTreeScope())
+        return treeScope().rootNode();
+    return traverseToRootNode();
+}
+
 // This constant controls how much buffer is initially allocated
 // for a Node Vector that is used to store child Nodes of a given Node.
 // FIXME: Optimize the value.

Modified: trunk/Source/WebCore/dom/Node.cpp (217523 => 217524)


--- trunk/Source/WebCore/dom/Node.cpp	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/Source/WebCore/dom/Node.cpp	2017-05-27 20:15:02 UTC (rev 217524)
@@ -1223,11 +1223,8 @@
     return downcast<Element>(parent);
 }
 
-Node& Node::rootNode() const
+Node& Node::traverseToRootNode() const
 {
-    if (isInTreeScope())
-        return treeScope().rootNode();
-
     Node* node = const_cast<Node*>(this);
     Node* highest = node;
     for (; node; node = node->parentNode())

Modified: trunk/Source/WebCore/dom/Node.h (217523 => 217524)


--- trunk/Source/WebCore/dom/Node.h	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/Source/WebCore/dom/Node.h	2017-05-27 20:15:02 UTC (rev 217524)
@@ -260,6 +260,7 @@
     Element* parentOrShadowHostElement() const;
     void setParentNode(ContainerNode*);
     Node& rootNode() const;
+    Node& traverseToRootNode() const;
     Node& shadowIncludingRoot() const;
 
     struct GetRootNodeOptions {

Modified: trunk/Source/WebCore/html/FormAssociatedElement.cpp (217523 => 217524)


--- trunk/Source/WebCore/html/FormAssociatedElement.cpp	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/Source/WebCore/html/FormAssociatedElement.cpp	2017-05-27 20:15:02 UTC (rev 217524)
@@ -26,6 +26,7 @@
 #include "FormAssociatedElement.h"
 
 #include "EditorClient.h"
+#include "ElementAncestorIterator.h"
 #include "FormController.h"
 #include "Frame.h"
 #include "HTMLFormControlElement.h"
@@ -87,25 +88,14 @@
         resetFormAttributeTargetObserver();
 }
 
-// Compute the highest ancestor instead of calling Node::rootNode in removedFrom / formRemovedFromTree
-// since isConnected flag on some form associated elements may not have been updated yet.
-static Node* computeRootNode(Node& node)
+void FormAssociatedElement::removedFrom(ContainerNode&)
 {
-    Node* current = &node;
-    Node* parent = current;
-    while ((current = current->parentNode()))
-        parent = current;
-    return parent;
-}
+    m_formAttributeTargetObserver = nullptr;
 
-void FormAssociatedElement::removedFrom(ContainerNode& insertionPoint)
-{
-    HTMLElement& element = asHTMLElement();
-    if (insertionPoint.isConnected() && element.hasAttributeWithoutSynchronization(formAttr))
-        m_formAttributeTargetObserver = nullptr;
     // If the form and element are both in the same tree, preserve the connection to the form.
     // Otherwise, null out our form and remove ourselves from the form's list of elements.
-    if (m_form && computeRootNode(element) != computeRootNode(*m_form))
+    // Do not rely on rootNode() because our IsInTreeScope is outdated.
+    if (m_form && &asHTMLElement().traverseToRootNode() != &m_form->traverseToRootNode())
         setForm(nullptr);
 }
 
@@ -130,10 +120,22 @@
     return currentAssociatedForm;
 }
 
-void FormAssociatedElement::formRemovedFromTree(const Node* formRoot)
+void FormAssociatedElement::formOwnerRemovedFromTree(const Node& formRoot)
 {
     ASSERT(m_form);
-    if (computeRootNode(asHTMLElement()) != formRoot)
+    Node* rootNode = &asHTMLElement();
+    for (auto* ancestor = asHTMLElement().parentNode(); ancestor; ancestor = ancestor->parentNode()) {
+        if (ancestor == m_form) {
+            // Form is our ancestor so we don't need to reset our owner, we also no longer
+            // need an id observer since we are no longer connected.
+            m_formAttributeTargetObserver = nullptr;
+            return;
+        }
+        rootNode = ancestor;
+    }
+
+    // We are no longer in the same tree as our form owner so clear our owner.
+    if (rootNode != &formRoot)
         setForm(nullptr);
 }
 

Modified: trunk/Source/WebCore/html/FormAssociatedElement.h (217523 => 217524)


--- trunk/Source/WebCore/html/FormAssociatedElement.h	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/Source/WebCore/html/FormAssociatedElement.h	2017-05-27 20:15:02 UTC (rev 217524)
@@ -65,7 +65,7 @@
 
     void resetFormOwner();
 
-    void formRemovedFromTree(const Node* formRoot);
+    void formOwnerRemovedFromTree(const Node&);
 
     // ValidityState attribute implementations
     bool badInput() const { return hasBadInput(); }

Modified: trunk/Source/WebCore/html/HTMLFormElement.cpp (217523 => 217524)


--- trunk/Source/WebCore/html/HTMLFormElement.cpp	2017-05-27 19:03:41 UTC (rev 217523)
+++ trunk/Source/WebCore/html/HTMLFormElement.cpp	2017-05-27 20:15:02 UTC (rev 217524)
@@ -133,20 +133,12 @@
     return InsertionDone;
 }
 
-static inline Node* findRoot(Node* n)
-{
-    Node* root = n;
-    for (; n; n = n->parentNode())
-        root = n;
-    return root;
-}
-
 void HTMLFormElement::removedFrom(ContainerNode& insertionPoint)
 {
-    Node* root = findRoot(this);
+    Node& root = traverseToRootNode(); // Do not rely on rootNode() because our IsInTreeScope is outdated.
     Vector<FormAssociatedElement*> associatedElements(m_associatedElements);
     for (auto& associatedElement : associatedElements)
-        associatedElement->formRemovedFromTree(root);
+        associatedElement->formOwnerRemovedFromTree(root);
     HTMLElement::removedFrom(insertionPoint);
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to