Changeset 267699 in webkit

Timestamp:

Sep 28, 2020, 7:55:51 AM (6 years ago)

Author:

Aditya Keerthi

Message:

Crash under DateTimeEditElement::blurFromField
​https://bugs.webkit.org/show_bug.cgi?id=216930
<rdar://problem/69308322>

Reviewed by Ryosuke Niwa.

DateTimeEditElement::blurFromField incorrectly invoked
HTMLElement::dispatchBlurEvent, as it was not the element being blurred.
Accessing member variables after this call is unsafe, as there is
nothing protecting the DateTimeEditElement.

To fix, the problematic methods were removed entirely, and replaced
with a new approach to responding to blur events within a DateTimeEditElement.

The blur event's relatedTarget is now compared to the fields within the
DateTimeEditElement, in order to determine when to close the calendar.
This approach is safe, since the relatedTarget is protected by
Document::setFocusedElement.

• html/BaseDateAndTimeInputType.cpp:

(WebCore::BaseDateAndTimeInputType::elementDidBlur): Remove unnecessary call.

If the input has editable fields, closing the calendar is handled by the
new codepath. Running this code unconditionally will result in a flash of the
calendar when clicking on a part of the input type that doesn't contain
an editable field.

• html/shadow/DateTimeEditElement.cpp:

(WebCore::DateTimeEditElement::didBlurFromField):

Check if the event's relatedTarget is another field within the same
DateTimeEditElement. If yes, the calendar should remain open.

• html/shadow/DateTimeEditElement.h:
• html/shadow/DateTimeFieldElement.cpp:

(WebCore::DateTimeFieldElement::defaultEventHandler):

Call handleBlurEvent() if a blur event occurs. This was previously done by
the now-removed dispatchBlurEvent override.

(WebCore::DateTimeFieldElement::handleBlurEvent):

• html/shadow/DateTimeFieldElement.h:
• html/shadow/DateTimeNumericFieldElement.cpp:

(WebCore::DateTimeNumericFieldElement::handleBlurEvent):

• html/shadow/DateTimeNumericFieldElement.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/BaseDateAndTimeInputType.cpp (modified) (1 diff)
• html/shadow/DateTimeEditElement.cpp (modified) (1 diff)
• html/shadow/DateTimeEditElement.h (modified) (1 diff)
• html/shadow/DateTimeFieldElement.cpp (modified) (2 diffs)
• html/shadow/DateTimeFieldElement.h (modified) (3 diffs)
• html/shadow/DateTimeNumericFieldElement.cpp (modified) (1 diff)
• html/shadow/DateTimeNumericFieldElement.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-09-28  Aditya Keerthi  <akeerthi@apple.com>
+
+        Crash under DateTimeEditElement::blurFromField
+        https://bugs.webkit.org/show_bug.cgi?id=216930
+        <rdar://problem/69308322>
+
+        Reviewed by Ryosuke Niwa.
+
+        DateTimeEditElement::blurFromField incorrectly invoked
+        HTMLElement::dispatchBlurEvent, as it was not the element being blurred.
+        Accessing member variables after this call is unsafe, as there is
+        nothing protecting the DateTimeEditElement.
+
+        To fix, the problematic methods were removed entirely, and replaced
+        with a new approach to responding to blur events within a DateTimeEditElement.
+
+        The blur event's relatedTarget is now compared to the fields within the
+        DateTimeEditElement, in order to determine when to close the calendar.
+        This approach is safe, since the relatedTarget is protected by
+        Document::setFocusedElement.
+
+        * html/BaseDateAndTimeInputType.cpp:
+        (WebCore::BaseDateAndTimeInputType::elementDidBlur): Remove unnecessary call.
+
+        If the input has editable fields, closing the calendar is handled by the
+        new codepath. Running this code unconditionally will result in a flash of the
+        calendar when clicking on a part of the input type that doesn't contain
+        an editable field.
+
+        * html/shadow/DateTimeEditElement.cpp:
+        (WebCore::DateTimeEditElement::didBlurFromField):
+
+        Check if the event's relatedTarget is another field within the same
+        DateTimeEditElement. If yes, the calendar should remain open.
+
+        * html/shadow/DateTimeEditElement.h:
+        * html/shadow/DateTimeFieldElement.cpp:
+        (WebCore::DateTimeFieldElement::defaultEventHandler):
+
+        Call handleBlurEvent() if a blur event occurs. This was previously done by
+        the now-removed dispatchBlurEvent override.
+
+        (WebCore::DateTimeFieldElement::handleBlurEvent):
+        * html/shadow/DateTimeFieldElement.h:
+        * html/shadow/DateTimeNumericFieldElement.cpp:
+        (WebCore::DateTimeNumericFieldElement::handleBlurEvent):
+        * html/shadow/DateTimeNumericFieldElement.h:
+
 2020-09-28  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/html/BaseDateAndTimeInputType.cpp

