[iOS] -updateSelectionWithExtentPoint:completionHandler: should work without requirin...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Feb 2020 18:10:41 +0000 (18:10 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Feb 2020 18:10:41 +0000 (18:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207680
<rdar://problem/59340940>

Reviewed by Tim Horton.

Source/WebKit:

Currently, WebPage::updateSelectionWithExtentPoint consults the value of m_selectionAnchor to determine whether
it should attempt to modify the current selection using the hit-tested visible position. m_selectionAnchor is
only set under WebPage::beginSelectionInDirection, which is only invoked when the user begins a floating cursor
gesture. When attempting to perform an out-of-band selection update (i.e. without calling
beginSelectionInDirection beforehand), we will end up consulting an arbitrary value for m_selectionAnchor
(::Start by default; otherwise, the last value set by beginSelectionInDirection if it was previously called).
This means that the selection can often only be extended in one direction (typically forwards) when an API
client attempts to use -updateSelectionWithExtentPoint:completionHandler: to extend the current selection.

To fix this, make it so that we only respect the selection anchor (m_selectionAnchor) in the case where the user
is currently using the floating cursor; otherwise, allow the SPI to expand the selection, such that it contains
the visible position for the given location.

Test: UIWKInteractionViewProtocol.UpdateSelectionWithExtentPoint

* Platform/spi/ios/UIKitSPI.h:
* Scripts/webkit/messages.py:
* Shared/ios/GestureTypes.h:

Add a new flag to tell the web process whether it should limit selection extent updates to the current selection
anchor. Also, remove an existing enum type, SelectionHandlePosition, that is unused (since we no longer support
block text selection).

* UIProcess/WebPageProxy.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[UITextInteractionAssistant _wk_hasFloatingCursor]):

Add a helper method to determine (using the text interaction assistant) whether there's an active floating
cursor gesture. In the case where floating cursor is active, the text interaction assistant will be in an active
gesture but its UITextInteraction will not, since the gesture recognizer belongs to the keyboard or input view
rather than the first responder (in this case, WKContentView).

(-[WKContentView updateSelectionWithExtentPoint:completionHandler:]):

Only respect the selection anchor if we're in floating cursor mode; otherwise, allow selection updates with an
extent point to extend the current selection to include the new position.

* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::updateSelectionWithExtentPoint):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:

Plumb the RespectsSelectionAnchor flag over to the web process.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::updateSelectionWithExtentPoint):

Tools:

Add a new API test to verify that calling -updateSelectionWithExtentPoint:completionHandler: with a point behind
the current selection works.

* TestWebKitAPI/Tests/ios/UIWKInteractionViewProtocol.mm:
(-[TestWKWebView updateSelectionWithExtentPoint:]):
* TestWebKitAPI/ios/UIKitSPI.h:

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

13 files changed:
Source/WebKit/ChangeLog
Source/WebKit/Platform/spi/ios/UIKitSPI.h
Source/WebKit/Scripts/webkit/messages.py
Source/WebKit/Shared/ios/GestureTypes.h
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/ios/UIWKInteractionViewProtocol.mm
Tools/TestWebKitAPI/ios/UIKitSPI.h

index 0cf7db1..719647f 100644 (file)
@@ -1,3 +1,58 @@
+2020-02-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] -updateSelectionWithExtentPoint:completionHandler: should work without requiring floating cursor
+        https://bugs.webkit.org/show_bug.cgi?id=207680
+        <rdar://problem/59340940>
+
+        Reviewed by Tim Horton.
+
+        Currently, WebPage::updateSelectionWithExtentPoint consults the value of m_selectionAnchor to determine whether
+        it should attempt to modify the current selection using the hit-tested visible position. m_selectionAnchor is
+        only set under WebPage::beginSelectionInDirection, which is only invoked when the user begins a floating cursor
+        gesture. When attempting to perform an out-of-band selection update (i.e. without calling
+        beginSelectionInDirection beforehand), we will end up consulting an arbitrary value for m_selectionAnchor
+        (::Start by default; otherwise, the last value set by beginSelectionInDirection if it was previously called).
+        This means that the selection can often only be extended in one direction (typically forwards) when an API
+        client attempts to use -updateSelectionWithExtentPoint:completionHandler: to extend the current selection.
+
+        To fix this, make it so that we only respect the selection anchor (m_selectionAnchor) in the case where the user
+        is currently using the floating cursor; otherwise, allow the SPI to expand the selection, such that it contains
+        the visible position for the given location.
+
+        Test: UIWKInteractionViewProtocol.UpdateSelectionWithExtentPoint
+
+        * Platform/spi/ios/UIKitSPI.h:
+        * Scripts/webkit/messages.py:
+        * Shared/ios/GestureTypes.h:
+
+        Add a new flag to tell the web process whether it should limit selection extent updates to the current selection
+        anchor. Also, remove an existing enum type, SelectionHandlePosition, that is unused (since we no longer support
+        block text selection).
+
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[UITextInteractionAssistant _wk_hasFloatingCursor]):
+
+        Add a helper method to determine (using the text interaction assistant) whether there's an active floating
+        cursor gesture. In the case where floating cursor is active, the text interaction assistant will be in an active
+        gesture but its UITextInteraction will not, since the gesture recognizer belongs to the keyboard or input view
+        rather than the first responder (in this case, WKContentView).
+
+        (-[WKContentView updateSelectionWithExtentPoint:completionHandler:]):
+
+        Only respect the selection anchor if we're in floating cursor mode; otherwise, allow selection updates with an
+        extent point to extend the current selection to include the new position.
+
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::updateSelectionWithExtentPoint):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+
+        Plumb the RespectsSelectionAnchor flag over to the web process.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::updateSelectionWithExtentPoint):
+
 2020-02-13  Said Abou-Hallawa  <said@apple.com>
 
         Unreviewed, rolling out r255158, 255405 and r255486
