Title: [222114] trunk
Revision
222114
Author
wenson_hs...@apple.com
Date
2017-09-15 15:18:51 -0700 (Fri, 15 Sep 2017)

Log Message

Avoid style recomputation when forwarding a focus event to an text field's input type
https://bugs.webkit.org/show_bug.cgi?id=176160
<rdar://problem/34184820>

Reviewed by Ryosuke Niwa.

Source/WebCore:

Currently, TextFieldInputType::forwardEvent synchronously triggers style recomputation, for the purpose of
scrolling to the origin upon handling a blur event, and also for updating caps lock state after a blur or focus.
In synchronously triggering style recomputation, we may end up running arbitrary _javascript_, which may change
the HTMLInputElement's type and cause the current TextFieldInputType to be destroyed.

To mitigate this, we only update caps lock state when forwarding a focus or blur event to the InputType, and
instead scroll blurred text fields to the origin later, in HTMLInputElement::didBlur (invoked from
Document::setFocusedElement after blur and focusout events have fired). Instead of having the InputType update
style, lift the call to Document::updateStyleIfNeeded up into HTMLInputElement so that we gracefully handle the
case where the page destroys and sets a new InputType within the scope of this style update.

Test: fast/forms/change-input-type-in-focus-handler.html

* dom/Document.cpp:
(WebCore::Document::setFocusedElement):
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::didBlur):
* html/HTMLInputElement.h:
* html/InputType.h:
(WebCore::InputType::elementDidBlur):
* html/TextFieldInputType.cpp:
(WebCore::TextFieldInputType::forwardEvent):
(WebCore::TextFieldInputType::elementDidBlur):
* html/TextFieldInputType.h:

LayoutTests:

Adds a new layout test verifying that we don't crash when changing the input type from within a focus event listener.

* fast/forms/change-input-type-in-focus-handler-expected.txt: Added.
* fast/forms/change-input-type-in-focus-handler.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (222113 => 222114)


--- trunk/LayoutTests/ChangeLog	2017-09-15 21:28:55 UTC (rev 222113)
+++ trunk/LayoutTests/ChangeLog	2017-09-15 22:18:51 UTC (rev 222114)
@@ -1,3 +1,16 @@
+2017-09-15  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Avoid style recomputation when forwarding a focus event to an text field's input type
+        https://bugs.webkit.org/show_bug.cgi?id=176160
+        <rdar://problem/34184820>
+
+        Reviewed by Ryosuke Niwa.
+
+        Adds a new layout test verifying that we don't crash when changing the input type from within a focus event listener.
+
+        * fast/forms/change-input-type-in-focus-handler-expected.txt: Added.
+        * fast/forms/change-input-type-in-focus-handler.html: Added.
+
 2017-09-14  Ryan Haddad  <ryanhad...@apple.com>
 
         Move test expectations from 'mac-highsierra' to 'mac' directory

Added: trunk/LayoutTests/fast/forms/change-input-type-in-focus-handler-expected.txt (0 => 222114)


--- trunk/LayoutTests/fast/forms/change-input-type-in-focus-handler-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/change-input-type-in-focus-handler-expected.txt	2017-09-15 22:18:51 UTC (rev 222114)
@@ -0,0 +1 @@
+PASS

Added: trunk/LayoutTests/fast/forms/change-input-type-in-focus-handler.html (0 => 222114)


--- trunk/LayoutTests/fast/forms/change-input-type-in-focus-handler.html	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/change-input-type-in-focus-handler.html	2017-09-15 22:18:51 UTC (rev 222114)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<div id="div">
+    <form id="form">
+        <input id="input" _onfocus_="focused()" type="tel"></input>
+    </form>
+</div>
+
+<script>
+function focused() {
+    if (input.autofocus) {
+        input.type = "checkbox";
+        document.body.innerHTML = "<code style='color: green'>PASS</code>";
+        return;
+    }
+
+    document.body.appendChild(input);
+    input.autofocus = true;
+}
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+input.focus();
+</script>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (222113 => 222114)


