Radio button groups are not scoped by shadow boundaries
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Oct 2019 07:34:32 +0000 (07:34 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Oct 2019 07:34:32 +0000 (07:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199568

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

Rebaselined a test now that it passes.

* web-platform-tests/shadow-dom/input-type-radio-expected.txt:

Source/WebCore:

Fixed the bug that radio button groups are not scoped to each shadow tree by moving
RadioButtonGroups from FormController, which is a per-document object, to TreeScope.

Test: imported/w3c/web-platform-tests/shadow-dom/input-type-radio.html

* dom/RadioButtonGroups.h:
(WebCore::RadioButtonGroups): Made this bmalloc'ed now that it's allocated standalone.
* dom/TreeScope.cpp:
(WebCore::TreeScope::radioButtonGroups): Added.
* dom/TreeScope.h:
* html/FormController.h:
(WebCore::FormController::radioButtonGroups): Deleted.
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::~HTMLInputElement):
(WebCore::HTMLInputElement::removedFromAncestor): Update the radio button group here.
(WebCore::HTMLInputElement::didMoveToNewDocument): Removed the code to update radio
button group here since it's done in removedFromAncestor now. Note that insertion case
is alrady taken care of by HTMLInputElement::didFinishInsertingNode.
(WebCore::HTMLInputElement::radioButtonGroups const): Ditto.

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

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/shadow-dom/input-type-radio-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/RadioButtonGroups.h
Source/WebCore/dom/TreeScope.cpp
Source/WebCore/dom/TreeScope.h
Source/WebCore/html/FormController.h
Source/WebCore/html/HTMLInputElement.cpp

index 3dd487a..1c1cc64 100644 (file)
@@ -1,3 +1,14 @@
+2019-10-04  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Radio button groups are not scoped by shadow boundaries
+        https://bugs.webkit.org/show_bug.cgi?id=199568
+
+        Reviewed by Antti Koivisto.
+
+        Rebaselined a test now that it passes.
+
+        * web-platform-tests/shadow-dom/input-type-radio-expected.txt:
+
 2019-10-03  Antti Koivisto  <antti@apple.com>
 
         [CSS Shadow Parts] Correct interaction with other pseudo elements
index 9c3c5bd..dc1a81b 100644 (file)
@@ -1,4 +1,4 @@
  
 
-FAIL input type=radio elements should form a group inside shadow DOM. assert_true: expected true got false
+PASS input type=radio elements should form a group inside shadow DOM. 
 
index 5753bf3..c4dff65 100644 (file)
@@ -1,3 +1,30 @@
+2019-10-04  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Radio button groups are not scoped by shadow boundaries
+        https://bugs.webkit.org/show_bug.cgi?id=199568
+
+        Reviewed by Antti Koivisto.
+
+        Fixed the bug that radio button groups are not scoped to each shadow tree by moving
+        RadioButtonGroups from FormController, which is a per-document object, to TreeScope.
+
+        Test: imported/w3c/web-platform-tests/shadow-dom/input-type-radio.html
+
+        * dom/RadioButtonGroups.h:
+        (WebCore::RadioButtonGroups): Made this bmalloc'ed now that it's allocated standalone.
+        * dom/TreeScope.cpp:
+        (WebCore::TreeScope::radioButtonGroups): Added.
+        * dom/TreeScope.h:
+        * html/FormController.h:
+        (WebCore::FormController::radioButtonGroups): Deleted.
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::~HTMLInputElement):
+        (WebCore::HTMLInputElement::removedFromAncestor): Update the radio button group here.
+        (WebCore::HTMLInputElement::didMoveToNewDocument): Removed the code to update radio
+        button group here since it's done in removedFromAncestor now. Note that insertion case
+        is alrady taken care of by HTMLInputElement::didFinishInsertingNode.
+        (WebCore::HTMLInputElement::radioButtonGroups const): Ditto.
+
 2019-10-03  Antti Koivisto  <antti@apple.com>
 
         [CSS Shadow Parts] Correct interaction with other pseudo elements
index 26f1f71..52b336f 100644 (file)
@@ -31,6 +31,7 @@ class HTMLInputElement;
 class RadioButtonGroup;
 
 class RadioButtonGroups {
+    WTF_MAKE_FAST_ALLOCATED;
 public:
     RadioButtonGroups();
     ~RadioButtonGroups();
index 531acdf..72097f4 100644 (file)
@@ -44,6 +44,7 @@
 #include "Page.h"
 #include "PointerLockController.h"
 #include "PseudoElement.h"
+#include "RadioButtonGroups.h"
 #include "RenderView.h"
 #include "RuntimeEnabledFeatures.h"
 #include "Settings.h"
@@ -53,7 +54,7 @@
 namespace WebCore {
 
 struct SameSizeAsTreeScope {
-    void* pointers[9];
+    void* pointers[10];
 };
 
 COMPILE_ASSERT(sizeof(TreeScope) == sizeof(SameSizeAsTreeScope), treescope_should_stay_small);
@@ -539,4 +540,11 @@ TreeScope* commonTreeScope(Node* nodeA, Node* nodeB)
     return treeScopesA[indexA] == treeScopesB[indexB] ? treeScopesA[indexA] : nullptr;
 }
 
+RadioButtonGroups& TreeScope::radioButtonGroups()
+{
+    if (!m_radioButtonGroups)
+        m_radioButtonGroups = makeUnique<RadioButtonGroups>();
+    return *m_radioButtonGroups;
+}
+
 } // namespace WebCore
