Attachment 395276 Details for Bug 209909 – Add description to test case

[patch] Add description to test case

bug-209909-20200402100452.patch (text/plain), 12.07 KB, created by Wenson Hsieh on 2020-04-02 10:04:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2020-04-02 10:04:53 PDT

Size: 12.07 KB

patch

obsolete

>Subversion Revision: 259375
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 77b3697845ecfcc0d3e42d2981f321a214919fc4..58c22d270b27debc45ad06ce0c1aef023fbc7006 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,65 @@
>+2020-04-02  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        Avoid null deref after inserting a text field with a list attribute
>+        https://bugs.webkit.org/show_bug.cgi?id=209909
>+        <rdar://problem/60742229>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        On macOS, when painting a text field with an associated datalist (i.e. `HTMLInputElement::list()` is non-null),
>+        we assume that the datalist suggestions dropdown button has a renderer (in other words, it does not have a style
>+        of `display: none`).
>+
>+        Existing logic in `TextFieldInputType` is responsible for upholding this invariant -- when the list attribute
>+        changes on an input field (e.g. when we parse the list attribute, or when it is set by JavaScript), we update
>+        the inline display style of `m_dataListDropdownIndicator`, such that it is set to `display: none` only if there
>+        is either no list attribute, or the list attribute is empty, or the list does not refer to a connected datalist
>+        element. However, there is one scenario in which this invariant is violated. Consider the following:
>+
>+        1. An input field is created, and its list attribute is set to "foo". Importantly, it is not connected yet.
>+        2. A datalist element with id "foo" is then created and then added to the document.
>+        3. The input field created in (1) is then added to the document.
>+
>+        In this scenario, `listAttributeTargetChanged()` is invoked after (1), but since it is not connected, it has no
>+        datalist yet, and so `m_dataListDropdownIndicator` will remain non-rendered. When it is later added to the DOM,
>+        nothing attempts to `m_dataListDropdownIndicator` even though its list attribute now refers to a datalist, so
>+        it remains hidden. When we later go to paint the input's datalist dropdown button in
>+        `RenderThemeMac::paintListButtonForInput`, we assume that the dropdown button must be rendered because the input
>+        has a datalist and subsequently crash since `buttonElement->renderer()` remains null.
>+
>+        To fix this, we add logic to update the datalist dropdown button's inline display style when it is connected to
>+        the document with an existing, non-empty list attribute.
>+
>+        Test: fast/forms/datalist/append-input-with-list-attribute.html
>+
>+        * html/HTMLInputElement.cpp:
>+        (WebCore::HTMLInputElement::parseAttribute):
>+        (WebCore::HTMLInputElement::didFinishInsertingNode):
>+
>+        Notify the InputType subclass that the datalist element may have changed after an input element is connected
>+        to the document with a non-empty list attribute.
>+
>+        (WebCore::HTMLInputElement::dataListMayHaveChanged):
>+        (WebCore::ListAttributeTargetObserver::idTargetChanged):
>+        (WebCore::HTMLInputElement::listAttributeTargetChanged): Deleted.
>+
>+        Rename listAttributeTargetChanged to dataListMayHaveChanged, since it is no longer called only when the list
>+        attribute changes value, but rather when the input's datalist element may have changed.
>+
>+        * html/HTMLInputElement.h:
>+        * html/InputType.cpp:
>+        (WebCore::InputType::dataListMayHaveChanged):
>+        (WebCore::InputType::listAttributeTargetChanged): Deleted.
>+        * html/InputType.h:
>+        * html/RangeInputType.cpp:
>+        (WebCore::RangeInputType::dataListMayHaveChanged):
>+        (WebCore::RangeInputType::listAttributeTargetChanged): Deleted.
>+        * html/RangeInputType.h:
>+        * html/TextFieldInputType.cpp:
>+        (WebCore::TextFieldInputType::dataListMayHaveChanged):
>+        (WebCore::TextFieldInputType::listAttributeTargetChanged): Deleted.
>+        * html/TextFieldInputType.h:
>+
> 2020-04-01  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Remove some PLATFORM(IOS_FAMILY) guards in TextFieldInputType
>diff --git a/Source/WebCore/html/HTMLInputElement.cpp b/Source/WebCore/html/HTMLInputElement.cpp
>index f8c4dcc414a27b1810caa48d1fae559fa17457c3..b4a2033e5161926eaada67218bedbca6aef92c36 100644
>--- a/Source/WebCore/html/HTMLInputElement.cpp
>+++ b/Source/WebCore/html/HTMLInputElement.cpp
>@@ -798,7 +798,7 @@ void HTMLInputElement::parseAttribute(const QualifiedName& name, const AtomStrin
>         m_hasNonEmptyList = !value.isEmpty();
>         if (m_hasNonEmptyList) {
>             resetListAttributeTargetObserver();
>-            listAttributeTargetChanged();
>+            dataListMayHaveChanged();
>         }
>     }
> #endif
>@@ -1552,6 +1552,10 @@ void HTMLInputElement::didFinishInsertingNode()
>     HTMLTextFormControlElement::didFinishInsertingNode();
>     if (isInTreeScope() && !form())
>         addToRadioButtonGroup();
>+#if ENABLE(DATALIST_ELEMENT)
>+    if (isConnected() && m_hasNonEmptyList)
>+        dataListMayHaveChanged();
>+#endif
> }
> 
> void HTMLInputElement::removedFromAncestor(RemovalType removalType, ContainerNode& oldParentOfRemovedTree)
>@@ -1653,9 +1657,9 @@ void HTMLInputElement::resetListAttributeTargetObserver()
>         m_listAttributeTargetObserver = nullptr;
> }
> 
>-void HTMLInputElement::listAttributeTargetChanged()
>+void HTMLInputElement::dataListMayHaveChanged()
> {
>-    m_inputType->listAttributeTargetChanged();
>+    m_inputType->dataListMayHaveChanged();
> }
> 
> #endif // ENABLE(DATALIST_ELEMENT)
>@@ -1965,7 +1969,7 @@ ListAttributeTargetObserver::ListAttributeTargetObserver(const AtomString& id, H
> 
> void ListAttributeTargetObserver::idTargetChanged()
> {
>-    m_element->listAttributeTargetChanged();
>+    m_element->dataListMayHaveChanged();
> }
> #endif
> 
>diff --git a/Source/WebCore/html/HTMLInputElement.h b/Source/WebCore/html/HTMLInputElement.h
>index 3deab6b4efad0bad6a7a509bfc87e62206612c55..dfc3f90b045ef27e8cbf71ed8876f8c5f6bbab21 100644
>--- a/Source/WebCore/html/HTMLInputElement.h
>+++ b/Source/WebCore/html/HTMLInputElement.h
>@@ -276,7 +276,7 @@ public:
> #if ENABLE(DATALIST_ELEMENT)
>     WEBCORE_EXPORT RefPtr<HTMLElement> list() const;
>     RefPtr<HTMLDataListElement> dataList() const;
>-    void listAttributeTargetChanged();
>+    void dataListMayHaveChanged();
> #endif
> 
>     Vector<Ref<HTMLInputElement>> radioButtonGroup() const;
>diff --git a/Source/WebCore/html/InputType.cpp b/Source/WebCore/html/InputType.cpp
>index f57fb6afcf450cfc165cabbc2469edee2a2871c2..10c81cc520f72b121908fda18f3d9fe9ab09a7ee 100644
>--- a/Source/WebCore/html/InputType.cpp
>+++ b/Source/WebCore/html/InputType.cpp
>@@ -926,7 +926,7 @@ String InputType::defaultToolTip() const
> }
> 
> #if ENABLE(DATALIST_ELEMENT)
>-void InputType::listAttributeTargetChanged()
>+void InputType::dataListMayHaveChanged()
> {
> }
> 
>diff --git a/Source/WebCore/html/InputType.h b/Source/WebCore/html/InputType.h
>index 208bf259041a350ca5123d0a7a1fe758d4a3b5e0..128a8ee74fc0715ac43015d8980f9d4c0f1b243f 100644
>--- a/Source/WebCore/html/InputType.h
>+++ b/Source/WebCore/html/InputType.h
>@@ -301,7 +301,7 @@ public:
>     void dispatchSimulatedClickIfActive(KeyboardEvent&) const;
> 
> #if ENABLE(DATALIST_ELEMENT)
>-    virtual void listAttributeTargetChanged();
>+    virtual void dataListMayHaveChanged();
>     virtual Optional<Decimal> findClosestTickMarkValue(const Decimal&);
> #endif
> 
>diff --git a/Source/WebCore/html/RangeInputType.cpp b/Source/WebCore/html/RangeInputType.cpp
>index d9d2612a8f0380647e6a412ac554827626f29ca2..bc649df23467329f95ebec821ac8eab6c76ae1f5 100644
>--- a/Source/WebCore/html/RangeInputType.cpp
>+++ b/Source/WebCore/html/RangeInputType.cpp
>@@ -374,7 +374,7 @@ bool RangeInputType::shouldRespectListAttribute()
> }
> 
> #if ENABLE(DATALIST_ELEMENT)
>-void RangeInputType::listAttributeTargetChanged()
>+void RangeInputType::dataListMayHaveChanged()
> {
>     m_tickMarkValuesDirty = true;
>     RefPtr<HTMLElement> sliderTrackElement = this->sliderTrackElement();
>diff --git a/Source/WebCore/html/RangeInputType.h b/Source/WebCore/html/RangeInputType.h
>index 51fb79d3f499e412e710fa76957f8baac92ffa21..4327c948ac9b87c49ca5e217c7a9310ee9d1926b 100644
>--- a/Source/WebCore/html/RangeInputType.h
>+++ b/Source/WebCore/html/RangeInputType.h
>@@ -68,7 +68,7 @@ private:
>     SliderThumbElement& typedSliderThumbElement() const;
> 
> #if ENABLE(DATALIST_ELEMENT)
>-    void listAttributeTargetChanged() final;
>+    void dataListMayHaveChanged() final;
>     void updateTickMarkValues();
>     Optional<Decimal> findClosestTickMarkValue(const Decimal&) final;
> 
>diff --git a/Source/WebCore/html/TextFieldInputType.cpp b/Source/WebCore/html/TextFieldInputType.cpp
>index 7895a7ebb4d376ef87b9a34539b2afcb99b6ed83..743c32aafe641328a25841c8dbef1eca28a74ecb 100644
>--- a/Source/WebCore/html/TextFieldInputType.cpp
>+++ b/Source/WebCore/html/TextFieldInputType.cpp
>@@ -835,7 +835,7 @@ void TextFieldInputType::updateAutoFillButton()
> 
> #if ENABLE(DATALIST_ELEMENT)
> 
>-void TextFieldInputType::listAttributeTargetChanged()
>+void TextFieldInputType::dataListMayHaveChanged()
> {
>     m_cachedSuggestions = { };
> 
>diff --git a/Source/WebCore/html/TextFieldInputType.h b/Source/WebCore/html/TextFieldInputType.h
>index d1d89a7bc6895dce91373e1d1afa6be89c124f1d..2c45722da1be0d56316a3abddac16a6f288f0dff 100644
>--- a/Source/WebCore/html/TextFieldInputType.h
>+++ b/Source/WebCore/html/TextFieldInputType.h
>@@ -126,7 +126,7 @@ private:
> #if ENABLE(DATALIST_ELEMENT)
>     void createDataListDropdownIndicator();
>     bool isPresentingAttachedView() const final;
>-    void listAttributeTargetChanged() final;
>+    void dataListMayHaveChanged() final;
>     void displaySuggestions(DataListSuggestionActivationType);
>     void closeSuggestions();
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 4faaaa2c80323df3c33d28d541629b04231b11e3..80e7d768309e887917afbda99003ddc2f330b4a2 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2020-04-02  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        Avoid null deref after inserting a text field with a list attribute
>+        https://bugs.webkit.org/show_bug.cgi?id=209909
>+        <rdar://problem/60742229>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a layout test to exercise the crashing scenario, and verify that the end result of programmatically
>+        inserting the text field is identical to simply putting an input field with a datalist in the markup.
>+
>+        * fast/forms/datalist/append-input-with-list-attribute-expected.html: Added.
>+        * fast/forms/datalist/append-input-with-list-attribute.html: Added.
>+
> 2020-04-01  Ross Kirsling  <ross.kirsling@sony.com>
> 
>         Intl.NumberFormat.prototype.format must preserve sign of -0
>diff --git a/LayoutTests/fast/forms/datalist/append-input-with-list-attribute-expected.html b/LayoutTests/fast/forms/datalist/append-input-with-list-attribute-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..177672dd9893b373735d5b560123f4a556962cb5
>--- /dev/null
>+++ b/LayoutTests/fast/forms/datalist/append-input-with-list-attribute-expected.html
>@@ -0,0 +1,6 @@
>+<!DOCTYPE html>
>+<body>
>+    <p>This test passes if it shows an input field with a datalist dropdown button (on platforms that show a dropdown button by default).</p>
>+    <datalist id="datalist"></datalist>
>+    <input list="datalist">
>+</body>
>\ No newline at end of file
>diff --git a/LayoutTests/fast/forms/datalist/append-input-with-list-attribute.html b/LayoutTests/fast/forms/datalist/append-input-with-list-attribute.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..867cea846dd69dd2789631743d7c916e582bde9a
>--- /dev/null
>+++ b/LayoutTests/fast/forms/datalist/append-input-with-list-attribute.html
>@@ -0,0 +1,12 @@
>+<!DOCTYPE html>
>+<body>
>+    <p>This test passes if it shows an input field with a datalist dropdown button (on platforms that show a dropdown button by default).</p>
>+    <script>
>+        const input = document.createElement("input");
>+        input.setAttribute("list", "datalist");
>+        const datalist = document.createElement("datalist");
>+        datalist.setAttribute("id", "datalist");
>+        document.body.appendChild(datalist);
>+        document.body.appendChild(input);
>+    </script>
>+</body>
>\ No newline at end of file

Actions: View | Formatted Diff | Diff

Attachments on bug 209909: 395274 | 395276