--- trunk/Source/WebCore/ChangeLog	2017-09-15 21:28:55 UTC (rev 222113)
+++ trunk/Source/WebCore/ChangeLog	2017-09-15 22:18:51 UTC (rev 222114)
@@ -1,3 +1,36 @@
+2017-09-15  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Avoid style recomputation when forwarding a focus event to an text field's input type
+        https://bugs.webkit.org/show_bug.cgi?id=176160
+        <rdar://problem/34184820>
+
+        Reviewed by Ryosuke Niwa.
+
+        Currently, TextFieldInputType::forwardEvent synchronously triggers style recomputation, for the purpose of
+        scrolling to the origin upon handling a blur event, and also for updating caps lock state after a blur or focus.
+        In synchronously triggering style recomputation, we may end up running arbitrary _javascript_, which may change
+        the HTMLInputElement's type and cause the current TextFieldInputType to be destroyed.
+
+        To mitigate this, we only update caps lock state when forwarding a focus or blur event to the InputType, and
+        instead scroll blurred text fields to the origin later, in HTMLInputElement::didBlur (invoked from
+        Document::setFocusedElement after blur and focusout events have fired). Instead of having the InputType update
+        style, lift the call to Document::updateStyleIfNeeded up into HTMLInputElement so that we gracefully handle the
+        case where the page destroys and sets a new InputType within the scope of this style update.
+
+        Test: fast/forms/change-input-type-in-focus-handler.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::setFocusedElement):
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::didBlur):
+        * html/HTMLInputElement.h:
+        * html/InputType.h:
+        (WebCore::InputType::elementDidBlur):
+        * html/TextFieldInputType.cpp:
+        (WebCore::TextFieldInputType::forwardEvent):
+        (WebCore::TextFieldInputType::elementDidBlur):
+        * html/TextFieldInputType.h:
+
 2017-09-15  JF Bastien  <jfbast...@apple.com>
 
         WTF: use Forward.h when appropriate instead of Vector.h

Modified: trunk/Source/WebCore/dom/Document.cpp (222113 => 222114)


--- trunk/Source/WebCore/dom/Document.cpp	2017-09-15 21:28:55 UTC (rev 222113)
+++ trunk/Source/WebCore/dom/Document.cpp	2017-09-15 22:18:51 UTC (rev 222114)
@@ -3839,6 +3839,9 @@
             else
                 view()->setFocus(false);
         }
+
+        if (is<HTMLInputElement>(oldFocusedElement.get()))
+            downcast<HTMLInputElement>(*oldFocusedElement).didBlur();
     }
 
     if (newFocusedElement && newFocusedElement->isFocusable()) {

Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (222113 => 222114)


--- trunk/Source/WebCore/html/HTMLInputElement.cpp	2017-09-15 21:28:55 UTC (rev 222113)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp	2017-09-15 22:18:51 UTC (rev 222114)
@@ -1120,6 +1120,14 @@
     m_inputType->didDispatchClick(&event, state);
 }
 
