[iOS] Dropping in an editable element should result in a ranged selection
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 May 2019 01:03:10 +0000 (01:03 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 May 2019 01:03:10 +0000 (01:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198267
<rdar://problem/51145977>

Reviewed by Tim Horton.

Source/WebKit:

When drag and drop was first implemented for iOS in iOS 11, selection behavior when dropping into editable
elements matched that of macOS, by leaving the inserted content selected after performing the drop. However, in
other parts of the platform (e.g. Notes), both the keyboard and selection views are not shown after a drop.

Instead of matching macOS behavior, WebKit on iOS should match the rest of the platform. This is a little
tricky, since we use the selection range after a drop to create a TextIndicator snapshot when creating a drag
preview. To resolve this, we refactor some of the logic introduced in r245778 to remember the DOM range to
snapshot before collapsing the range to the end of the inserted content.

Tested by adjusting some existing API tests.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):

Remove some logic that currently presents the keyboard while the user is performing a drop that focuses an
editable element.

* WebProcess/WebPage/WebPage.h:

Add a member variable to keep track of which range should be snapshotted when generating a drop preview.

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

Collapse the selection range to the end after an edit drag (i.e., a drop in an editable area that inserted
content).

(WebKit::WebPage::computeAndSendEditDragSnapshot):

Tools:

Adjust some existing API tests that currently check for selection rects after a drop. Instead of checking for
visible selection rects, simply check for the start caret rect, as determined by WKContentView's
-selectionRange.

* TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
(TestWebKitAPI::TEST):
(makeCGRectValue): Deleted.
(checkSelectionRectsWithLogging): Deleted.
* TestWebKitAPI/cocoa/DragAndDropSimulator.h:

Replace finalSelectionRects with finalSelectionStartRect.

* TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
(-[DragAndDropSimulator _resetSimulatedState]):
(-[DragAndDropSimulator runFrom:to:additionalItemRequestLocations:]):
(-[DragAndDropSimulator finalSelectionRects]): Deleted.

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h
Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm

index 3fcbc07..a848671 100644 (file)
@@ -1,3 +1,41 @@
+2019-05-27  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Dropping in an editable element should result in a ranged selection
+        https://bugs.webkit.org/show_bug.cgi?id=198267
+        <rdar://problem/51145977>
+
+        Reviewed by Tim Horton.
+
+        When drag and drop was first implemented for iOS in iOS 11, selection behavior when dropping into editable
+        elements matched that of macOS, by leaving the inserted content selected after performing the drop. However, in
+        other parts of the platform (e.g. Notes), both the keyboard and selection views are not shown after a drop.
+
+        Instead of matching macOS behavior, WebKit on iOS should match the rest of the platform. This is a little
+        tricky, since we use the selection range after a drop to create a TextIndicator snapshot when creating a drag
+        preview. To resolve this, we refactor some of the logic introduced in r245778 to remember the DOM range to
+        snapshot before collapsing the range to the end of the inserted content.
+
+        Tested by adjusting some existing API tests.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):
+
+        Remove some logic that currently presents the keyboard while the user is performing a drop that focuses an
+        editable element.
+
+        * WebProcess/WebPage/WebPage.h:
+
+        Add a member variable to keep track of which range should be snapshotted when generating a drop preview.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::didConcludeDrop):
+        (WebKit::WebPage::didConcludeEditDrag):
+
+        Collapse the selection range to the end after an edit drag (i.e., a drop in an editable area that inserted
+        content).
+
+        (WebKit::WebPage::computeAndSendEditDragSnapshot):
+
 2019-05-27  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed Win Cairo build fix after r245796.
