Changeset 238409 in webkit

Timestamp:

Nov 20, 2018 4:12:25 PM (5 years ago)

Author:

rniwa@webkit.org

Message:

Input element gains focus when a selectstart event listener on document prevents the default action
​https://bugs.webkit.org/show_bug.cgi?id=191714
<rdar://problem/46174389>

Reviewed by Antti Koivisto.

Source/WebCore:

The bug was caused by WebKit keep firing selectstart upon mousemove after the drag had already started
when preventDefault had been called in the previous firings of selectstart event. Because input element
has its own editable element and fires selectstart on the input element itself, which won't be prevented
by selectstart on docuemnt, this allowed the selection to be set inside the input element even though
the mouse cursor was simply passing over the input element after the drag had already started.

Fixed the bug by not firing selectstart if the default action had been prevented by the initial firing
of selectstart by setting m_mouseDownMayStartDrag to false. This also matches the behaviors of Chrome
and Firefox.

Test: fast/events/selectstart-prevent-default-should-not-focus-input.html

• page/EventHandler.cpp:

(WebCore::EventHandler::updateSelectionForMouseDownDispatchingSelectStart):
(WebCore::EventHandler::updateSelectionForMouseDrag):

LayoutTests:

Added a regression test.

• fast/events/selectstart-prevent-default-should-not-focus-input-expected.txt: Added.
• fast/events/selectstart-prevent-default-should-not-focus-input.html: Added.
• platform/ios/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/selectstart-prevent-default-should-not-focus-input-expected.txt (added)
• LayoutTests/fast/events/selectstart-prevent-default-should-not-focus-input.html (added)
• LayoutTests/platform/ios/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/EventHandler.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-11-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Input element gains focus when a selectstart event listener on document prevents the default action
+        https://bugs.webkit.org/show_bug.cgi?id=191714
+        <rdar://problem/46174389>
+
+        Reviewed by Antti Koivisto.
+
+        Added a regression test.
+
+        * fast/events/selectstart-prevent-default-should-not-focus-input-expected.txt: Added.
+        * fast/events/selectstart-prevent-default-should-not-focus-input.html: Added.
+        * platform/ios/TestExpectations:
+
 2018-11-19  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/LayoutTests/platform/ios/TestExpectations

@@ -530 +530 @@
 fast/events/selectstart-by-single-click-with-shift.html [ Skip ]
 fast/events/selectstart-prevent-selection-on-right-click.html [ Skip ]
+fast/events/selectstart-prevent-default-should-not-focus-input.html [ Skip ]
 fast/events/shadow-event-path-2.html [ Skip ]
 fast/events/shadow-event-path.html [ Skip ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-11-20  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Input element gains focus when a selectstart event listener on document prevents the default action
+        https://bugs.webkit.org/show_bug.cgi?id=191714
+        <rdar://problem/46174389>
+
+        Reviewed by Antti Koivisto.
+
+        The bug was caused by WebKit keep firing selectstart upon mousemove after the drag had already started
+        when preventDefault had been called in the previous firings of selectstart event. Because input element
+        has its own editable element and fires selectstart on the input element itself, which won't be prevented
+        by selectstart on docuemnt, this allowed the selection to be set inside the input element even though
+        the mouse cursor was simply passing over the input element after the drag had already started.
+
+        Fixed the bug by not firing selectstart if the default action had been prevented by the initial firing
+        of selectstart by setting m_mouseDownMayStartDrag to false. This also matches the behaviors of Chrome
+        and Firefox.
+
+        Test: fast/events/selectstart-prevent-default-should-not-focus-input.html
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::updateSelectionForMouseDownDispatchingSelectStart):
+        (WebCore::EventHandler::updateSelectionForMouseDrag):
+
 2018-11-20  Christopher Reid  <chris.reid@sony.com>
 

trunk/Source/WebCore/page/EventHandler.cpp

@@ -511 +511 @@
         return false;
 
-    if (!dispatchSelectStart(targetNode))
-        return false;
+    if (!dispatchSelectStart(targetNode)) {
+        m_mouseDownMayStartSelect = false;
+        return false;
+    }
 
     if (selection.isRange())
@@ -954 +956 @@
     // Special case to limit selection to the containing block for SVG text.
     // FIXME: Isn't there a better non-SVG-specific way to do this?
-    if (Node* selectionBaseNode = newSelection.base().deprecatedNode())
-        if (RenderObject* selectionBaseRenderer = selectionBaseNode->renderer())
-            if (selectionBaseRenderer->isSVGText())
+    if (Node* selectionBaseNode = newSelection.base().deprecatedNode()) {
+        if (RenderObject* selectionBaseRenderer = selectionBaseNode->renderer()) {
+            if (selectionBaseRenderer->isSVGText()) {
                 if (target->renderer()->containingBlock() != selectionBaseRenderer->containingBlock())
                     return;
-
-    if (m_selectionInitiationState == HaveNotStartedSelection && !dispatchSelectStart(target))
-        return;
+            }
+        }
+    }
+
+
+    if (m_selectionInitiationState == HaveNotStartedSelection && !dispatchSelectStart(target)) {
+        m_mouseDownMayStartSelect = false;
+        return;
+    }
 
     if (m_selectionInitiationState != ExtendedSelection) {