Changeset 245746 in webkit

Timestamp:

May 24, 2019 9:33:57 AM (5 years ago)

Author:

rniwa@webkit.org

Message:

Asssertion failure in dispatchSubtreeModifiedEvent due to TextFieldInputType updating UA shadow tree inside Element::removedFromAncestor
​https://bugs.webkit.org/show_bug.cgi?id=198216

Reviewed by Brent Fulgham.

Source/WebCore:

The bug was caused by ListAttributeTargetObserver::idTargetChanged() updating the shadow tree of an input element
within Element::removedFromAncestor via TextFieldInputType::createDataListDropdownIndicator(). Fixed it by
supressing the assertions with ScriptDisallowedScope::EventAllowedScope since it's always safe to update
UA shadow trees of input elements as it's not exposed to author scripts.

Avoiding the creation of dropdown indicator in this particular scenario is a lot more involved and it's not
particularly correct because there could be another datalist element which matches the ID specified in list
content attribute after the removal of the old datalist element.

Test: fast/forms/datalist/datalist-removal-assertion.html

• html/TextFieldInputType.cpp:

(WebCore::TextFieldInputType::createDataListDropdownIndicator):
(WebCore::TextFieldInputType::createContainer):

• html/shadow/DataListButtonElement.cpp:

(WebCore::DataListButtonElement::DataListButtonElement):

LayoutTests:

Added a regression test.

• fast/forms/datalist/datalist-removal-assertion-expected.txt: Added.
• fast/forms/datalist/datalist-removal-assertion.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/forms/datalist/datalist-removal-assertion-expected.txt (added)
• LayoutTests/fast/forms/datalist/datalist-removal-assertion.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/TextFieldInputType.cpp (modified) (3 diffs)
• Source/WebCore/html/shadow/DataListButtonElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-05-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Asssertion failure in dispatchSubtreeModifiedEvent due to TextFieldInputType updating UA shadow tree inside Element::removedFromAncestor
+        https://bugs.webkit.org/show_bug.cgi?id=198216
+
+        Reviewed by Brent Fulgham.
+
+        Added a regression test.
+
+        * fast/forms/datalist/datalist-removal-assertion-expected.txt: Added.
+        * fast/forms/datalist/datalist-removal-assertion.html: Added.
+
 2019-05-23  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-05-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Asssertion failure in dispatchSubtreeModifiedEvent due to TextFieldInputType updating UA shadow tree inside Element::removedFromAncestor
+        https://bugs.webkit.org/show_bug.cgi?id=198216
+
+        Reviewed by Brent Fulgham.
+
+        The bug was caused by ListAttributeTargetObserver::idTargetChanged() updating the shadow tree of an input element
+        within Element::removedFromAncestor via TextFieldInputType::createDataListDropdownIndicator(). Fixed it by
+        supressing the assertions with ScriptDisallowedScope::EventAllowedScope since it's always safe to update
+        UA shadow trees of input elements as it's not exposed to author scripts.
+
+        Avoiding the creation of dropdown indicator in this particular scenario is a lot more involved and it's not
+        particularly correct because there could be another datalist element which matches the ID specified in list
+        content attribute after the removal of the old datalist element.
+
+        Test: fast/forms/datalist/datalist-removal-assertion.html
+
+        * html/TextFieldInputType.cpp:
+        (WebCore::TextFieldInputType::createDataListDropdownIndicator):
+        (WebCore::TextFieldInputType::createContainer):
+        * html/shadow/DataListButtonElement.cpp:
+        (WebCore::DataListButtonElement::DataListButtonElement):
+
 2019-05-24  Saam barati  <sbarati@apple.com>
 

trunk/Source/WebCore/html/TextFieldInputType.cpp

@@ -53 +53 @@
 #include "RenderTheme.h"
 #include "RuntimeEnabledFeatures.h"
+#include "ScriptDisallowedScope.h"
 #include "ShadowRoot.h"
 #include "TextControlInnerElements.h"
@@ -453 +454 @@
     if (!m_container)
         createContainer();
+
+    ScriptDisallowedScope::EventAllowedScope allowedScope(*m_container);
     m_dataListDropdownIndicator = DataListButtonElement::create(element()->document(), *this);
+    m_container->appendChild(*m_dataListDropdownIndicator);
+    m_dataListDropdownIndicator->setPseudo(AtomicString("-webkit-list-button", AtomicString::ConstructFromLiteral));
     m_dataListDropdownIndicator->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone, true);
-    m_container->appendChild(*m_dataListDropdownIndicator);
+
 }
 #endif
@@ -774 +779 @@
     ASSERT(element());
 
+    ScriptDisallowedScope::EventAllowedScope allowedScope(*element()->userAgentShadowRoot());
+
     m_container = TextControlInnerContainer::create(element()->document());
+    element()->userAgentShadowRoot()->appendChild(*m_container);
     m_container->setPseudo(AtomicString("-webkit-textfield-decoration-container", AtomicString::ConstructFromLiteral));
 
     m_innerBlock = TextControlInnerElement::create(element()->document());
+    m_container->appendChild(*m_innerBlock);
     m_innerBlock->appendChild(*m_innerText);
-    m_container->appendChild(*m_innerBlock);
-
-    element()->userAgentShadowRoot()->appendChild(*m_container);
 }
 

trunk/Source/WebCore/html/shadow/DataListButtonElement.cpp

@@ -50 +50 @@
     , m_owner(owner)
 {
-    setPseudo(AtomicString("-webkit-list-button", AtomicString::ConstructFromLiteral));
 }