index 6cb5a53..89669dc 100644 (file)
@@ -5074,11 +5074,6 @@ static RetainPtr<NSObject <WKFormPeripheral>> createInputPeripheralWithView(WebK
             if (userIsInteracting)
                 return YES;
 
-#if ENABLE(DRAG_SUPPORT)
-            if (_dragDropInteractionState.isPerformingDrop())
-                return YES;
-#endif
-
             if (self.isFirstResponder || _becomingFirstResponder) {
                 // When the software keyboard is being used to enter an url, only the focus activity state is changing.
                 // In this case, auto focus on the page being navigated to should be disabled, unless a hardware
index fba06c4..681cbf5 100644 (file)
@@ -1780,6 +1780,7 @@ private:
 
 #if ENABLE(DRAG_SUPPORT) && PLATFORM(IOS_FAMILY)
     HashSet<RefPtr<WebCore::HTMLImageElement>> m_pendingImageElementsForDropSnapshot;
+    RefPtr<WebCore::Range> m_rangeForDropSnapshot;
 #endif
 
     bool m_cachedMainFrameIsPinnedToLeftSide { true };
index 22f2d40..ad2f69b 100644 (file)
@@ -827,6 +827,7 @@ void WebPage::requestAdditionalItemsForDragSession(const IntPoint& clientPositio
 
 void WebPage::didConcludeDrop()
 {
+    m_rangeForDropSnapshot = nullptr;
     m_pendingImageElementsForDropSnapshot.clear();
 }
 
@@ -839,9 +840,9 @@ void WebPage::didConcludeEditDrag()
     m_pendingImageElementsForDropSnapshot.clear();
 
     bool waitingForAnyImageToLoad = false;
-    auto& frame = m_page->focusController().focusedOrMainFrame();
-    if (auto range = frame.selection().selection().toNormalizedRange()) {
-        for (TextIterator iterator(range.get()); !iterator.atEnd(); iterator.advance()) {
+    auto frame = makeRef(m_page->focusController().focusedOrMainFrame());
+    if (auto selectionRange = frame->selection().selection().toNormalizedRange()) {
+        for (TextIterator iterator(selectionRange.get()); !iterator.atEnd(); iterator.advance()) {
             auto* node = iterator.node();
             if (!is<HTMLImageElement>(node))
                 continue;
@@ -853,6 +854,10 @@ void WebPage::didConcludeEditDrag()
                 waitingForAnyImageToLoad = true;
             }
         }
+        auto collapsedRange = Range::create(selectionRange->ownerDocument(), selectionRange->endPosition(), selectionRange->endPosition());
+        frame->selection().setSelectedRange(collapsedRange.ptr(), DOWNSTREAM, FrameSelection::ShouldCloseTyping::Yes, UserTriggered);
+
+        m_rangeForDropSnapshot = WTFMove(selectionRange);
     }
 
     if (!waitingForAnyImageToLoad)
@@ -874,8 +879,7 @@ void WebPage::computeAndSendEditDragSnapshot()
 {
     Optional<TextIndicatorData> textIndicatorData;
     static auto defaultTextIndicatorOptionsForEditDrag = TextIndicatorOptionIncludeSnapshotOfAllVisibleContentWithoutSelection | TextIndicatorOptionExpandClipBeyondVisibleRect | TextIndicatorOptionPaintAllContent | TextIndicatorOptionIncludeMarginIfRangeMatchesSelection | TextIndicatorOptionPaintBackgrounds | TextIndicatorOptionComputeEstimatedBackgroundColor| TextIndicatorOptionUseSelectionRectForSizing | TextIndicatorOptionIncludeSnapshotWithSelectionHighlight;
-    auto& frame = m_page->focusController().focusedOrMainFrame();
-    if (auto range = frame.selection().selection().toNormalizedRange()) {
+    if (auto range = std::exchange(m_rangeForDropSnapshot, nullptr)) {
         if (auto textIndicator = TextIndicator::createWithRange(*range, defaultTextIndicatorOptionsForEditDrag, TextIndicatorPresentationTransition::None, { }))
             textIndicatorData = textIndicator->data();
     }
index b34f82e..0b5dcf3 100644 (file)
@@ -1,3 +1,28 @@
+2019-05-27  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Dropping in an editable element should result in a ranged selection
+        https://bugs.webkit.org/show_bug.cgi?id=198267
+        <rdar://problem/51145977>
+
+        Reviewed by Tim Horton.
+
+        Adjust some existing API tests that currently check for selection rects after a drop. Instead of checking for
+        visible selection rects, simply check for the start caret rect, as determined by WKContentView's
+        -selectionRange.
+
+        * TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
+        (TestWebKitAPI::TEST):
+        (makeCGRectValue): Deleted.
+        (checkSelectionRectsWithLogging): Deleted.
+        * TestWebKitAPI/cocoa/DragAndDropSimulator.h:
+
+        Replace finalSelectionRects with finalSelectionStartRect.
+
+        * TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
+        (-[DragAndDropSimulator _resetSimulatedState]):
+        (-[DragAndDropSimulator runFrom:to:additionalItemRequestLocations:]):
+        (-[DragAndDropSimulator finalSelectionRects]): Deleted.
+
 2019-05-27  Oriol Brufau  <obrufau@igalia.com>
 
         [css-grid] Preserve repeat() notation when serializing declared values
index f1cc240..0876a4b 100644 (file)
@@ -117,11 +117,6 @@ static void loadTestPageAndEnsureInputSession(DragAndDropSimulator *simulator, N
     [simulator ensureInputSession];
 }
 
-static NSValue *makeCGRectValue(CGFloat x, CGFloat y, CGFloat width, CGFloat height)
-{
-    return [NSValue valueWithCGRect:CGRectMake(x, y, width, height)];
-}
-
 static void checkCGRectIsEqualToCGRectWithLogging(CGRect expected, CGRect observed)
 {
     BOOL isEqual = CGRectEqualToRect(expected, observed);
@@ -130,13 +125,6 @@ static void checkCGRectIsEqualToCGRectWithLogging(CGRect expected, CGRect observ
         NSLog(@"Expected: %@ but observed: %@", NSStringFromCGRect(expected), NSStringFromCGRect(observed));
 }
 
-static void checkSelectionRectsWithLogging(NSArray *expected, NSArray *observed)
-{
-    if (![expected isEqualToArray:observed])
-        NSLog(@"Expected selection rects: %@ but observed: %@", expected, observed);
-    EXPECT_TRUE([expected isEqualToArray:observed]);
-}
-
 static void checkRichTextTypePrecedesPlainTextType(DragAndDropSimulator *simulator)
 {
     // At least one of "com.apple.flat-rtfd" or "public.rtf" is expected to have higher precedence than "public.utf8-plain-text".
@@ -275,7 +263,7 @@ TEST(DragAndDropTests, ImageToContentEditable)
     EXPECT_TRUE([observedEventNames containsObject:@"dragenter"]);
     EXPECT_TRUE([observedEventNames containsObject:@"dragover"]);
     EXPECT_TRUE([observedEventNames containsObject:@"drop"]);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(1, 201, 215, 174) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(214, 201, 2, 174), [simulator finalSelectionStartRect]);
     checkFirstTypeIsPresentAndSecondTypeIsMissing(simulator.get(), kUTTypePNG, kUTTypeFileURL);
     checkEstimatedSize(simulator.get(), { 215, 174 });
     EXPECT_TRUE([simulator lastKnownDropProposal].precise);
@@ -320,7 +308,7 @@ TEST(DragAndDropTests, ImageInLinkToInput)
     [simulator runFrom:CGPointMake(100, 50) to:CGPointMake(100, 300)];
 
     EXPECT_WK_STREQ("https://www.apple.com/", [webView editorValue].UTF8String);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(101, 241, 2057, 232) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(2156, 241, 2, 232), [simulator finalSelectionStartRect]);
     checkSuggestedNameAndEstimatedSize(simulator.get(), @"icon.png", { 215, 174 });
     checkTypeIdentifierIsRegisteredAtIndex(simulator.get(), (__bridge NSString *)kUTTypePNG, 0);
     EXPECT_TRUE([simulator lastKnownDropProposal].precise);
@@ -390,7 +378,7 @@ TEST(DragAndDropTests, ContentEditableToContentEditable)
     EXPECT_TRUE([observedEventNames containsObject:@"dragenter"]);
     EXPECT_TRUE([observedEventNames containsObject:@"dragover"]);
     EXPECT_TRUE([observedEventNames containsObject:@"drop"]);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(1, 201, 961, 227) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(960, 201, 2, 227), [simulator finalSelectionStartRect]);
     checkRichTextTypePrecedesPlainTextType(simulator.get());
     EXPECT_TRUE([simulator lastKnownDropProposal].precise);
 
@@ -414,7 +402,7 @@ TEST(DragAndDropTests, ContentEditableToTextarea)
     EXPECT_TRUE([observedEventNames containsObject:@"dragenter"]);
     EXPECT_TRUE([observedEventNames containsObject:@"dragover"]);
     EXPECT_TRUE([observedEventNames containsObject:@"drop"]);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(101, 203, 990, 232) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(1089, 203, 2, 232), [simulator finalSelectionStartRect]);
     checkRichTextTypePrecedesPlainTextType(simulator.get());
     EXPECT_TRUE([simulator lastKnownDropProposal].precise);
 }
@@ -471,7 +459,7 @@ TEST(DragAndDropTests, ContentEditableMoveParagraphs)
     EXPECT_FALSE(firstParagraphOffset == NSNotFound);
     EXPECT_FALSE(secondParagraphOffset == NSNotFound);
     EXPECT_GT(firstParagraphOffset, secondParagraphOffset);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(190, 100, 130, 20), makeCGRectValue(0, 120, 320, 100), makeCGRectValue(0, 220, 252, 20) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(251, 220, 2, 20), [simulator finalSelectionStartRect]);
     EXPECT_TRUE([simulator lastKnownDropProposal].precise);
 }
 
@@ -497,7 +485,7 @@ TEST(DragAndDropTests, TextAreaToInput)
     EXPECT_TRUE([simulator suppressedSelectionCommandsDuringDrop]);
     EXPECT_EQ([webView stringByEvaluatingJavaScript:@"source.value"].length, 0UL);
     EXPECT_WK_STREQ("Hello world", [webView editorValue].UTF8String);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(101, 241, 990, 232) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(1089, 241, 2, 232), [simulator finalSelectionStartRect]);
 }
 
 TEST(DragAndDropTests, SinglePlainTextWordTypeIdentifiers)