index 2f257f0..5d6e868 100644 (file)
@@ -44,6 +44,7 @@ class HTMLMapElement;
 class LayoutPoint;
 class IdTargetObserverRegistry;
 class Node;
+class RadioButtonGroups;
 class ShadowRoot;
 
 class TreeScope {
@@ -109,6 +110,8 @@ public:
 
     IdTargetObserverRegistry& idTargetObserverRegistry() const { return *m_idTargetObserverRegistry.get(); }
 
+    RadioButtonGroups& radioButtonGroups();
+
 protected:
     TreeScope(ShadowRoot&, Document&);
     explicit TreeScope(Document&);
@@ -135,6 +138,8 @@ private:
     std::unique_ptr<TreeScopeOrderedMap> m_labelsByForAttribute;
 
     std::unique_ptr<IdTargetObserverRegistry> m_idTargetObserverRegistry;
+    
+    std::unique_ptr<RadioButtonGroups> m_radioButtonGroups;
 };
 
 inline bool TreeScope::hasElementWithId(const AtomStringImpl& id) const
index 9d5268e..a1352f5 100644 (file)
@@ -21,8 +21,8 @@
 
 #pragma once
 
-#include "RadioButtonGroups.h"
 #include <wtf/Forward.h>
+#include <wtf/HashMap.h>
 #include <wtf/ListHashSet.h>
 #include <wtf/Vector.h>
 #include <wtf/text/WTFString.h>
@@ -42,8 +42,6 @@ public:
     FormController();
     ~FormController();
 
-    RadioButtonGroups& radioButtonGroups() { return m_radioButtonGroups; }
-
     void registerFormElementWithState(HTMLFormControlElementWithState&);
     void unregisterFormElementWithState(HTMLFormControlElementWithState&);
 
@@ -67,7 +65,6 @@ private:
     FormControlState takeStateForFormElement(const HTMLFormControlElementWithState&);
     static void formStatesFromStateVector(const Vector<String>&, SavedFormStateMap&);
 
-    RadioButtonGroups m_radioButtonGroups;
     FormElementListHashSet m_formElementsWithState;
     SavedFormStateMap m_savedFormStateMap;
     std::unique_ptr<FormKeyGenerator> m_formKeyGenerator;
index 29c0726..fb59207 100644 (file)
@@ -173,7 +173,7 @@ HTMLInputElement::~HTMLInputElement()
     // actually adds the button to the document groups in the latter case.
     // That is inelegant, but harmless since we remove it here.
     if (isRadioButton())
-        document().formController().radioButtonGroups().removeButton(*this);
+        treeScope().radioButtonGroups().removeButton(*this);
 
 #if ENABLE(TOUCH_EVENTS)
     if (m_hasTouchEventHandler)
@@ -1556,6 +1556,8 @@ void HTMLInputElement::didFinishInsertingNode()
 
 void HTMLInputElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
 {
+    if (removalType.treeScopeChanged && isRadioButton())
+        oldParentOfRemovedTree.treeScope().radioButtonGroups().removeButton(*this);
     if (removalType.disconnectedFromDocument && !form())
         removeFromRadioButtonGroup();
     HTMLTextFormControlElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
@@ -1576,11 +1578,6 @@ void HTMLInputElement::didMoveToNewDocument(Document& oldDocument, Document& new
         newDocument.registerForDocumentSuspensionCallbacks(*this);
     }
 
-    // We call this even for radio buttons in forms; it's harmless because the
-    // removeButton function is written to be safe for buttons not in any group.
-    if (isRadioButton())
-        oldDocument.formController().radioButtonGroups().removeButton(*this);
-
 #if ENABLE(TOUCH_EVENTS)
     if (m_hasTouchEventHandler) {
         oldDocument.didRemoveEventTargetNode(*this);
@@ -1922,8 +1919,8 @@ RadioButtonGroups* HTMLInputElement::radioButtonGroups() const
         return nullptr;
     if (auto* formElement = form())
         return &formElement->radioButtonGroups();
-    if (isConnected())
-        return &document().formController().radioButtonGroups();
+    if (isInTreeScope())
+        return &treeScope().radioButtonGroups();
     return nullptr;
 }