Title: [271386] trunk
Revision
271386
Author
[email protected]
Date
2021-01-11 16:54:36 -0800 (Mon, 11 Jan 2021)

Log Message

Double tap to select does not work if the page clears selections on tap, like grammarly.com does
https://bugs.webkit.org/show_bug.cgi?id=220454
Source/WebKit:

rdar://67757411

Reviewed by Wenson Hsieh.

Double tap to select content did not work on grammerly.com on iPad because this is a desktop website run on a touch-based device,
and there was a script running that would clear the selection on a mouseDown event. When we would send synthetic click events to the page
we would first update the selection and then on completion of the tap, we would dispatch a mouseDown and mouseUp event on iOS. On Mac, we
update the selection between the mouseDown and the mouseUp, so in order to bring us more in line with mac/mouse behaviors, we are now saving
the information needed to update the selection when we believe we are in the middle of a potential tap, and then setting the selection between
these two events. This makes for a more expected change of events, and does not let grammerly.com clear a valid selection based off of synthetic
clicks.

Test: fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch.html

* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::completeSyntheticClick):
(WebKit::WebPage::setSelectionRange):
(WebKit::WebPage::selectTextWithGranularityAtPoint):

LayoutTests:

Reviewed by Wenson Hsieh.

* fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch-expected.txt: Added.
* fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (271385 => 271386)


--- trunk/LayoutTests/ChangeLog	2021-01-12 00:11:48 UTC (rev 271385)
+++ trunk/LayoutTests/ChangeLog	2021-01-12 00:54:36 UTC (rev 271386)
@@ -1,3 +1,13 @@
+2021-01-11  Megan Gardner  <[email protected]>
+
+        Double tap to select does not work if the page clears selections on tap, like grammarly.com does
+        https://bugs.webkit.org/show_bug.cgi?id=220454
+
+        Reviewed by Wenson Hsieh.
+
+        * fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch-expected.txt: Added.
+        * fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch.html: Added.
+
 2021-01-11  Julian Gonzalez  <[email protected]>
 
         Relax assertion in Element::dispatchFocusOutEvent() for non-web process case

Added: trunk/LayoutTests/fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch-expected.txt (0 => 271386)


--- trunk/LayoutTests/fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch-expected.txt	2021-01-12 00:54:36 UTC (rev 271386)
@@ -0,0 +1,3 @@
+PASS: Has Caret Selection
+PASS: Correct Selection
+

Added: trunk/LayoutTests/fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch.html (0 => 271386)