@@ -568,7 +556,7 @@ TEST(DragAndDropTests, LinkToInput)
     EXPECT_TRUE([observedEventNames containsObject:@"dragenter"]);
     EXPECT_TRUE([observedEventNames containsObject:@"dragover"]);
     EXPECT_TRUE([observedEventNames containsObject:@"drop"]);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(101, 273, 2057, 232) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(2156, 273, 2, 232), [simulator finalSelectionStartRect]);
     checkTypeIdentifierIsRegisteredAtIndex(simulator.get(), (__bridge NSString *)kUTTypeURL, 0);
 }
 
@@ -586,7 +574,7 @@ TEST(DragAndDropTests, BackgroundImageLinkToInput)
     EXPECT_TRUE([observedEventNames containsObject:@"dragenter"]);
     EXPECT_TRUE([observedEventNames containsObject:@"dragover"]);
     EXPECT_TRUE([observedEventNames containsObject:@"drop"]);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(101, 241, 2057, 232) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(2156, 241, 2, 232), [simulator finalSelectionStartRect]);
     checkTypeIdentifierIsRegisteredAtIndex(simulator.get(), (__bridge NSString *)kUTTypeURL, 0);
 }
 
@@ -604,7 +592,7 @@ TEST(DragAndDropTests, CanPreventStart)
     NSArray *observedEventNames = [simulator observedEventNames];
     EXPECT_FALSE([observedEventNames containsObject:@"dragenter"]);
     EXPECT_FALSE([observedEventNames containsObject:@"dragover"]);