index b30670d..3febaa4 100644 (file)
@@ -453,6 +453,8 @@ typedef enum {
 - (void)selectionChanged;
 - (void)setGestureRecognizers;
 - (void)willStartScrollingOverflow;
+- (void)willStartScrollingOrZooming;
+- (void)didEndScrollingOrZooming;
 @end
 
 @interface UITextSuggestion : NSObject
@@ -1110,9 +1112,13 @@ typedef NS_OPTIONS(NSInteger, UIWKDocumentRequestFlags) {
 
 #define UIWKDocumentRequestMarkedTextRects (1 << 5)
 
-@interface UITextInteractionAssistant (Staging_55645619)
-- (void)didEndScrollingOrZooming;
-- (void)willStartScrollingOrZooming;
+@interface UITextInteractionAssistant (IPI)
+@property (nonatomic, readonly) BOOL inGesture;
+@property (nonatomic, readonly) UITextInteraction *interactions;
+@end
+
+@interface UITextInteraction (IPI)
+@property (nonatomic, readonly) BOOL inGesture;
 @end
 
 #if HAVE(LINK_PREVIEW) && USE(UICONTEXTMENU)
index 82db012..c8200de 100644 (file)
@@ -614,6 +614,7 @@ def headers_for_type(type):
         'WebKit::LayerHostingContextID': ['"LayerHostingContext.h"'],
         'WebKit::LayerHostingMode': ['"LayerTreeContext.h"'],
         'WebKit::PageState': ['"SessionState.h"'],
+        'WebKit::RespectSelectionAnchor': ['"GestureTypes.h"'],
         'WebKit::WebGestureEvent': ['"WebEvent.h"'],
         'WebKit::WebKeyboardEvent': ['"WebEvent.h"'],
         'WebKit::WebMouseEvent': ['"WebEvent.h"'],
index 686f0d8..9c365eb 100644 (file)
@@ -75,11 +75,9 @@ enum SelectionFlags {
     PhraseBoundaryChanged = 1 << 1,
 };
 
-enum class SelectionHandlePosition {
-    Top,
-    Right,
-    Bottom,
-    Left
+enum class RespectSelectionAnchor : bool {
+    No,
+    Yes
 };
 
 } // namespace WebKit
index a670856..7dfcfa3 100644 (file)
 #include <wtf/text/WTFString.h>
 
 #if PLATFORM(IOS_FAMILY)
+#include "GestureTypes.h"
 #include "WebAutocorrectionContext.h"
 #endif
 
@@ -740,7 +741,7 @@ public:
     void selectPositionAtBoundaryWithDirection(const WebCore::IntPoint, WebCore::TextGranularity, WebCore::SelectionDirection, bool isInteractingWithFocusedElement, WTF::Function<void(CallbackBase::Error)>&&);
     void moveSelectionAtBoundaryWithDirection(WebCore::TextGranularity, WebCore::SelectionDirection, WTF::Function<void(CallbackBase::Error)>&&);
     void beginSelectionInDirection(WebCore::SelectionDirection, WTF::Function<void (uint64_t, CallbackBase::Error)>&&);
-    void updateSelectionWithExtentPoint(const WebCore::IntPoint, bool isInteractingWithFocusedElement, WTF::Function<void(uint64_t, CallbackBase::Error)>&&);
+    void updateSelectionWithExtentPoint(const WebCore::IntPoint, bool isInteractingWithFocusedElement, RespectSelectionAnchor, WTF::Function<void(uint64_t, CallbackBase::Error)>&&);
     void updateSelectionWithExtentPointAndBoundary(const WebCore::IntPoint, WebCore::TextGranularity, bool isInteractingWithFocusedElement, WTF::Function<void(uint64_t, CallbackBase::Error)>&&);
     void requestAutocorrectionData(const String& textForAutocorrection, CompletionHandler<void(WebAutocorrectionData)>&&);
     void applyAutocorrection(const String& correction, const String& originalText, WTF::Function<void (const String&, CallbackBase::Error)>&&);
index f4c095d..e1c2bb8 100644 (file)
@@ -313,6 +313,10 @@ constexpr double fasterTapSignificantZoomThreshold = 0.8;
 - (void)selectWord;
 @end
 
+@interface UITextInteractionAssistant (WebKit)
+@property (nonatomic, readonly) BOOL _wk_hasFloatingCursor;
+@end
+
 @interface UIView (UIViewInternalHack)
 + (BOOL)_addCompletion:(void(^)(BOOL))completion;
 @end
@@ -323,6 +327,15 @@ constexpr double fasterTapSignificantZoomThreshold = 0.8;
 - (instancetype)initWithFocusedElementInformation:(const WebKit::FocusedElementInformation&)information isUserInitiated:(BOOL)isUserInitiated userObject:(NSObject <NSSecureCoding> *)userObject;
 @end
 
+@implementation UITextInteractionAssistant (WebKit)
+
+- (BOOL)_wk_hasFloatingCursor
+{
+    return self.inGesture && !self.interactions.inGesture;
+}
+
+@end
+
 @implementation WKFormInputSession {
     WeakObjCPtr<WKContentView> _contentView;
     RetainPtr<WKFocusedElementInfo> _focusedElementInfo;
@@ -4070,7 +4083,8 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
 {
     UIWKSelectionWithDirectionCompletionHandler selectionHandler = [completionHandler copy];
     
-    _page->updateSelectionWithExtentPoint(WebCore::IntPoint(point), [self _isInteractingWithFocusedElement], [selectionHandler](bool endIsMoving, WebKit::CallbackBase::Error error) {
+    auto respectSelectionAnchor = self.interactionAssistant._wk_hasFloatingCursor ? WebKit::RespectSelectionAnchor::Yes : WebKit::RespectSelectionAnchor::No;
+    _page->updateSelectionWithExtentPoint(WebCore::IntPoint(point), self._isInteractingWithFocusedElement, respectSelectionAnchor, [selectionHandler](bool endIsMoving, WebKit::CallbackBase::Error error) {
         selectionHandler(endIsMoving);
         [selectionHandler release];
     });
index 34d23d4..2250c34 100644 (file)
@@ -542,7 +542,7 @@ void WebPageProxy::beginSelectionInDirection(WebCore::SelectionDirection directi
     m_process->send(Messages::WebPage::BeginSelectionInDirection(direction, callbackID), m_webPageID);
 }
 
-void WebPageProxy::updateSelectionWithExtentPoint(const WebCore::IntPoint point, bool isInteractingWithFocusedElement, WTF::Function<void(uint64_t, CallbackBase::Error)>&& callbackFunction)
+void WebPageProxy::updateSelectionWithExtentPoint(const WebCore::IntPoint point, bool isInteractingWithFocusedElement, RespectSelectionAnchor respectSelectionAnchor, WTF::Function<void(uint64_t, CallbackBase::Error)>&& callbackFunction)
 {
     if (!hasRunningProcess()) {
         callbackFunction(0, CallbackBase::Error::Unknown);
@@ -550,7 +550,7 @@ void WebPageProxy::updateSelectionWithExtentPoint(const WebCore::IntPoint point,
     }
     
     auto callbackID = m_callbacks.put(WTFMove(callbackFunction), m_process->throttler().backgroundActivity("WebPageProxy::updateSelectionWithExtentPoint"_s));
-    m_process->send(Messages::WebPage::UpdateSelectionWithExtentPoint(point, isInteractingWithFocusedElement, callbackID), m_webPageID);
+    m_process->send(Messages::WebPage::UpdateSelectionWithExtentPoint(point, isInteractingWithFocusedElement, respectSelectionAnchor, callbackID), m_webPageID);
     
 }
 
index f98e946..bdab81b 100644 (file)
@@ -674,7 +674,7 @@ public:
     void moveSelectionAtBoundaryWithDirection(uint32_t granularity, uint32_t direction, CallbackID);
     void selectPositionAtPoint(const WebCore::IntPoint&, bool isInteractingWithFocusedElement, CallbackID);
     void beginSelectionInDirection(uint32_t direction, CallbackID);
-    void updateSelectionWithExtentPoint(const WebCore::IntPoint&, bool isInteractingWithFocusedElement, CallbackID);
+    void updateSelectionWithExtentPoint(const WebCore::IntPoint&, bool isInteractingWithFocusedElement, RespectSelectionAnchor, CallbackID);
     void updateSelectionWithExtentPointAndBoundary(const WebCore::IntPoint&, uint32_t granularity, bool isInteractingWithFocusedElement, CallbackID);
 
     void requestDictationContext(CallbackID);
index ab97b2b..1b4618b 100644 (file)
@@ -74,7 +74,7 @@ messages -> WebPage LegacyReceiver {
     MoveSelectionAtBoundaryWithDirection(uint32_t granularity, uint32_t direction, WebKit::CallbackID callbackID)
     SelectPositionAtPoint(WebCore::IntPoint point, bool isInteractingWithFocusedElement, WebKit::CallbackID callbackID)
     BeginSelectionInDirection(uint32_t direction, WebKit::CallbackID callbackID)
-    UpdateSelectionWithExtentPoint(WebCore::IntPoint point, bool isInteractingWithFocusedElement, WebKit::CallbackID callbackID)
+    UpdateSelectionWithExtentPoint(WebCore::IntPoint point, bool isInteractingWithFocusedElement, enum:bool WebKit::RespectSelectionAnchor respectSelectionAnchor, WebKit::CallbackID callbackID)
     UpdateSelectionWithExtentPointAndBoundary(WebCore::IntPoint point, uint32_t granularity, bool isInteractingWithFocusedElement, WebKit::CallbackID callbackID)
     RequestDictationContext(WebKit::CallbackID callbackID)
     ReplaceDictatedText(String oldText, String newText)
index 220ed3b..f4bb892 100644 (file)
@@ -34,7 +34,6 @@
 #import "DrawingArea.h"
 #import "EditingRange.h"
 #import "EditorState.h"
-#import "GestureTypes.h"
 #import "InteractionInformationAtPosition.h"
 #import "Logging.h"
 #import "NativeWebKeyboardEvent.h"
@@ -2135,7 +2134,7 @@ void WebPage::updateSelectionWithExtentPointAndBoundary(const WebCore::IntPoint&
     send(Messages::WebPageProxy::UnsignedCallback(selectionStart == m_initialSelection->startPosition(), callbackID));
 }
 
-void WebPage::updateSelectionWithExtentPoint(const WebCore::IntPoint& point, bool isInteractingWithFocusedElement, CallbackID callbackID)
+void WebPage::updateSelectionWithExtentPoint(const WebCore::IntPoint& point, bool isInteractingWithFocusedElement, RespectSelectionAnchor respectSelectionAnchor, CallbackID callbackID)
 {
     auto& frame = m_page->focusController().focusedOrMainFrame();
     VisiblePosition position = visiblePositionInFocusedNodeForPoint(frame, point, isInteractingWithFocusedElement);
@@ -2149,23 +2148,33 @@ void WebPage::updateSelectionWithExtentPoint(const WebCore::IntPoint& point, boo
     VisiblePosition selectionStart;
     VisiblePosition selectionEnd;
     
-    if (m_selectionAnchor == Start) {
-        selectionStart = frame.selection().selection().visibleStart();
-        selectionEnd = position;
-
-        if (position <= selectionStart) {
-            selectionStart = selectionStart.previous();
+    if (respectSelectionAnchor == RespectSelectionAnchor::Yes) {
+        if (m_selectionAnchor == Start) {
+            selectionStart = frame.selection().selection().visibleStart();
+            selectionEnd = position;
+            if (position <= selectionStart) {
+                selectionStart = selectionStart.previous();
+                selectionEnd = frame.selection().selection().visibleEnd();
+                m_selectionAnchor = End;
+            }
+        } else {
+            selectionStart = position;
             selectionEnd = frame.selection().selection().visibleEnd();
-            m_selectionAnchor = End;
+            if (position >= selectionEnd) {
+                selectionStart = frame.selection().selection().visibleStart();
+                selectionEnd = selectionEnd.next();
+                m_selectionAnchor = Start;
+            }
         }
     } else {
-        selectionStart = position;
-        selectionEnd = frame.selection().selection().visibleEnd();
-        
-        if (position >= selectionEnd) {
-            selectionStart = frame.selection().selection().visibleStart();
-            selectionEnd = selectionEnd.next();
-            m_selectionAnchor = Start;
+        auto currentStart = frame.selection().selection().visibleStart();
+        auto currentEnd = frame.selection().selection().visibleEnd();
+        if (position <= currentStart) {
+            selectionStart = position;
+            selectionEnd = currentEnd;
+        } else if (position >= currentEnd) {
+            selectionStart = currentStart;
+            selectionEnd = position;
         }
     }
     
index 30ee285..7e8e95f 100644 (file)
@@ -1,3 +1,18 @@
+2020-02-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] -updateSelectionWithExtentPoint:completionHandler: should work without requiring floating cursor
+        https://bugs.webkit.org/show_bug.cgi?id=207680
+        <rdar://problem/59340940>
+
+        Reviewed by Tim Horton.
+
+        Add a new API test to verify that calling -updateSelectionWithExtentPoint:completionHandler: with a point behind
+        the current selection works.
+
+        * TestWebKitAPI/Tests/ios/UIWKInteractionViewProtocol.mm:
+        (-[TestWKWebView updateSelectionWithExtentPoint:]):
+        * TestWebKitAPI/ios/UIKitSPI.h:
+
 2020-02-13  Said Abou-Hallawa  <said@apple.com>
 
         Unreviewed, rolling out r255158, 255405 and r255486
index 250db19..72f4226 100644 (file)
@@ -34,6 +34,7 @@
 
 @interface TestWKWebView (UIWKInteractionViewTesting)
 - (void)selectTextWithGranularity:(UITextGranularity)granularity atPoint:(CGPoint)point;
+- (void)updateSelectionWithExtentPoint:(CGPoint)point;
 - (void)updateSelectionWithExtentPoint:(CGPoint)point withBoundary:(UITextGranularity)granularity;
 @end
 
     TestWebKitAPI::Util::run(&done);
 }
 
+- (void)updateSelectionWithExtentPoint:(CGPoint)point
+{
+    __block bool done = false;
+    [self.textInputContentView updateSelectionWithExtentPoint:point completionHandler:^(BOOL) {
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+}
+
 - (void)updateSelectionWithExtentPoint:(CGPoint)point withBoundary:(UITextGranularity)granularity
 {
     __block bool done = false;
@@ -68,4 +78,18 @@ TEST(UIWKInteractionViewProtocol, SelectTextWithCharacterGranularity)
     EXPECT_WK_STREQ("Hello world", [webView stringByEvaluatingJavaScript:@"getSelection().toString()"]);
 }
 
+TEST(UIWKInteractionViewProtocol, UpdateSelectionWithExtentPoint)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400)]);
+    [webView synchronouslyLoadHTMLString:@"<body contenteditable style='font-size: 20px;'>Hello world</body>"];
+
+    [webView evaluateJavaScript:@"getSelection().setPosition(document.body, 1)" completionHandler:nil];
+    [webView updateSelectionWithExtentPoint:CGPointMake(5, 20)];
+    EXPECT_WK_STREQ("Hello world", [webView stringByEvaluatingJavaScript:@"getSelection().toString()"]);
+
+    [webView evaluateJavaScript:@"getSelection().setPosition(document.body, 0)" completionHandler:nil];
+    [webView updateSelectionWithExtentPoint:CGPointMake(300, 20)];
+    EXPECT_WK_STREQ("Hello world", [webView stringByEvaluatingJavaScript:@"getSelection().toString()"]);
+}
+
 #endif
index 75f9a03..f276319 100644 (file)
@@ -175,6 +175,7 @@ typedef NS_OPTIONS(NSInteger, UIWKDocumentRequestFlags) {
 - (void)requestAutocorrectionContextWithCompletionHandler:(void (^)(UIWKAutocorrectionContext *autocorrectionContext))completionHandler;
 - (void)selectWordBackward;
 - (void)selectTextWithGranularity:(UITextGranularity)granularity atPoint:(CGPoint)point completionHandler:(void (^)(void))completionHandler;
+- (void)updateSelectionWithExtentPoint:(CGPoint)point completionHandler:(void (^)(BOOL selectionEndIsMoving))completionHandler;
 - (void)updateSelectionWithExtentPoint:(CGPoint)point withBoundary:(UITextGranularity)granularity completionHandler:(void (^)(BOOL selectionEndIsMoving))completionHandler;
 @property (nonatomic, readonly) NSString *selectedText;
 @end