Title: [233311] trunk
Revision
233311
Author
[email protected]
Date
2018-06-28 10:12:05 -0700 (Thu, 28 Jun 2018)

Log 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 <[email protected]> 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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (233310 => 233311)


--- trunk/LayoutTests/ChangeLog	2018-06-28 17:07:36 UTC (rev 233310)
+++ trunk/LayoutTests/ChangeLog	2018-06-28 17:12:05 UTC (rev 233311)
@@ -1,3 +1,17 @@
+2018-06-28  Aditya Keerthi  <[email protected]>
+
+        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  <[email protected]>
 
         [css-masking] Update clip-path box mapping to unified box

Added: trunk/LayoutTests/fast/events/ios/click-selectionchange-once-expected.txt (0 => 233311)


--- trunk/LayoutTests/fast/events/ios/click-selectionchange-once-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/ios/click-selectionchange-once-expected.txt	2018-06-28 17:12:05 UTC (rev 233311)
@@ -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
+
+

Added: trunk/LayoutTests/fast/events/ios/click-selectionchange-once.html (0 => 233311)


--- trunk/LayoutTests/fast/events/ios/click-selectionchange-once.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/ios/click-selectionchange-once.html	2018-06-28 17:12:05 UTC (rev 233311)
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+    <script src=""
+    <script src=""
+</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=""
+</html>

Modified: trunk/Source/WebCore/ChangeLog (233310 => 233311)


--- trunk/Source/WebCore/ChangeLog	2018-06-28 17:07:36 UTC (rev 233310)
+++ trunk/Source/WebCore/ChangeLog	2018-06-28 17:12:05 UTC (rev 233311)
@@ -1,3 +1,27 @@
+2018-06-28  Aditya Keerthi  <[email protected]>
+
+        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  <[email protected]>
 
         Fix encoding / decoding issues in ResourceLoadStatistics

Modified: trunk/Source/WebCore/page/EventHandler.cpp (233310 => 233311)


--- trunk/Source/WebCore/page/EventHandler.cpp	2018-06-28 17:07:36 UTC (rev 233310)
+++ trunk/Source/WebCore/page/EventHandler.cpp	2018-06-28 17:12:05 UTC (rev 233311)
@@ -681,6 +681,12 @@
     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()) {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to