Title: [184103] releases/WebKitGTK/webkit-2.8
Revision
184103
Author
[email protected]
Date
2015-05-11 08:47:11 -0700 (Mon, 11 May 2015)

Log Message

Merge r183436 - Form control may be associated with the wrong HTML Form element after form id change
https://bugs.webkit.org/show_bug.cgi?id=133456
<rdar://problem/17095055>

Reviewed by Andy Estes.

Source/WebCore:

Fixes an issue where a form control may be associated with the wrong HTML Form element
after the id of the HTML Form element associated with the form control is changed when
there is more than one HTML Form element with the same id in the document. Specifically,
a form control that has an HTML form attribute value X will always be associated with
some HTML Form element f where f.id = X regardless of whether f.id is subsequently
changed.

Tests: fast/forms/change-form-id-to-be-unique-then-submit-form.html
       fast/forms/change-form-id-to-be-unique.html

* dom/Element.cpp:
(WebCore::Element::attributeChanged): Notify observers when the id of an element changed.
(WebCore::Element::updateId): Added parameter NotifyObservers (defaults to NotifyObservers::Yes),
as to whether we should notify observers of the id change.
(WebCore::Element::updateIdForTreeScope): Ditto.
(WebCore::Element::willModifyAttribute): Do not notify observers of the id change immediately. As
indicated by the name of this method, we plan to modify the DOM attribute id of the element, but
we have not actually modified it when this method is called. Instead we will notify observers
in Element::attributeChanged(), which is called after the DOM attribute id is modified.
(WebCore::Element::cloneAttributesFromElement): Ditto.
* dom/Element.h: Defined enum class NotifyObservers.
* dom/TreeScope.cpp:
(WebCore::TreeScope::addElementById): Added boolean parameter notifyObservers (defaults to true)
as to whether we should dispatch a notification to all observers.
(WebCore::TreeScope::removeElementById): Ditto.
* dom/TreeScope.h:

LayoutTests:

Add tests to ensure that we associate the correct HTML Form element with a
<select> after changing the id of its associated HTML form element.

* fast/forms/change-form-id-to-be-unique-expected.txt: Added.
* fast/forms/change-form-id-to-be-unique-then-submit-form-expected.txt: Added.
* fast/forms/change-form-id-to-be-unique-then-submit-form.html: Added.
* fast/forms/change-form-id-to-be-unique.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog (184102 => 184103)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog	2015-05-11 15:40:47 UTC (rev 184102)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/ChangeLog	2015-05-11 15:47:11 UTC (rev 184103)
@@ -1,3 +1,19 @@
+2015-04-27  Daniel Bates  <[email protected]>
+
+        Form control may be associated with the wrong HTML Form element after form id change
+        https://bugs.webkit.org/show_bug.cgi?id=133456
+        <rdar://problem/17095055>
+
+        Reviewed by Andy Estes.
+
+        Add tests to ensure that we associate the correct HTML Form element with a
+        <select> after changing the id of its associated HTML form element.
+
+        * fast/forms/change-form-id-to-be-unique-expected.txt: Added.
+        * fast/forms/change-form-id-to-be-unique-then-submit-form-expected.txt: Added.
+        * fast/forms/change-form-id-to-be-unique-then-submit-form.html: Added.
+        * fast/forms/change-form-id-to-be-unique.html: Added.
+
 2015-04-27  Yoav Weiss  <[email protected]>
 
         Fix viewport units in Media Queries

Added: releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique-expected.txt (0 => 184103)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique-expected.txt	2015-05-11 15:47:11 UTC (rev 184103)
@@ -0,0 +1 @@
+PASS did not cause an assertion failure.

Added: releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique-then-submit-form-expected.txt (0 => 184103)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique-then-submit-form-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique-then-submit-form-expected.txt	2015-05-11 15:47:11 UTC (rev 184103)
@@ -0,0 +1,4 @@
+This tests that we submit the form element associated with id "a" after changing the id of one of the <form id="a">s in a document that contains two such HTML Form elements.
+
+PASS submitted second <form>.
+