-    checkSelectionRectsWithLogging(@[ ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(0, 0, 0, 0), [simulator finalSelectionStartRect]);
 }
 
 TEST(DragAndDropTests, CanPreventOperation)
@@ -620,7 +608,7 @@ TEST(DragAndDropTests, CanPreventOperation)
     NSArray *observedEventNames = [simulator observedEventNames];
     EXPECT_TRUE([observedEventNames containsObject:@"dragenter"]);
     EXPECT_TRUE([observedEventNames containsObject:@"dragover"]);
-    checkSelectionRectsWithLogging(@[ ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(0, 0, 0, 0), [simulator finalSelectionStartRect]);
 }
 
 TEST(DragAndDropTests, EnterAndLeaveEvents)
@@ -638,7 +626,7 @@ TEST(DragAndDropTests, EnterAndLeaveEvents)
     EXPECT_TRUE([observedEventNames containsObject:@"dragover"]);
     EXPECT_TRUE([observedEventNames containsObject:@"dragleave"]);
     EXPECT_FALSE([observedEventNames containsObject:@"drop"]);
-    checkSelectionRectsWithLogging(@[ ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(0, 0, 0, 0), [simulator finalSelectionStartRect]);
 }
 
 TEST(DragAndDropTests, CanStartDragOnDivWithDraggableAttribute)
