Attachment 370565 Details for Bug 198216 – Fixes the bug

[patch] Fixes the bug

bug-198216-20190523231345.patch (text/plain), 6.41 KB, created by Ryosuke Niwa on 2019-05-23 23:13:46 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2019-05-23 23:13:46 PDT

Size: 6.41 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 245743)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2019-05-23  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 NOBODY (OOPS!).
>+
>+        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-23  Simon Fraser  <simon.fraser@apple.com>
> 
>         With async overflow scrolling, programmatic scroll to a negative offset fails to clamp the scroll offset
>Index: Source/WebCore/html/TextFieldInputType.cpp
>===================================================================
>--- Source/WebCore/html/TextFieldInputType.cpp	(revision 245721)
>+++ Source/WebCore/html/TextFieldInputType.cpp	(working copy)
>@@ -52,6 +52,7 @@
> #include "RenderTextControlSingleLine.h"
> #include "RenderTheme.h"
> #include "RuntimeEnabledFeatures.h"
>+#include "ScriptDisallowedScope.h"
> #include "ShadowRoot.h"
> #include "TextControlInnerElements.h"
> #include "TextEvent.h"
>@@ -452,9 +453,13 @@ void TextFieldInputType::createDataListD
>     ASSERT(!m_dataListDropdownIndicator);
>     if (!m_container)
>         createContainer();
>+
>+    ScriptDisallowedScope::EventAllowedScope allowedScope(*m_container);
>     m_dataListDropdownIndicator = DataListButtonElement::create(element()->document(), *this);
>-    m_dataListDropdownIndicator->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone, true);
>     m_container->appendChild(*m_dataListDropdownIndicator);
>+    m_dataListDropdownIndicator->setPseudo(AtomicString("-webkit-list-button", AtomicString::ConstructFromLiteral));
>+    m_dataListDropdownIndicator->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone, true);
>+
> }
> #endif
> 
>@@ -773,14 +778,15 @@ void TextFieldInputType::createContainer
>     ASSERT(!m_container);
>     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_innerBlock->appendChild(*m_innerText);
>     m_container->appendChild(*m_innerBlock);
>-
>-    element()->userAgentShadowRoot()->appendChild(*m_container);
>+    m_innerBlock->appendChild(*m_innerText);
> }
> 
> void TextFieldInputType::createAutoFillButton(AutoFillButtonType autoFillButtonType)
>Index: Source/WebCore/html/shadow/DataListButtonElement.cpp
>===================================================================
>--- Source/WebCore/html/shadow/DataListButtonElement.cpp	(revision 245721)
>+++ Source/WebCore/html/shadow/DataListButtonElement.cpp	(working copy)
>@@ -49,7 +49,6 @@ DataListButtonElement::DataListButtonEle
>     : HTMLDivElement(divTag, document)
>     , m_owner(owner)
> {
>-    setPseudo(AtomicString("-webkit-list-button", AtomicString::ConstructFromLiteral));
> }
> 
> DataListButtonElement::~DataListButtonElement() { }
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 245721)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2019-05-23  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 NOBODY (OOPS!).
>+
>+        Added a regression test.
>+
>+        * fast/forms/datalist/datalist-removal-assertion-expected.txt: Added.
>+        * fast/forms/datalist/datalist-removal-assertion.html: Added.
>+
> 2019-05-23  Saam barati  <sbarati@apple.com>
> 
>         [WHLSL] Property resolver needs to recurse to handle the base when simplifying rvalues
>Index: LayoutTests/fast/forms/datalist/datalist-removal-assertion-expected.txt
>===================================================================
>--- LayoutTests/fast/forms/datalist/datalist-removal-assertion-expected.txt	(nonexistent)
>+++ LayoutTests/fast/forms/datalist/datalist-removal-assertion-expected.txt	(working copy)
>@@ -0,0 +1,4 @@
>+This tests removing the datalist element immediately after changing the type of the input element's type.
>+WebKit should not hit a debug assertion.
>+
>+PASS
>Index: LayoutTests/fast/forms/datalist/datalist-removal-assertion.html
>===================================================================
>--- LayoutTests/fast/forms/datalist/datalist-removal-assertion.html	(nonexistent)
>+++ LayoutTests/fast/forms/datalist/datalist-removal-assertion.html	(working copy)
>@@ -0,0 +1,21 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>This tests removing the datalist element immediately after changing the type of the input element's type.<br>
>+WebKit should not hit a debug assertion.</p>
>+<input id="input" list="fruits" type="checkbox">
>+<datalist id="fruits">
>+    <option>Orange</option>
>+    <option>Pear</option>
>+    <option>Apple</option>
>+</datalist>
>+<script>
>+if (window.testRunner)
>+    testRunner.dumpAsText();
>+input.type = 'text';
>+fruits.remove();
>+input.remove();
>+document.write('PASS');
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 198216: 370565