Title: [250708] trunk
Revision
250708
Author
rn...@webkit.org
Date
2019-10-04 00:34:32 -0700 (Fri, 04 Oct 2019)

Log Message

Radio button groups are not scoped by shadow boundaries
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.

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (250707 => 250708)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2019-10-04 06:15:39 UTC (rev 250707)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2019-10-04 07:34:32 UTC (rev 250708)
@@ -1,3 +1,14 @@
+2019-10-04  Ryosuke Niwa  <rn...@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  <an...@apple.com>
 
         [CSS Shadow Parts] Correct interaction with other pseudo elements

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/input-type-radio-expected.txt (250707 => 250708)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/input-type-radio-expected.txt	2019-10-04 06:15:39 UTC (rev 250707)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/shadow-dom/input-type-radio-expected.txt	2019-10-04 07:34:32 UTC (rev 250708)
@@ -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. 
 

Modified: trunk/Source/WebCore/ChangeLog (250707 => 250708)


--- trunk/Source/WebCore/ChangeLog	2019-10-04 06:15:39 UTC (rev 250707)
+++ trunk/Source/WebCore/ChangeLog	2019-10-04 07:34:32 UTC (rev 250708)
@@ -1,3 +1,30 @@
+2019-10-04  Ryosuke Niwa  <rn...@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  <an...@apple.com>
 
         [CSS Shadow Parts] Correct interaction with other pseudo elements

Modified: trunk/Source/WebCore/dom/RadioButtonGroups.h (250707 => 250708)


--- trunk/Source/WebCore/dom/RadioButtonGroups.h	2019-10-04 06:15:39 UTC (rev 250707)
+++ trunk/Source/WebCore/dom/RadioButtonGroups.h	2019-10-04 07:34:32 UTC (rev 250708)
@@ -31,6 +31,7 @@
 class RadioButtonGroup;
 
 class RadioButtonGroups {
+    WTF_MAKE_FAST_ALLOCATED;
 public:
     RadioButtonGroups();
     ~RadioButtonGroups();

Modified: trunk/Source/WebCore/dom/TreeScope.cpp (250707 => 250708)


--- trunk/Source/WebCore/dom/TreeScope.cpp	2019-10-04 06:15:39 UTC (rev 250707)
+++ trunk/Source/WebCore/dom/TreeScope.cpp	2019-10-04 07:34:32 UTC (rev 250708)
@@ -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 @@
     return treeScopesA[indexA] == treeScopesB[indexB] ? treeScopesA[indexA] : nullptr;
 }
 
+RadioButtonGroups& TreeScope::radioButtonGroups()
+{
+    if (!m_radioButtonGroups)
+        m_radioButtonGroups = makeUnique<RadioButtonGroups>();
+    return *m_radioButtonGroups;
+}
+
 } // namespace WebCore

Modified: trunk/Source/WebCore/dom/TreeScope.h (250707 => 250708)


--- trunk/Source/WebCore/dom/TreeScope.h	2019-10-04 06:15:39 UTC (rev 250707)
+++ trunk/Source/WebCore/dom/TreeScope.h	2019-10-04 07:34:32 UTC (rev 250708)
@@ -44,6 +44,7 @@
 class LayoutPoint;
 class IdTargetObserverRegistry;
 class Node;
+class RadioButtonGroups;
 class ShadowRoot;
 
 class TreeScope {
@@ -109,6 +110,8 @@
 
     IdTargetObserverRegistry& idTargetObserverRegistry() const { return *m_idTargetObserverRegistry.get(); }
 
+    RadioButtonGroups& radioButtonGroups();
+
 protected:
     TreeScope(ShadowRoot&, Document&);
     explicit TreeScope(Document&);
@@ -135,6 +138,8 @@
     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

Modified: trunk/Source/WebCore/html/FormController.h (250707 => 250708)


--- trunk/Source/WebCore/html/FormController.h	2019-10-04 06:15:39 UTC (rev 250707)
+++ trunk/Source/WebCore/html/FormController.h	2019-10-04 07:34:32 UTC (rev 250708)
@@ -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 @@
     FormController();
     ~FormController();
 
-    RadioButtonGroups& radioButtonGroups() { return m_radioButtonGroups; }
-
     void registerFormElementWithState(HTMLFormControlElementWithState&);
     void unregisterFormElementWithState(HTMLFormControlElementWithState&);
 
@@ -67,7 +65,6 @@
     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;

Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (250707 => 250708)


--- trunk/Source/WebCore/html/HTMLInputElement.cpp	2019-10-04 06:15:39 UTC (rev 250707)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp	2019-10-04 07:34:32 UTC (rev 250708)
@@ -173,7 +173,7 @@
     // 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::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 @@
         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 @@
         return nullptr;
     if (auto* formElement = form())
         return &formElement->radioButtonGroups();
-    if (isConnected())
-        return &document().formController().radioButtonGroups();
+    if (isInTreeScope())
+        return &treeScope().radioButtonGroups();
     return nullptr;
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to