Attachment 358070 Details for Bug 193012 – Patch

[patch] Patch

bug-193012-20181225094726.patch (text/plain), 7.16 KB, created by zalan on 2018-12-25 09:47:27 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-12-25 09:47:27 PST

Size: 7.16 KB

patch

obsolete

>Subversion Revision: 239551
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index cc1b98ade03d35cac7252acf16c46af23d009b4f..7bdde46a94bc69e3b05edb015c345c95040c30eb 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-12-25  Zalan Bujtas  <zalan@apple.com>
>+
>+        [Datalist] Crash when input with datalist is dynamically added.
>+        https://bugs.webkit.org/show_bug.cgi?id=193012
>+        <rdar://problem/45923457>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        In certain cases (cloning, setAttribute), it's too early to check for the list attribute in createShadowSubtree
>+        to see whether the input needs datalist related items. The list attribute is simply not set yet.
>+        This patch only addresses the obvious crash. m_dataListDropdownIndicator clearly lacks proper lifecycle management (see webkit.org/b/193032). 
>+
>+        Test: fast/forms/datalist/datalist-crash-when-dynamic.html
>+
>+        * html/TextFieldInputType.cpp:
>+        (WebCore::TextFieldInputType::createShadowSubtree):
>+        (WebCore::TextFieldInputType::attributeChanged):
>+        (WebCore::TextFieldInputType::createDataListDropdownIndicator):
>+        * html/TextFieldInputType.h:
>+
> 2018-12-24  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         Remove "using namespace std;"
>diff --git a/Source/WebCore/html/TextFieldInputType.cpp b/Source/WebCore/html/TextFieldInputType.cpp
>index a62971353f9fc3bbc71611da74366697175ffa89..cdf5f74d64aaf19faaf013775304676f537bbbb5 100644
>--- a/Source/WebCore/html/TextFieldInputType.cpp
>+++ b/Source/WebCore/html/TextFieldInputType.cpp
>@@ -299,9 +299,6 @@ RenderPtr<RenderElement> TextFieldInputType::createInputRenderer(RenderStyle&& s
> 
> bool TextFieldInputType::needsContainer() const
> {
>-#if ENABLE(DATALIST_ELEMENT)
>-    return element()->hasAttributeWithoutSynchronization(listAttr);
>-#endif
>     return false;
> }
> 
>@@ -358,14 +355,7 @@ void TextFieldInputType::createShadowSubtree()
> 
>         m_container->appendChild(*m_capsLockIndicator);
>     }
>-
>     updateAutoFillButton();
>-
>-#if ENABLE(DATALIST_ELEMENT)
>-    m_dataListDropdownIndicator = DataListButtonElement::create(element()->document(), *this);
>-    m_dataListDropdownIndicator->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone, true);
>-    m_container->appendChild(*m_dataListDropdownIndicator);
>-#endif
> }
> 
> HTMLElement* TextFieldInputType::containerElement() const
>@@ -456,6 +446,18 @@ bool TextFieldInputType::shouldUseInputMethod() const
>     return true;
> }
> 
>+#if ENABLE(DATALIST_ELEMENT)
>+void TextFieldInputType::createDataListDropdownIndicator()
>+{
>+    ASSERT(!m_dataListDropdownIndicator);
>+    if (!m_container)
>+        createContainer();
>+    m_dataListDropdownIndicator = DataListButtonElement::create(element()->document(), *this);
>+    m_dataListDropdownIndicator->setInlineStyleProperty(CSSPropertyDisplay, CSSValueBlock, true);
>+    m_container->appendChild(*m_dataListDropdownIndicator);
>+}
>+#endif
>+
> // FIXME: The name of this function doesn't make clear the two jobs it does:
> // 1) Limits the string to a particular number of grapheme clusters.
> // 2) Truncates the string at the first character which is a control character other than tab.
>@@ -832,7 +834,7 @@ void TextFieldInputType::listAttributeTargetChanged()
>     m_cachedSuggestions = std::make_pair(String(), Vector<String>());
> 
>     if (!m_dataListDropdownIndicator)
>-        return;
>+        createDataListDropdownIndicator();
> 
> #if !PLATFORM(IOS_FAMILY)
>     m_dataListDropdownIndicator->setInlineStyleProperty(CSSPropertyDisplay, element()->list() ? CSSValueBlock : CSSValueNone, true);
>diff --git a/Source/WebCore/html/TextFieldInputType.h b/Source/WebCore/html/TextFieldInputType.h
>index 291f8fbc74c9ce1e7a6eabf37263b867831fa1e9..2ffa7b2921aaa6ef1c90e64987b5c893db7c93fa 100644
>--- a/Source/WebCore/html/TextFieldInputType.h
>+++ b/Source/WebCore/html/TextFieldInputType.h
>@@ -124,6 +124,7 @@ private:
>     void createAutoFillButton(AutoFillButtonType);
> 
> #if ENABLE(DATALIST_ELEMENT)
>+    void createDataListDropdownIndicator();
>     bool isPresentingAttachedView() const final;
>     void listAttributeTargetChanged() final;
>     void displaySuggestions(DataListSuggestionActivationType);
>diff --git a/Source/WebCore/rendering/RenderThemeMac.mm b/Source/WebCore/rendering/RenderThemeMac.mm
>index aa8b68b52dc49c76ee9b7de942f0e239a0f62d99..5a0de9951d890c09af32bde40846a224822b26e7 100644
>--- a/Source/WebCore/rendering/RenderThemeMac.mm
>+++ b/Source/WebCore/rendering/RenderThemeMac.mm
>@@ -1145,8 +1145,11 @@ void RenderThemeMac::paintListButtonForInput(const RenderObject& o, GraphicsCont
>         [listButton setUserInterfaceLayoutDirection:NSUserInterfaceLayoutDirectionLeftToRight];
> 
>     [listButton setHighlighted:input.isPresentingAttachedView()];
>-    if (!input.isPresentingAttachedView())
>-        updatePressedState(listButton, *(input.dataListButtonElement()->renderer()));
>+    if (!input.isPresentingAttachedView()) {
>+        ASSERT(input.dataListButtonElement());
>+        if (auto* buttonElement = input.dataListButtonElement())
>+            updatePressedState(listButton, *buttonElement->renderer());
>+    }
> 
>     [listButton drawWithFrame:listButtonFrame inView:documentViewFor(o)];
>     [listButton setControlView:nil];
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 76c38d92500820641145ee49c7c8d94fec3584c1..760979406904f329457462123fced5edd71f910a 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-12-25  Zalan Bujtas  <zalan@apple.com>
>+
>+        [Datalist] Crash when input with datalist is dynamically added.
>+        https://bugs.webkit.org/show_bug.cgi?id=193012
>+        <rdar://problem/45923457>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/forms/datalist/datalist-crash-when-dynamic-expected.txt: Added.
>+        * fast/forms/datalist/datalist-crash-when-dynamic.html: Added.
>+
> 2018-12-23  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         Unreviewed GTK+ gardening. Rebaseline fast/text/zero-font-size.html after r239539.
>diff --git a/LayoutTests/fast/forms/datalist/datalist-crash-when-dynamic-expected.txt b/LayoutTests/fast/forms/datalist/datalist-crash-when-dynamic-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..98528afa0c31f2d37260e0e53168739c12a84fe1
>--- /dev/null
>+++ b/LayoutTests/fast/forms/datalist/datalist-crash-when-dynamic-expected.txt
>@@ -0,0 +1,2 @@
>+Pass if no crash or assert
>+  
>diff --git a/LayoutTests/fast/forms/datalist/datalist-crash-when-dynamic.html b/LayoutTests/fast/forms/datalist/datalist-crash-when-dynamic.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..5cdcea2d2df6f05559cb688ed812d891aa07339f
>--- /dev/null
>+++ b/LayoutTests/fast/forms/datalist/datalist-crash-when-dynamic.html
>@@ -0,0 +1,16 @@
>+<div>Pass if no crash or assert</div>
>+<input id="ice" list="ice-cream-flavors" name="ice-cream-choice">
>+<datalist id="ice-cream-flavors">
>+    <option value="Chocolate">
>+    </option><option value="Coconut">
>+    </option><option value="Mint">
>+	</option>
>+</datalist>
>+
>+<script>
>+if (window.testRunner)
>+    testRunner.dumpAsText();
>+document.body.offsetHeight;
>+ice.setAttribute("list", "ice-cream-flavors");
>+document.body.appendChild(ice.cloneNode());
>+</script>

Actions: View | Formatted Diff | Diff

Attachments on bug 193012: 358028 | 358070 | 358072 | 358189 | 358208