Changeset 233311 in webkit

Timestamp:

Jun 28, 2018 10:12:05 AM (6 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION (r232040): Cursor jumping in Safari text fields
​https://bugs.webkit.org/show_bug.cgi?id=187142
<rdar://problem/41397577>

Patch by Aditya Keerthi <Aditya Keerthi> on 2018-06-28
Reviewed by Tim Horton.

Source/WebCore:

r232040 enabled click events to fire on nodes that are already being edited in
iOS. This resulted FrameSelection::setSelection being called twice. One call
originated from the UIWKTextInteractionAssistant, which snaps the caret to word
boundaries. The other call originates from handleMousePressEvent in EventHandler,
and uses character boundaries. Consequently, we see the caret jumping around.

To fix this issue, an early return was added in the handleMousePressEvent
codepath, which prevents FrameSelection::setSelection from being called when
clicking on a node that is already being edited. This ensures that the
UIWKTextInteractionAssistant codepath is the only influence on the caret position.

Test: fast/events/ios/click-selectionchange-once.html

• page/EventHandler.cpp:

(WebCore::EventHandler::handleMousePressEventSingleClick):

LayoutTests:

Added test to ensure that the 'selectionchange' event is only fired once per
click in an editable node.

• fast/events/ios/click-selectionchange-once-expected.txt: Added.
• fast/events/ios/click-selectionchange-once.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/ios/click-selectionchange-once-expected.txt (added)
• LayoutTests/fast/events/ios/click-selectionchange-once.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/EventHandler.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-06-28  Aditya Keerthi  <akeerthi@apple.com>
+
+        REGRESSION (r232040): Cursor jumping in Safari text fields
+        https://bugs.webkit.org/show_bug.cgi?id=187142
+        <rdar://problem/41397577>
+
+        Reviewed by Tim Horton.
+
+        Added test to ensure that the 'selectionchange' event is only fired once per
+        click in an editable node.
+
+        * fast/events/ios/click-selectionchange-once-expected.txt: Added.
+        * fast/events/ios/click-selectionchange-once.html: Added.
+
 2018-06-28  Dirk Schulze  <krit@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-06-28  Aditya Keerthi  <akeerthi@apple.com>
+
+        REGRESSION (r232040): Cursor jumping in Safari text fields
+        https://bugs.webkit.org/show_bug.cgi?id=187142
+        <rdar://problem/41397577>
+
+        Reviewed by Tim Horton.
+
+        r232040 enabled click events to fire on nodes that are already being edited in
+        iOS. This resulted FrameSelection::setSelection being called twice. One call
+        originated from the UIWKTextInteractionAssistant, which snaps the caret to word
+        boundaries. The other call originates from handleMousePressEvent in EventHandler,
+        and uses character boundaries. Consequently, we see the caret jumping around.
+
+        To fix this issue, an early return was added in the handleMousePressEvent
+        codepath, which prevents FrameSelection::setSelection from being called when
+        clicking on a node that is already being edited. This ensures that the
+        UIWKTextInteractionAssistant codepath is the only influence on the caret position.
+
+        Test: fast/events/ios/click-selectionchange-once.html
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::handleMousePressEventSingleClick):
+
 2018-06-28  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/page/EventHandler.cpp

@@ -682 +682 @@
     TextGranularity granularity = CharacterGranularity;
 
+#if PLATFORM(IOS)
+    // The text selection assistant will handle selection in the case where we are already editing the node
+    if (newSelection.rootEditableElement() == targetNode->rootEditableElement())
+        return true;
+#endif
+
     if (extendSelection && newSelection.isCaretOrRange()) {
         VisibleSelection selectionInUserSelectAll = expandSelectionToRespectSelectOnMouseDown(*targetNode, VisibleSelection(pos));