Added: releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique-then-submit-form.html (0 => 184103)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique-then-submit-form.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique-then-submit-form.html	2015-05-11 15:47:11 UTC (rev 184103)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#test-container { visibility: hidden; }
+</style>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+function logMessageAndDone(message)
+{
+    document.getElementById("console").textContent = message;
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+
+function runTest()
+{
+    var search = document.location.search;
+    if (search === "?submitted=secondFormElement")
+        logMessageAndDone("PASS submitted second <form>.");
+    else if (search === "?submitted=firstFormElement")
+        logMessageAndDone("FAIL should have submitted second <form>, but submitted first <form>.");
+    else {
+        document.getElementById("a").id = "y"; // Changes the id of the first <form> (traversing the DOM from top-to-bottom).
+        document.getElementById("submit").click();
+    }
+}
+
+window._onload_ = runTest;
+</script>
+</head>
+<body>
+<p>This tests that we submit the form element associated with id &quot;a&quot; after changing the id of one of the &lt;form id=&quot;a&quot;&gt;s in a document that contains two such HTML Form elements.</p>
+<div id="console"></div>
+<div id="test-container">
+    <form id="a"><input type="hidden" name="submitted" value="firstFormElement"></form>
+    <form id="a"><input type="hidden" name="submitted" value="secondFormElement"></form>
+    <input id="submit" type="submit" form="a" value="Submit">
+</div>
+</body>
+</html>

Added: releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique.html (0 => 184103)


--- releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.8/LayoutTests/fast/forms/change-form-id-to-be-unique.html	2015-05-11 15:47:11 UTC (rev 184103)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+</head>
+<body _onload_="document.body.removeChild(document.getElementById('x'));">
+<p>PASS did not cause an assertion failure.</p>
+<div id="x">
+    <form id="a"></form>
+    <form id="a">
+        <select form="a"></select>
+    </form>
+</div>
+<script>
+document.getElementById("a").setAttribute("id", "y");
+</script>
+</body>
+</html>

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog (184102 => 184103)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog	2015-05-11 15:40:47 UTC (rev 184102)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/ChangeLog	2015-05-11 15:47:11 UTC (rev 184103)
@@ -1,3 +1,38 @@
+2015-04-27  Daniel Bates  <[email protected]>
+
+        Form control may be associated with the wrong HTML Form element after form id change
+        https://bugs.webkit.org/show_bug.cgi?id=133456
+        <rdar://problem/17095055>
+
+        Reviewed by Andy Estes.
+
+        Fixes an issue where a form control may be associated with the wrong HTML Form element
+        after the id of the HTML Form element associated with the form control is changed when
+        there is more than one HTML Form element with the same id in the document. Specifically,
+        a form control that has an HTML form attribute value X will always be associated with
+        some HTML Form element f where f.id = X regardless of whether f.id is subsequently
+        changed.
+
+        Tests: fast/forms/change-form-id-to-be-unique-then-submit-form.html
+               fast/forms/change-form-id-to-be-unique.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::attributeChanged): Notify observers when the id of an element changed.
+        (WebCore::Element::updateId): Added parameter NotifyObservers (defaults to NotifyObservers::Yes),
+        as to whether we should notify observers of the id change.
+        (WebCore::Element::updateIdForTreeScope): Ditto.
+        (WebCore::Element::willModifyAttribute): Do not notify observers of the id change immediately. As
+        indicated by the name of this method, we plan to modify the DOM attribute id of the element, but
+        we have not actually modified it when this method is called. Instead we will notify observers
+        in Element::attributeChanged(), which is called after the DOM attribute id is modified.
+        (WebCore::Element::cloneAttributesFromElement): Ditto.
+        * dom/Element.h: Defined enum class NotifyObservers.
+        * dom/TreeScope.cpp:
+        (WebCore::TreeScope::addElementById): Added boolean parameter notifyObservers (defaults to true)
+        as to whether we should dispatch a notification to all observers.
+        (WebCore::TreeScope::removeElementById): Ditto.
+        * dom/TreeScope.h:
+
 2015-04-27  Yoav Weiss  <[email protected]>
 
         Fix viewport units in Media Queries

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/Element.cpp (184102 => 184103)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/Element.cpp	2015-05-11 15:40:47 UTC (rev 184102)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/Element.cpp	2015-05-11 15:47:11 UTC (rev 184103)
@@ -55,6 +55,7 @@
 #include "HTMLSelectElement.h"
 #include "HTMLTableRowsCollection.h"
 #include "HTMLTemplateElement.h"
