Attachment 369415 Details for Bug 197686 – Patch

[patch] Patch

bug-197686-20190508132952.patch (text/plain), 16.46 KB, created by Tim Horton on 2019-05-08 13:29:52 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Horton

Created: 2019-05-08 13:29:52 PDT

Size: 16.46 KB

patch

obsolete

>Subversion Revision: 245064
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 94d1685284bafdba8a885f18b32031e81954e771..58854dfd64b5697a86e9430634350c6ef792200a 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,28 @@
>+2019-05-07  Tim Horton  <timothy_horton@apple.com>
>+
>+        iOS: Selection is dismissed even if click is preventDefault()'d
>+        https://bugs.webkit.org/show_bug.cgi?id=197686
>+        <rdar://problem/49398824>
>+
>+        Reviewed by Wenson Hsieh.
>+
>+        We currently unconditionally dismiss the selection on any tap; however
>+        if a site preventDefault()s on click, we shouldn't perform the default
>+        action of dismissing the selection.
>+
>+        Instead of clearing the selection in the UI process, clear it in the
>+        Web content process if we don't dispatch a synthetic click; the normal
>+        WebCore machinery will handle it in the case that we do.
>+
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView _singleTapRecognized:]):
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::commitPotentialTapFailed):
>+        (WebKit::WebPage::selectWithGesture):
>+        (WebKit::WebPage::clearSelection):
>+        (WebKit::WebPage::selectTextWithGranularityAtPoint):
>+
> 2019-05-08  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] Add a quirk to synthesize mouse events when modifying the selection
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index aed5449e890a5075cf4e0f373c9a661a10146a98..9ab06d101695dbdca458955d43dc4fc02c3d0d9f 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -2348,12 +2348,6 @@ - (void)_singleTapRecognized:(UITapGestureRecognizer *)gestureRecognizer
> 
>     ASSERT(_potentialTapInProgress);
> 
>-    // We don't want to clear the selection if it is in editable content.
>-    // The selection could have been set by autofocusing on page load and not
>-    // reflected in the UI process since the user was not interacting with the page.
>-    if (!_page->editorState().isContentEditable)
>-        _page->clearSelection();
>-
>     _lastInteractionLocation = gestureRecognizer.location;
> 
>     [self _endPotentialTapAndEnableDoubleTapGesturesIfNecessary];
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index 6eccb3e9bbe9e4202bfdb5eb92e8f572b4a55e1b..4b07cdef132e82ca81336ef4928cecf8732ac0c1 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -1818,7 +1818,6 @@ private:
>     WebCore::FloatSize m_screenSize;
>     WebCore::FloatSize m_availableScreenSize;
>     WebCore::FloatSize m_overrideScreenSize;
>-    RefPtr<WebCore::Range> m_currentBlockSelection;
>     WebCore::IntRect m_blockRectForTextSelection;
> 
>     RefPtr<WebCore::Range> m_initialSelection;
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 0d05744c9beb6f02f04c7afc1cf4c982f638132e..8f4047e9c2775b47cc26d4b597ed4d00e4b30368 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -947,6 +947,9 @@ void WebPage::commitPotentialTap(OptionSet<WebEvent::Modifier> modifiers, uint64
> 
> void WebPage::commitPotentialTapFailed()
> {
>+    if (!m_page->focusController().focusedOrMainFrame().selection().selection().isContentEditable())
>+        clearSelection();
>+
>     send(Messages::WebPageProxy::CommitPotentialTapFailed());
>     send(Messages::WebPageProxy::DidNotHandleTapAsClick(roundedIntPoint(m_potentialTapLocation)));
> }
>@@ -1367,7 +1370,6 @@ void WebPage::selectWithGesture(const IntPoint& point, uint32_t granularity, uin
>         if (wkGestureState == GestureRecognizerState::Began) {
>             m_blockSelectionDesiredSize.setWidth(blockSelectionStartWidth);
>             m_blockSelectionDesiredSize.setHeight(blockSelectionStartHeight);
>-            m_currentBlockSelection = nullptr;
>         }
>         range = rangeForWebSelectionAtPosition(point, position, flags);
>         break;
>@@ -1499,7 +1501,6 @@ static IntRect elementRectInRootViewCoordinates(const Element& element)
> void WebPage::clearSelection()
> {
>     m_startingGestureRange = nullptr;
>-    m_currentBlockSelection = nullptr;
>     m_page->focusController().focusedOrMainFrame().selection().clear();
> }
> 
>@@ -1958,7 +1959,6 @@ void WebPage::selectTextWithGranularityAtPoint(const WebCore::IntPoint& point, u
>     if (!isInteractingWithFocusedElement) {
>         m_blockSelectionDesiredSize.setWidth(blockSelectionStartWidth);
>         m_blockSelectionDesiredSize.setHeight(blockSelectionStartHeight);
>-        m_currentBlockSelection = nullptr;
>         auto* renderer = range ? range->startContainer().renderer() : nullptr;
>         if (renderer && renderer->style().preserveNewline())
>             m_blockRectForTextSelection = renderer->absoluteBoundingBoxRect(true);
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f32b5a20b8f25d234408b9c7c43bb5db7fe46b9d..a71418d08b47d5d5513e369f9481c66430aa969f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-05-08  Tim Horton  <timothy_horton@apple.com>
>+
>+        iOS: Selection is dismissed even if click is preventDefault()'d
>+        https://bugs.webkit.org/show_bug.cgi?id=197686
>+        <rdar://problem/49398824>
>+
>+        Reviewed by Wenson Hsieh.
>+
>+        * editing/selection/ios/clear-selection-after-tapping-on-element-with-no-click-handler-expected.txt: Added.
>+        * editing/selection/ios/clear-selection-after-tapping-on-element-with-no-click-handler.html: Added.
>+        * editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt: Added.
>+        * editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html: Added.
>+        New tests.
>+
>+        * platform/ios/editing/deleting/smart-delete-paragraph-003-expected.txt:
>+        * platform/ios/editing/pasteboard/smart-paste-paragraph-003-expected.txt:
>+        Rebaseline since we changed the timing of editing callbacks by changing where the selection happens.
>+
> 2019-05-08  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] Add a quirk to synthesize mouse events when modifying the selection
>diff --git a/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-no-click-handler-expected.txt b/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-no-click-handler-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..6e639dbca0afca6cc4c5272fc9b6d6f40d09a762
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-no-click-handler-expected.txt
>@@ -0,0 +1,3 @@
>+WebKit
>+The selected text is: ""
>+This test verifies that the DOM selection is dismissed when tapping on an element that does not listen to click events.
>diff --git a/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-no-click-handler.html b/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-no-click-handler.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..2c8197cd95fa313a63347346cde2d99d154f404b
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/clear-selection-after-tapping-on-element-with-no-click-handler.html
>@@ -0,0 +1,63 @@
>+<!DOCTYPE html> 
>+<html>
>+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
>+<head>
>+    <script src="../../../resources/basic-gestures.js"></script>
>+    <script src="../../../resources/ui-helper.js"></script>
>+    <style>
>+    body {
>+        margin: 0;
>+    }
>+
>+    #target {
>+        font-size: 100px;
>+    }
>+
>+    #clickTarget {
>+        width: 100px;
>+        height: 100px;
>+    }
>+    </style>
>+    <script>
>+    if (window.testRunner) {
>+        testRunner.dumpAsText();
>+        testRunner.waitUntilDone();
>+    }
>+
>+    async function run()
>+    {
>+        if (!window.testRunner)
>+            return;
>+
>+        function didChangeSelection()
>+        {
>+            result.textContent = window.getSelection().toString()
>+        }
>+
>+        document.addEventListener("selectionchange", didChangeSelection);
>+
>+        var clickTarget = document.getElementById("clickTarget");
>+
>+        var target = document.getElementById("target");        
>+        window.getSelection().setBaseAndExtent(target, 0, target, 6);
>+
>+        await UIHelper.activateElement(clickTarget);
>+
>+        setTimeout(async function () {
>+            // The test is done, but we need to tap again to ensure we don't
>+            // hang the next test with a double tap.
>+            document.removeEventListener("selectionchange", didChangeSelection);
>+            await UIHelper.tapAt(10, 500);
>+
>+            testRunner.notifyDone();
>+        }, 0);
>+    }
>+    </script>
>+</head>
>+<body onload="run()">
>+    <div id="target">WebKit</div>
>+    <div id="clickTarget"></div>
>+    <pre>The selected text is: "<span id="result"></span>"</pre>
>+    <p>This test verifies that the DOM selection is dismissed when tapping on an element that does not listen to click events.</p>
>+</body>
>+</html>
>diff --git a/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..e6d97ef4fdd88f241ebc862748ce1f29546a6ab9
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler-expected.txt
>@@ -0,0 +1,3 @@
>+WebKit
>+The selected text is: "WebKit"
>+This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the click event.
>diff --git a/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..5ba42ac86dd37d2d9170dbddee38d63ada6b4a81
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/persist-selection-after-tapping-on-element-with-click-handler.html
>@@ -0,0 +1,68 @@
>+<!DOCTYPE html> 
>+<html>
>+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
>+<head>
>+    <script src="../../../resources/basic-gestures.js"></script>
>+    <script src="../../../resources/ui-helper.js"></script>
>+    <style>
>+    body {
>+        margin: 0;
>+    }
>+
>+    #target {
>+        font-size: 100px;
>+    }
>+
>+    #clickTarget {
>+        width: 100px;
>+        height: 100px;
>+        background-color: green;
>+    }
>+    </style>
>+    <script>
>+    if (window.testRunner) {
>+        testRunner.dumpAsText();
>+        testRunner.waitUntilDone();
>+    }
>+
>+    async function run()
>+    {
>+        if (!window.testRunner)
>+            return;
>+
>+        function didChangeSelection()
>+        {
>+            result.textContent = window.getSelection().toString()
>+        }
>+
>+        document.addEventListener("selectionchange", didChangeSelection);
>+
>+        var clickTarget = document.getElementById("clickTarget");
>+
>+        clickTarget.addEventListener("click", event => {
>+            event.preventDefault();
>+
>+            setTimeout(async function () {
>+                // The test is done, but we need to tap again to ensure we don't
>+                // hang the next test with a double tap.
>+                document.removeEventListener("selectionchange", didChangeSelection);
>+                await UIHelper.tapAt(10, 500);
>+
>+                testRunner.notifyDone();
>+            }, 0);
>+        });
>+
>+        var target = document.getElementById("target");        
>+        window.getSelection().setBaseAndExtent(target, 0, target, 6);
>+
>+        await UIHelper.activateElement(clickTarget);
>+    }
>+    </script>
>+</head>
>+<body onload="run()">
>+    <div id="target">WebKit</div>
>+    <div id="clickTarget"></div>
>+    <pre>The selected text is: "<span id="result"></span>"</pre>
>+    <p>This test verifies that the DOM selection is not dismissed when tapping on an element that preventDefault()s the click event.</p>
>+</body>
>+</html>
>diff --git a/LayoutTests/platform/ios/editing/deleting/smart-delete-paragraph-003-expected.txt b/LayoutTests/platform/ios/editing/deleting/smart-delete-paragraph-003-expected.txt
>index 230845a7cdedefe32781eeaed62b483ec77e9fb3..ed21c523ce20ab88ed5d3ad26c75ccbfed505724 100644
>--- a/LayoutTests/platform/ios/editing/deleting/smart-delete-paragraph-003-expected.txt
>+++ b/LayoutTests/platform/ios/editing/deleting/smart-delete-paragraph-003-expected.txt
>@@ -1,10 +1,7 @@
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 3 of BODY > HTML > #document to 3 of BODY > HTML > #document toDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment toDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment toDOMRange:range from 0 of #text > DIV > #document-fragment to 4 of #text > DIV > #document-fragment affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: shouldDeleteDOMRange:range from 0 of #text > DIV > #document-fragment to 15 of #text > DIV > #document-fragment
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>diff --git a/LayoutTests/platform/ios/editing/pasteboard/smart-paste-paragraph-003-expected.txt b/LayoutTests/platform/ios/editing/pasteboard/smart-paste-paragraph-003-expected.txt
>index 352dbbb9cd487bcce35df3f87c75fc55bef02653..0de35282084b95b1bf6c3386b183ed581c35c22c 100644
>--- a/LayoutTests/platform/ios/editing/pasteboard/smart-paste-paragraph-003-expected.txt
>+++ b/LayoutTests/platform/ios/editing/pasteboard/smart-paste-paragraph-003-expected.txt
>@@ -1,10 +1,7 @@
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 3 of BODY > HTML > #document to 3 of BODY > HTML > #document toDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment toDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > #document-fragment to 0 of DIV > #document-fragment toDOMRange:range from 0 of #text > DIV > #document-fragment to 4 of #text > DIV > #document-fragment affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification

Actions: View | Formatted Diff | Diff

Attachments on bug 197686: 369363 | 369366 | 369371 | 369374 | 369380 | 369397 | 369401 | 369412 | 369415