--- trunk/LayoutTests/fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch.html	2021-01-12 00:54:36 UTC (rev 271386)
@@ -0,0 +1,96 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <script src=""
+    <script>
+
+        function clearSelection()
+        {
+            var selection = window.getSelection() || document.selection;
+            selection.removeAllRanges();
+        }
+
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+    
+        async function runTest()
+        {
+            if (!testRunner.runUIScript)
+                return;
+
+            var output = '';
+            var editableTargetRect = document.getElementById('editable').getBoundingClientRect();
+            var firstTargetRect = document.getElementById('firstSelection').getBoundingClientRect();
+            var noneditableToEditableOffset = document.getElementById('editable').getBoundingClientRect().y - document.getElementById('noneditable').getBoundingClientRect().y;
+
+            var tapPointX = editableTargetRect.x + editableTargetRect.width / 2;
+            var tapPointY = editableTargetRect.y + editableTargetRect.height / 2;
+            var doubleTapPointX = firstTargetRect.x + firstTargetRect.width / 2;
+            var doubleTapPointY = firstTargetRect.y + firstTargetRect.height / 2 + noneditableToEditableOffset;
+        
+            await tapAtPoint(tapPointX, tapPointY);
+            if (document.getSelection().type == "Caret")
+                output += 'PASS: Has Caret Selection';
+            else
+                output += 'FAIL: failed to activate caret as a result of a tap. Incorrect Selection: ' + document.getSelection().toString();
+            output += '<br>';
+
+            await didShowKeyboard();
+
+            await doubleTapAtPoint(doubleTapPointX, doubleTapPointY);
+            if (document.getSelection().toString() == "magna")
+                output += 'PASS: Correct Selection';
+            else
+                output += 'FAIL: failed to select a word as a result of a long press. Incorrect Selection: ' + document.getSelection().toString();
+            output += '<br>';
+
+            var noneditableElement = document.getElementById('noneditable');
+            noneditableElement.parentNode.removeChild(noneditableElement);
+            var editableElement = document.getElementById('editable');
+            editableElement.parentNode.removeChild(editableElement);
+            document.getElementById('target').innerHTML = output;
+            testRunner.notifyDone();
+        }
+
+        window.addEventListener('load', runTest, false);
+    </script>
+        <style>
+        #noneditable {
+            height: 200px;
+            width: 300px;
+            background-color: silver;
+            font-family: monospace;
+            font-size: 18px;
+        }
+        #editable {
+            height: 200px;
+            width: 300px;
+            background-color: silver;
+            font-family: monospace;
+            font-size: 18px;
+        }
+        #target {
+            height: 50px;
+            width: 300px;
+            background-color: silver;
+            font-family: monospace;
+            font-size: 18px;
+        }
+    </style>
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+</head>
+<body>
+    <div id="noneditable">
+        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore <span id="firstSelection">magna</span> aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</p>
+    </div>
+    <div id="editable" contenteditable _onmousedown_="clearSelection()">
+        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.</p>
+    </div> 
+    <div id="target">
+        This test requires UIScriptController to run.
+    </div>
+</body>
+</html>

Modified: trunk/Source/WebKit/ChangeLog (271385 => 271386)


--- trunk/Source/WebKit/ChangeLog	2021-01-12 00:11:48 UTC (rev 271385)
+++ trunk/Source/WebKit/ChangeLog	2021-01-12 00:54:36 UTC (rev 271386)
@@ -1,3 +1,27 @@
+2021-01-11  Megan Gardner  <[email protected]>
+
+        Double tap to select does not work if the page clears selections on tap, like grammarly.com does
+        https://bugs.webkit.org/show_bug.cgi?id=220454
+        rdar://67757411
+
+        Reviewed by Wenson Hsieh.
+
+        Double tap to select content did not work on grammerly.com on iPad because this is a desktop website run on a touch-based device,
+        and there was a script running that would clear the selection on a mouseDown event. When we would send synthetic click events to the page
+        we would first update the selection and then on completion of the tap, we would dispatch a mouseDown and mouseUp event on iOS. On Mac, we
+        update the selection between the mouseDown and the mouseUp, so in order to bring us more in line with mac/mouse behaviors, we are now saving
+        the information needed to update the selection when we believe we are in the middle of a potential tap, and then setting the selection between
+        these two events. This makes for a more expected change of events, and does not let grammerly.com clear a valid selection based off of synthetic 
+        clicks.
+
+        Test: fast/events/touch/ios/double-tap-on-editable-content-for-selection-with-clear-on-touch.html
+
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::completeSyntheticClick):
+        (WebKit::WebPage::setSelectionRange):
+        (WebKit::WebPage::selectTextWithGranularityAtPoint):
+
 2021-01-11  Kate Cheney  <[email protected]>
 
         Crash in pageDidComputePageRects()

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (271385 => 271386)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2021-01-12 00:11:48 UTC (rev 271385)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2021-01-12 00:54:36 UTC (rev 271386)
@@ -1798,6 +1798,8 @@
 
     Vector<RefPtr<SandboxExtension>> consumeSandboxExtensions(SandboxExtension::HandleArray&&);
     void revokeSandboxExtensions(Vector<RefPtr<SandboxExtension>>& sandboxExtensions);
+
+    void setSelectionRange(const WebCore::IntPoint&, WebCore::TextGranularity, bool);
     
     WebCore::PageIdentifier m_identifier;
 
@@ -1825,6 +1827,7 @@
     HashMap<PDFPluginIdentifier, WeakPtr<PDFPlugin>> m_pdfPlugInsWithHUD;
 #endif
 