+#include "IdTargetObserverRegistry.h"
 #include "InsertionPoint.h"
 #include "KeyboardEvent.h"
 #include "MutationObserverInterestGroup.h"
@@ -1085,6 +1086,11 @@
     bool shouldInvalidateStyle = false;
 
     if (name == HTMLNames::idAttr) {
+        if (!oldValue.isEmpty())
+            treeScope().idTargetObserverRegistry().notifyObservers(*oldValue.impl());
+        if (!newValue.isEmpty())
+            treeScope().idTargetObserverRegistry().notifyObservers(*newValue.impl());
+
         AtomicString oldId = elementData()->idForStyleResolution();
         AtomicString newId = makeIdForStyleResolution(newValue, document().inQuirksMode());
         if (newId != oldId) {
@@ -2800,7 +2806,7 @@
     }
 }
 
-inline void Element::updateId(const AtomicString& oldId, const AtomicString& newId)
+inline void Element::updateId(const AtomicString& oldId, const AtomicString& newId, NotifyObservers notifyObservers)
 {
     if (!isInTreeScope())
         return;
@@ -2808,7 +2814,7 @@
     if (oldId == newId)
         return;
 
-    updateIdForTreeScope(treeScope(), oldId, newId);
+    updateIdForTreeScope(treeScope(), oldId, newId, notifyObservers);
 
     if (!inDocument())
         return;
@@ -2817,15 +2823,15 @@
     updateIdForDocument(downcast<HTMLDocument>(document()), oldId, newId, UpdateHTMLDocumentNamedItemMapsOnlyIfDiffersFromNameAttribute);
 }
 
-void Element::updateIdForTreeScope(TreeScope& scope, const AtomicString& oldId, const AtomicString& newId)
+void Element::updateIdForTreeScope(TreeScope& scope, const AtomicString& oldId, const AtomicString& newId, NotifyObservers notifyObservers)
 {
     ASSERT(isInTreeScope());
     ASSERT(oldId != newId);
 
     if (!oldId.isEmpty())
-        scope.removeElementById(*oldId.impl(), *this);
+        scope.removeElementById(*oldId.impl(), *this, notifyObservers == NotifyObservers::Yes);
     if (!newId.isEmpty())
-        scope.addElementById(*newId.impl(), *this);
+        scope.addElementById(*newId.impl(), *this, notifyObservers == NotifyObservers::Yes);
 }
 
 void Element::updateIdForDocument(HTMLDocument& document, const AtomicString& oldId, const AtomicString& newId, HTMLDocumentNamedItemMapsUpdatingCondition condition)