@@ -395 +395 @@
 void BaseDateAndTimeInputType::elementDidBlur()
 {
-    closeDateTimeChooser();
+    if (!m_dateTimeEditElement)
+        closeDateTimeChooser();
 }
 

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

@@ -272 +272 @@
 }
 
-void DateTimeEditElement::blurFromField(RefPtr<Element>&& newFocusedElement)
-{
-    bool notifyOwner = notFound == m_fields.findMatching([&] (auto& field) {
-        return field.ptr() == newFocusedElement.get();
-    });
-
-    HTMLElement::dispatchBlurEvent(WTFMove(newFocusedElement));
-
-    if (m_editControlOwner && notifyOwner)
-        m_editControlOwner->didBlurFromControl();
+void DateTimeEditElement::didBlurFromField(Event& event)
+{
+    if (!m_editControlOwner)
+        return;
+
+    if (auto* newFocusedElement = event.relatedTarget()) {
+        bool didFocusSiblingField = notFound != m_fields.findMatching([&] (auto& field) {
+            return field.ptr() == newFocusedElement;
+        });
+
+        if (didFocusSiblingField)
+            return;
+    }
+
+    m_editControlOwner->didBlurFromControl();
 }
 

trunk/Source/WebCore/html/shadow/DateTimeEditElement.h

@@ -97 +97 @@
 
     // DateTimeFieldElement::FieldOwner functions:
-    void blurFromField(RefPtr<Element>&& newFocusedElement) final;
+    void didBlurFromField(Event&) final;
     void fieldValueChanged() final;
     bool focusOnNextField(const DateTimeFieldElement&) final;

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

@@ -61 +61 @@
 void DateTimeFieldElement::defaultEventHandler(Event& event)
 {
+    if (event.type() == eventNames().blurEvent)
+        handleBlurEvent(event);
+
     if (is<KeyboardEvent>(event)) {
         auto& keyboardEvent = downcast<KeyboardEvent>(event);
@@ -139 +142 @@
 }
 
-void DateTimeFieldElement::dispatchBlurEvent(RefPtr<Element>&& newFocusedElement)
+void DateTimeFieldElement::handleBlurEvent(Event& event)
 {
     if (m_fieldOwner)
-        m_fieldOwner->blurFromField(WTFMove(newFocusedElement));
-    else
-        HTMLElement::dispatchBlurEvent(WTFMove(newFocusedElement));
-
-    didBlur();
-}
-
-void DateTimeFieldElement::didBlur()
-{
+        m_fieldOwner->didBlurFromField(event);
 }
 

trunk/Source/WebCore/html/shadow/DateTimeFieldElement.h

@@ -46 +46 @@
     public:
         virtual ~FieldOwner();
-        virtual void blurFromField(RefPtr<Element>&& newFocusedElement) = 0;
+        virtual void didBlurFromField(Event&) = 0;
         virtual void fieldValueChanged() = 0;
         virtual bool focusOnNextField(const DateTimeFieldElement&) = 0;
@@ -56 +56 @@
 
     void defaultEventHandler(Event&) override;
-    void dispatchBlurEvent(RefPtr<Element>&& newFocusedElement) override;
     bool isFocusable() const final;
 
@@ -77 +76 @@
     virtual int valueAsInteger() const = 0;
     virtual void handleKeyboardEvent(KeyboardEvent&) = 0;
-
-    virtual void didBlur();
+    virtual void handleBlurEvent(Event&);
 
 private:

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

@@ -164 +164 @@
 }
 
-void DateTimeNumericFieldElement::didBlur()
+void DateTimeNumericFieldElement::handleBlurEvent(Event& event)
 {
     m_typeAheadBuffer.clear();
+    DateTimeFieldElement::handleBlurEvent(event);
 }
 

trunk/Source/WebCore/html/shadow/DateTimeNumericFieldElement.h

@@ -67 +67 @@
     String value() const final;
     void handleKeyboardEvent(KeyboardEvent&) final;
-    void didBlur() final;
+    void handleBlurEvent(Event&) final;
 
     String formatValue(int) const;