+    WTF::Function<void()> m_selectionChangedHandler;
     bool m_isInRedo { false };
     bool m_isClosed { false };
     bool m_tabToLinks { false };

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (271385 => 271386)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-01-12 00:11:48 UTC (rev 271385)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-01-12 00:54:36 UTC (rev 271386)
@@ -853,6 +853,9 @@
     if (m_isClosed)
         return;
 
+    if (auto selectionChangedHandler = std::exchange(m_selectionChangedHandler, {}))
+        selectionChangedHandler();
+
     tapWasHandled |= mainframe.eventHandler().handleMouseReleaseEvent(PlatformMouseEvent(roundedAdjustedPoint, roundedAdjustedPoint, LeftButton, PlatformEvent::MouseReleased, 1, shiftKey, ctrlKey, altKey, metaKey, WallTime::now(), WebCore::ForceAtClick, syntheticClickType, pointerId));
     if (m_isClosed)
         return;
@@ -1166,6 +1169,9 @@
 
 void WebPage::commitPotentialTapFailed()
 {
+    if (auto selectionChangedHandler = std::exchange(m_selectionChangedHandler, {}))
+        selectionChangedHandler();
+
     ContentChangeObserver::didCancelPotentialTap(m_page->mainFrame());
     if (!m_page->focusController().focusedOrMainFrame().selection().selection().isContentEditable())
         clearSelection();
@@ -1182,6 +1188,9 @@
 
 void WebPage::cancelPotentialTapInFrame(WebFrame& frame)
 {
+    if (auto selectionChangedHandler = std::exchange(m_selectionChangedHandler, {}))
+        selectionChangedHandler();
+
     if (m_potentialTapNode) {
         auto* potentialTapFrame = m_potentialTapNode->document().frame();
         if (potentialTapFrame && !potentialTapFrame->tree().isDescendantOf(frame.coreFrame()))
@@ -2111,7 +2120,7 @@
         m_page->focusController().setFocusedFrame(result.innerNodeFrame());
 }
 
-void WebPage::selectTextWithGranularityAtPoint(const WebCore::IntPoint& point, WebCore::TextGranularity granularity, bool isInteractingWithFocusedElement, CompletionHandler<void()>&& completionHandler)
+void WebPage::setSelectionRange(const WebCore::IntPoint& point, WebCore::TextGranularity granularity, bool isInteractingWithFocusedElement)
 {
     setFocusedFrameBeforeSelectingTextAtLocation(point);
 
@@ -2120,9 +2129,32 @@
     if (range)
         frame.selection().setSelectedRange(*range, Affinity::Upstream, WebCore::FrameSelection::ShouldCloseTyping::Yes, UserTriggered);
     m_initialSelection = range;
-    completionHandler();
 }
 
+void WebPage::selectTextWithGranularityAtPoint(const WebCore::IntPoint& point, WebCore::TextGranularity granularity, bool isInteractingWithFocusedElement, CompletionHandler<void()>&& completionHandler)
+{
+    if (!m_potentialTapNode) {
+        setSelectionRange(point, granularity, isInteractingWithFocusedElement);
+        completionHandler();
+        return;
+    }
+    
+    ASSERT(!m_selectionChangedHandler);
+    if (auto selectionChangedHandler = std::exchange(m_selectionChangedHandler, {}))
+        selectionChangedHandler();
+
+    m_selectionChangedHandler = [point, granularity, isInteractingWithFocusedElement, completionHandler = WTFMove(completionHandler), webPage = makeWeakPtr(*this), this]() mutable {
+        RefPtr<WebPage> strongPage = webPage.get();
+        if (!strongPage) {
+            completionHandler();
+            return;
+        }
+        setSelectionRange(point, granularity, isInteractingWithFocusedElement);
+        completionHandler();
+    };
+
+}
+
 void WebPage::beginSelectionInDirection(WebCore::SelectionDirection direction, CallbackID callbackID)
 {
     m_selectionAnchor = direction == SelectionDirection::Left ? Start : End;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to