@@ -1025,7 +1013,7 @@ TEST(DragAndDropTests, ExternalSourceUTF8PlainTextOnly)
     [simulator setExternalItemProviders:@[ simulatedItemProvider.get() ]];
     [simulator runFrom:CGPointMake(300, 400) to:CGPointMake(100, 300)];
     EXPECT_WK_STREQ(textPayload.UTF8String, [webView stringByEvaluatingJavaScript:@"editor.textContent"].UTF8String);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(1, 201, 1936, 227) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(1935, 201, 2, 227), [simulator finalSelectionStartRect]);
 }
 
 TEST(DragAndDropTests, ExternalSourceJPEGOnly)
@@ -1045,7 +1033,7 @@ TEST(DragAndDropTests, ExternalSourceJPEGOnly)
     [simulator setExternalItemProviders:@[ simulatedItemProvider.get() ]];
     [simulator runFrom:CGPointMake(300, 400) to:CGPointMake(100, 300)];
     EXPECT_TRUE([webView editorContainsImageElement]);
-    checkSelectionRectsWithLogging(@[ makeCGRectValue(1, 201, 215, 174) ], [simulator finalSelectionRects]);
+    checkCGRectIsEqualToCGRectWithLogging(CGRectMake(214, 201, 2, 223), [simulator finalSelectionStartRect]);
 }
 
 TEST(DragAndDropTests, ExternalSourceTitledNSURL)
index c5bc0de..999c118 100644 (file)
@@ -108,7 +108,7 @@ typedef NSDictionary<NSNumber *, NSValue *> *ProgressToCGPointValueMap;
 
 @property (nonatomic, readonly) NSArray *sourceItemProviders;
 @property (nonatomic, readonly) NSArray *observedEventNames;
-@property (nonatomic, readonly) NSArray *finalSelectionRects;
+@property (nonatomic, readonly) CGRect finalSelectionStartRect;
 @property (nonatomic, readonly) CGRect lastKnownDragCaretRect;
 @property (nonatomic, readonly) NSArray<UITargetedDragPreview *> *liftPreviews;
 @property (nonatomic, readonly) NSArray *dropPreviews;
index 8edae8c..f6fb657 100644 (file)
@@ -303,7 +303,7 @@ IGNORE_WARNINGS_END
     RetainPtr<NSMutableArray> _observedEventNames;
     RetainPtr<NSArray> _externalItemProviders;
     RetainPtr<NSArray> _sourceItemProviders;
-    RetainPtr<NSArray> _finalSelectionRects;
+    CGRect _finalSelectionStartRect;
     CGPoint _startLocation;
     CGPoint _endLocation;
     CGRect _lastKnownDragCaretRect;
@@ -381,7 +381,7 @@ IGNORE_WARNINGS_END
     _observedEventNames = adoptNS([[NSMutableArray alloc] init]);
     _insertedAttachments = adoptNS([[NSMutableArray alloc] init]);
     _removedAttachments = adoptNS([[NSMutableArray alloc] init]);
-    _finalSelectionRects = @[ ];
+    _finalSelectionStartRect = CGRectNull;
     _dragSession = nil;
     _dropSession = nil;
     _lastKnownDropProposal = nil;
@@ -463,14 +463,12 @@ IGNORE_WARNINGS_END
     Util::run(&_isDoneWithCurrentRun);
     Util::run(&_isDoneWaitingForDelayedDropPreviews);
     [_webView clearMessageHandlers:dragAndDropEventNames()];
-    _finalSelectionRects = [_webView selectionRectsAfterPresentationUpdate];
+    [_webView waitForNextPresentationUpdate];
 
-    [defaultCenter removeObserver:self];
-}
+    auto contentView = [_webView textInputContentView];
+    _finalSelectionStartRect = [contentView caretRectForPosition:contentView.selectedTextRange.start];
 
-- (NSArray *)finalSelectionRects
-{
-    return _finalSelectionRects.get();
+    [defaultCenter removeObserver:self];
 }
 
 - (void)_concludeDropAndPerformOperationIfNecessary