Changeset 239786 in webkit

Timestamp:

Jan 9, 2019, 2:25:41 PM (8 years ago)

Author:

Alan Bujtas

Message:

[Datalist] Crash when input with datalist is dynamically added.
​https://bugs.webkit.org/show_bug.cgi?id=193012
<rdar://problem/45923457>

Reviewed by Brent Fulgham.

Source/WebCore:

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:

LayoutTests:

• fast/forms/datalist/datalist-crash-when-dynamic-expected.txt: Added.
• fast/forms/datalist/datalist-crash-when-dynamic.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/forms/datalist/datalist-crash-when-dynamic-expected.txt (added)
• LayoutTests/fast/forms/datalist/datalist-crash-when-dynamic.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/TextFieldInputType.cpp (modified) (4 diffs)
• Source/WebCore/html/TextFieldInputType.h (modified) (1 diff)
• Source/WebCore/rendering/RenderThemeMac.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-01-09  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 Brent Fulgham.
+
+        * fast/forms/datalist/datalist-crash-when-dynamic-expected.txt: Added.
+        * fast/forms/datalist/datalist-crash-when-dynamic.html: Added.
+
 2019-01-09  Justin Fan  <justin_fan@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-01-09  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 Brent Fulgham.
+
+        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:
+
 2019-01-09  Justin Fan  <justin_fan@apple.com>
 

trunk/Source/WebCore/html/TextFieldInputType.cpp

@@ -300 +300 @@
 bool TextFieldInputType::needsContainer() const
 {
-#if ENABLE(DATALIST_ELEMENT)
-    return element()->hasAttributeWithoutSynchronization(listAttr);
-#endif
     return false;
 }
@@ -359 +356 @@
         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
 }
 
@@ -456 +446 @@
     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, CSSValueNone, true);
+    m_container->appendChild(*m_dataListDropdownIndicator);
+}
+#endif
 
 // FIXME: The name of this function doesn't make clear the two jobs it does:
@@ -833 +835 @@
 
     if (!m_dataListDropdownIndicator)
-        return;
+        createDataListDropdownIndicator();
 
 #if !PLATFORM(IOS_FAMILY)

trunk/Source/WebCore/html/TextFieldInputType.h

@@ -125 +125 @@
 
 #if ENABLE(DATALIST_ELEMENT)
+    void createDataListDropdownIndicator();
     bool isPresentingAttachedView() const final;
     void listAttributeTargetChanged() final;

trunk/Source/WebCore/rendering/RenderThemeMac.mm

@@ -1146 +1146 @@
 
     [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)];