@@ -2869,7 +2875,7 @@
 void Element::willModifyAttribute(const QualifiedName& name, const AtomicString& oldValue, const AtomicString& newValue)
 {
     if (name == HTMLNames::idAttr)
-        updateId(oldValue, newValue);
+        updateId(oldValue, newValue, NotifyObservers::No); // Will notify observers after the attribute is actually changed.
     else if (name == HTMLNames::nameAttr)
         updateName(oldValue, newValue);
     else if (name == HTMLNames::forAttr && hasTagName(labelTag)) {
@@ -3098,7 +3104,7 @@
     const AtomicString& newID = other.getIdAttribute();
 
     if (!oldID.isNull() || !newID.isNull())
-        updateId(oldID, newID);
+        updateId(oldID, newID, NotifyObservers::No); // Will notify observers after the attribute is actually changed.
 
     const AtomicString& oldName = getNameAttribute();
     const AtomicString& newName = other.getNameAttribute();

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/Element.h (184102 => 184103)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/Element.h	2015-05-11 15:40:47 UTC (rev 184102)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/Element.h	2015-05-11 15:47:11 UTC (rev 184103)
@@ -607,8 +607,11 @@
     void updateName(const AtomicString& oldName, const AtomicString& newName);
     void updateNameForTreeScope(TreeScope&, const AtomicString& oldName, const AtomicString& newName);
     void updateNameForDocument(HTMLDocument&, const AtomicString& oldName, const AtomicString& newName);
-    void updateId(const AtomicString& oldId, const AtomicString& newId);
-    void updateIdForTreeScope(TreeScope&, const AtomicString& oldId, const AtomicString& newId);
+
+    enum class NotifyObservers { No, Yes };
+    void updateId(const AtomicString& oldId, const AtomicString& newId, NotifyObservers = NotifyObservers::Yes);
+    void updateIdForTreeScope(TreeScope&, const AtomicString& oldId, const AtomicString& newId, NotifyObservers = NotifyObservers::Yes);
+
     enum HTMLDocumentNamedItemMapsUpdatingCondition { AlwaysUpdateHTMLDocumentNamedItemMaps, UpdateHTMLDocumentNamedItemMapsOnlyIfDiffersFromNameAttribute };
     void updateIdForDocument(HTMLDocument&, const AtomicString& oldId, const AtomicString& newId, HTMLDocumentNamedItemMapsUpdatingCondition);
     void updateLabel(TreeScope&, const AtomicString& oldForAttributeValue, const AtomicString& newForAttributeValue);

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/TreeScope.cpp (184102 => 184103)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/TreeScope.cpp	2015-05-11 15:40:47 UTC (rev 184102)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/TreeScope.cpp	2015-05-11 15:47:11 UTC (rev 184103)
@@ -127,20 +127,22 @@
     return m_elementsById->getAllElementsById(*elementId.impl(), *this);
 }
 
-void TreeScope::addElementById(const AtomicStringImpl& elementId, Element& element)
+void TreeScope::addElementById(const AtomicStringImpl& elementId, Element& element, bool notifyObservers)
 {
     if (!m_elementsById)
         m_elementsById = std::make_unique<DocumentOrderedMap>();
     m_elementsById->add(elementId, element, *this);
-    m_idTargetObserverRegistry->notifyObservers(elementId);
+    if (notifyObservers)
+        m_idTargetObserverRegistry->notifyObservers(elementId);
 }
 
-void TreeScope::removeElementById(const AtomicStringImpl& elementId, Element& element)
+void TreeScope::removeElementById(const AtomicStringImpl& elementId, Element& element, bool notifyObservers)
 {
     if (!m_elementsById)
         return;
     m_elementsById->remove(elementId, element);
-    m_idTargetObserverRegistry->notifyObservers(elementId);
+    if (notifyObservers)
+        m_idTargetObserverRegistry->notifyObservers(elementId);
 }
 
 Element* TreeScope::getElementByName(const AtomicString& name) const

Modified: releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/TreeScope.h (184102 => 184103)


--- releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/TreeScope.h	2015-05-11 15:40:47 UTC (rev 184102)
+++ releases/WebKitGTK/webkit-2.8/Source/WebCore/dom/TreeScope.h	2015-05-11 15:47:11 UTC (rev 184103)
@@ -59,8 +59,8 @@
     const Vector<Element*>* getAllElementsById(const AtomicString&) const;
     bool hasElementWithId(const AtomicStringImpl&) const;
     bool containsMultipleElementsWithId(const AtomicString& id) const;
-    void addElementById(const AtomicStringImpl& elementId, Element&);
-    void removeElementById(const AtomicStringImpl& elementId, Element&);
+    void addElementById(const AtomicStringImpl& elementId, Element&, bool notifyObservers = true);
+    void removeElementById(const AtomicStringImpl& elementId, Element&, bool notifyObservers = true);
 
     Element* getElementByName(const AtomicString&) const;
     bool hasElementWithName(const AtomicStringImpl&) const;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to