+void HTMLInputElement::didBlur()
+{
+    // We need to update style here, rather than in InputType itself, since style recomputation may fire events
+    // that could change the input's type.
+    document().updateStyleIfNeeded();
+    m_inputType->elementDidBlur();
+}
+
 void HTMLInputElement::defaultEventHandler(Event& event)
 {
     if (is<MouseEvent>(event) && event.type() == eventNames().clickEvent && downcast<MouseEvent>(event).button() == LeftButton) {

Modified: trunk/Source/WebCore/html/HTMLInputElement.h (222113 => 222114)


--- trunk/Source/WebCore/html/HTMLInputElement.h	2017-09-15 21:28:55 UTC (rev 222113)
+++ trunk/Source/WebCore/html/HTMLInputElement.h	2017-09-15 22:18:51 UTC (rev 222114)
@@ -213,6 +213,8 @@
     void willDispatchEvent(Event&, InputElementClickState&);
     void didDispatchClickEvent(Event&, const InputElementClickState&);
 
+    void didBlur();
+
     int maxResults() const { return m_maxResults; }
 
     WEBCORE_EXPORT String defaultValue() const;

Modified: trunk/Source/WebCore/html/InputType.h (222113 => 222114)


--- trunk/Source/WebCore/html/InputType.h	2017-09-15 21:28:55 UTC (rev 222113)
+++ trunk/Source/WebCore/html/InputType.h	2017-09-15 22:18:51 UTC (rev 222114)
@@ -203,6 +203,8 @@
     virtual void subtreeHasChanged();
     virtual void blur();
 
+    virtual void elementDidBlur() { }
+
 #if ENABLE(TOUCH_EVENTS)
     virtual bool hasTouchEventHandler() const;
 #endif

Modified: trunk/Source/WebCore/html/TextFieldInputType.cpp (222113 => 222114)


--- trunk/Source/WebCore/html/TextFieldInputType.cpp	2017-09-15 21:28:55 UTC (rev 222113)
+++ trunk/Source/WebCore/html/TextFieldInputType.cpp	2017-09-15 22:18:51 UTC (rev 222114)
@@ -188,29 +188,31 @@
             return;
     }
 
-    if (event.isMouseEvent()
-        || event.type() == eventNames().blurEvent
-        || event.type() == eventNames().focusEvent)
-    {
-        element().document().updateStyleIfNeeded();
+    bool isFocusEvent = event.type() == eventNames().focusEvent;
+    bool isBlurEvent = event.type() == eventNames().blurEvent;
+    if (isFocusEvent || isBlurEvent)
+        capsLockStateMayHaveChanged();
+    if (event.isMouseEvent() || isFocusEvent || isBlurEvent)
+        element().forwardEvent(event);
+}
 
-        auto* renderer = element().renderer();
-        if (element().renderer()) {
-            if (event.type() == eventNames().blurEvent) {
-                if (auto* innerTextRenderer = innerTextElement()->renderer()) {
-                    if (auto* innerLayer = innerTextRenderer->layer()) {
-                        bool isLeftToRightDirection = downcast<RenderTextControlSingleLine>(*renderer).style().isLeftToRightDirection();
-                        ScrollOffset scrollOffset(isLeftToRightDirection ? 0 : innerLayer->scrollWidth(), 0);
-                        innerLayer->scrollToOffset(scrollOffset, RenderLayer::ScrollOffsetClamped);
-                    }
-                }
-                capsLockStateMayHaveChanged();
-            } else if (event.type() == eventNames().focusEvent)
-                capsLockStateMayHaveChanged();
+void TextFieldInputType::elementDidBlur()
+{
+    auto* renderer = element().renderer();
+    if (!renderer)
+        return;
 
-            element().forwardEvent(event);
-        }
-    }
+    auto* innerTextRenderer = innerTextElement()->renderer();
+    if (!innerTextRenderer)
+        return;
+
+    auto* innerLayer = innerTextRenderer->layer();
+    if (!innerLayer)
+        return;
+
+    bool isLeftToRightDirection = downcast<RenderTextControlSingleLine>(*renderer).style().isLeftToRightDirection();
+    ScrollOffset scrollOffset(isLeftToRightDirection ? 0 : innerLayer->scrollWidth(), 0);
+    innerLayer->scrollToOffset(scrollOffset, RenderLayer::ScrollOffsetClamped);
 }
 
 void TextFieldInputType::handleFocusEvent(Node* oldFocusedNode, FocusDirection)

Modified: trunk/Source/WebCore/html/TextFieldInputType.h (222113 => 222114)


--- trunk/Source/WebCore/html/TextFieldInputType.h	2017-09-15 21:28:55 UTC (rev 222113)
+++ trunk/Source/WebCore/html/TextFieldInputType.h	2017-09-15 22:18:51 UTC (rev 222114)
@@ -90,6 +90,7 @@
     void subtreeHasChanged() final;
     void capsLockStateMayHaveChanged() final;
     void updateAutoFillButton() final;
+    void elementDidBlur() final;
 
     // SpinButtonElement::SpinButtonOwner functions.
     void focusAndSelectSpinButtonOwner() final;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to