REGRESSION (r232040): Cursor jumping in Safari text fields
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Jun 2018 17:12:05 +0000 (17:12 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Jun 2018 17:12:05 +0000 (17:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187142
<rdar://problem/41397577>

Patch by Aditya Keerthi <akeerthi@apple.com> 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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233311 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/events/ios/click-selectionchange-once-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/ios/click-selectionchange-once.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/EventHandler.cpp

index 0d1eff4..18bca07 100644 (file)
@@ -1,3 +1,17 @@
+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>
 
         [css-masking] Update clip-path box mapping to unified box
diff --git a/LayoutTests/fast/events/ios/click-selectionchange-once-expected.txt b/LayoutTests/fast/events/ios/click-selectionchange-once-expected.txt
new file mode 100644 (file)
index 0000000..4f51a88
--- /dev/null
@@ -0,0 +1,12 @@
+PASS document.getElementById('selectionChange').textContent is "1"
+PASS document.getElementById('selectionStart').textContent is "45"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+The selectionchange event should only be fired once when clicking a node that is being edited.
+
+The selectionchange count in the editable node is: 1
+
+The caret position in the editable node is: 45
+
+
diff --git a/LayoutTests/fast/events/ios/click-selectionchange-once.html b/LayoutTests/fast/events/ios/click-selectionchange-once.html
new file mode 100644 (file)
index 0000000..86210ab
--- /dev/null
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <script src="../../../resources/js-test-pre.js"></script>
+    <script src="../../../resources/ui-helper.js"></script>
+</head>
+<body>
+    <div id="description">
+        <p>The selectionchange event should only be fired once when clicking a node that is being edited.</p>
+        <p>The selectionchange count in the editable node is: <span id="selectionChange">0</span></p>
+        <p>The caret position in the editable node is: <span id="selectionStart">0</span></p>
+    </div>
+    <input id="input" style:"width: 100%"/>
+</body>
+<script>
+
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    jsTestIsAsync = true;
+
+    const x = input.offsetLeft;
+    const y = input.offsetTop + input.offsetHeight / 2;
+    const string = "Thisisareallylongstringthatfillstheinputfield";
+    input.value = string;
+
+    UIHelper.activateAndWaitForInputSessionAt(x, y).then(() => {
+        selectionChangeCount = 0;
+        document.addEventListener("selectionchange", function(e) {
+            selectionChangeCount += 1;
+            selectionChange.textContent = selectionChangeCount;
+            selectionStart.textContent = input.selectionStart;
+        });
+
+        UIHelper.activateElement(input).then(() => {
+            shouldBeEqualToString("document.getElementById('selectionChange').textContent", `${selectionChangeCount}`);
+            shouldBeEqualToString("document.getElementById('selectionStart').textContent", `${string.length}`);
+            finishJSTest();
+        });
+    });
+}
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</html>
index 5e5baf7..469e7ea 100644 (file)
@@ -1,3 +1,27 @@
+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>
 
         Fix encoding / decoding issues in ResourceLoadStatistics
index c16cf2c..26c89b0 100644 (file)
@@ -681,6 +681,12 @@ bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestR
     VisibleSelection newSelection = m_frame.selection().selection();
     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));
         if (selectionInUserSelectAll.isRange()) {