Attachment 358952 Details for Bug 193364 – Patch

[patch] Patch

bug-193364-20190111145135.patch (text/plain), 19.71 KB, created by Wenson Hsieh on 2019-01-11 14:51:36 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-01-11 14:51:36 PST

Size: 19.71 KB

patch

obsolete

>Subversion Revision: 239864
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 8e2c89eef40eda86ba584289ec31574a441f2664..6ddf1efc2d53d74991b180922ce05ae82c59586b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Precision drop state thrashes when dragging near the top edge of an editable element
>+        https://bugs.webkit.org/show_bug.cgi?id=193364
>+        <rdar://problem/47214117>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a new helper method on DragCaretController to compute the bounds of the editable element around the drop
>+        caret position. This is either the enclosing form control (in the case of text fields and text areas), or the
>+        highest editable root. See WebKit ChangeLog for more details.
>+
>+        Test: DragAndDropTests.AvoidPreciseDropNearTopOfTextArea
>+
>+        * editing/FrameSelection.cpp:
>+        (WebCore::DragCaretController::editableElementRectInRootViewCoordinates const):
>+        * editing/FrameSelection.h:
>+
> 2019-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Introduce IDL files for runtime-enabled UndoManager and UndoItem JavaScript API
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index d53cc33c6927e61aa4e69dae90da8051257de925..6d96b167a3e038d51d9c7e9662bba2a6e24c8849 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,38 @@
>+2019-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Precision drop state thrashes when dragging near the top edge of an editable element
>+        https://bugs.webkit.org/show_bug.cgi?id=193364
>+        <rdar://problem/47214117>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        On iOS, marking a UIDropProposal as precise offsets the hit-testing location of the drop by a small distance
>+        either upwards or downwards from the actual location of the user's finger. When dragging over an editable
>+        element, WebKit currently marks the drop proposal as precise; however, when dragging over the top edge of an
>+        editable element, what happens is that the hit-testing location is offset to a location outside of the editable
>+        element, which causes us to turn off precision drop mode; subsequently, turning off precision drop mode removes
>+        the offset, which causes us to hit-test within the editable element once again and re-enable precision mode, and
>+        the cycle continues.
>+
>+        In order to mitigate this, bail out of precision drop mode when dragging near the top or bottom edges of the
>+        highest editable root that contains the current drop caret position (or, if the drop caret is inside of a text
>+        form control, use the form control as the editable element instead).
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::didPerformDragControllerAction):
>+        * UIProcess/WebPageProxy.h:
>+        (WebKit::WebPageProxy::currentDragCaretEditableElementRect const):
>+        * UIProcess/WebPageProxy.messages.in:
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView dropInteraction:sessionDidUpdate:]):
>+
>+        Avoid precise mode when we're less than 25pt away from the top and bottom edge of the editable element rect.
>+        Since the drag location offset amount is a fixed offset in window coordinates, we first convert this minimum
>+        distance to the content view's coordinate space by dividing by the content scale factor.
>+
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::performDragControllerAction):
>+
> 2019-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Introduce IDL files for runtime-enabled UndoManager and UndoItem JavaScript API
>diff --git a/Source/WebCore/editing/FrameSelection.cpp b/Source/WebCore/editing/FrameSelection.cpp
>index 53d658caeddb2d27a1b62d37b0e1505dadef95f9..8073ad7fc7e0f757d2c7c333df8ba8df1a5de3e7 100644
>--- a/Source/WebCore/editing/FrameSelection.cpp
>+++ b/Source/WebCore/editing/FrameSelection.cpp
>@@ -114,6 +114,30 @@ IntRect DragCaretController::caretRectInRootViewCoordinates() const
>     return { };
> }
> 
>+IntRect DragCaretController::editableElementRectInRootViewCoordinates() const
>+{
>+    if (!hasCaret())
>+        return { };
>+
>+    RefPtr<ContainerNode> editableContainer;
>+    if (auto* formControl = enclosingTextFormControl(m_position.deepEquivalent()))
>+        editableContainer = formControl;
>+    else
>+        editableContainer = highestEditableRoot(m_position.deepEquivalent());
>+
>+    if (!editableContainer)
>+        return { };
>+
>+    auto* renderer = editableContainer->renderer();
>+    if (!renderer)
>+        return { };
>+
>+    if (auto* view = editableContainer->document().view())
>+        return view->contentsToRootView(renderer->absoluteBoundingBoxRect());
>+
>+    return { };
>+}
>+
> static inline bool shouldAlwaysUseDirectionalSelection(Frame* frame)
> {
>     return !frame || frame->editor().behavior().shouldConsiderSelectionAsDirectional();
>diff --git a/Source/WebCore/editing/FrameSelection.h b/Source/WebCore/editing/FrameSelection.h
>index 23154ad2f2d95836a8561c65d12268c762c71028..e170721a90b6ddfdbb7ef6e97ca117ec76dee611 100644
>--- a/Source/WebCore/editing/FrameSelection.h
>+++ b/Source/WebCore/editing/FrameSelection.h
>@@ -104,6 +104,7 @@ public:
>     void setCaretPosition(const VisiblePosition&);
>     void clear() { setCaretPosition(VisiblePosition()); }
>     WEBCORE_EXPORT IntRect caretRectInRootViewCoordinates() const;
>+    WEBCORE_EXPORT IntRect editableElementRectInRootViewCoordinates() const;
> 
>     void nodeWillBeRemoved(Node&);
> 
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 392743bd5679cbd2267a2c10ecce195a61c33d29..c8ab70333b4039a8779c4ca3f31e1907563a3369 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -2090,7 +2090,7 @@ void WebPageProxy::performDragControllerAction(DragControllerAction action, Drag
> #endif
> }
> 
>-void WebPageProxy::didPerformDragControllerAction(uint64_t dragOperation, WebCore::DragHandlingMethod dragHandlingMethod, bool mouseIsOverFileInput, unsigned numberOfItemsToBeAccepted, const IntRect& insertionRect)
>+void WebPageProxy::didPerformDragControllerAction(uint64_t dragOperation, WebCore::DragHandlingMethod dragHandlingMethod, bool mouseIsOverFileInput, unsigned numberOfItemsToBeAccepted, const IntRect& insertionRect, const IntRect& editableElementRect)
> {
>     MESSAGE_CHECK(dragOperation <= DragOperationDelete);
> 
>@@ -2098,6 +2098,7 @@ void WebPageProxy::didPerformDragControllerAction(uint64_t dragOperation, WebCor
>     m_currentDragHandlingMethod = dragHandlingMethod;
>     m_currentDragIsOverFileInput = mouseIsOverFileInput;
>     m_currentDragNumberOfFilesToBeAccepted = numberOfItemsToBeAccepted;
>+    m_currentDragCaretEditableElementRect = editableElementRect;
>     setDragCaretRect(insertionRect);
> }
> 
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 232bb175fd182e4859bae352b11f145e1d289c92..de98694aa37db3c7f2ce27cc829a9200211de10d 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -979,7 +979,7 @@ public:
>     void performDragOperation(WebCore::DragData&, const String& dragStorageName, SandboxExtension::Handle&&, SandboxExtension::HandleArray&&);
>     void didPerformDragOperation(bool handled);
> 
>-    void didPerformDragControllerAction(uint64_t dragOperation, WebCore::DragHandlingMethod, bool mouseIsOverFileInput, unsigned numberOfItemsToBeAccepted, const WebCore::IntRect& insertionRect);
>+    void didPerformDragControllerAction(uint64_t dragOperation, WebCore::DragHandlingMethod, bool mouseIsOverFileInput, unsigned numberOfItemsToBeAccepted, const WebCore::IntRect& insertionRect, const WebCore::IntRect& editableElementRect);
>     void dragEnded(const WebCore::IntPoint& clientPosition, const WebCore::IntPoint& globalPosition, uint64_t operation);
>     void didStartDrag();
>     void dragCancelled();
>@@ -1038,6 +1038,7 @@ public:
>     bool currentDragIsOverFileInput() const { return m_currentDragIsOverFileInput; }
>     unsigned currentDragNumberOfFilesToBeAccepted() const { return m_currentDragNumberOfFilesToBeAccepted; }
>     WebCore::IntRect currentDragCaretRect() const { return m_currentDragCaretRect; }
>+    WebCore::IntRect currentDragCaretEditableElementRect() const { return m_currentDragCaretEditableElementRect; }
>     void resetCurrentDragInformation();
>     void didEndDragging();
> #endif
>@@ -2179,6 +2180,7 @@ private:
>     bool m_currentDragIsOverFileInput { false };
>     unsigned m_currentDragNumberOfFilesToBeAccepted { 0 };
>     WebCore::IntRect m_currentDragCaretRect;
>+    WebCore::IntRect m_currentDragCaretEditableElementRect;
> #endif
> 
>     PageLoadState m_pageLoadState;
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.messages.in b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>index 95fee23e0d9cb87019e76814011a5d6b764b549b..2d28ee18b69e8295c1f7a813f9cb471bdda48ff2 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.messages.in
>+++ b/Source/WebKit/UIProcess/WebPageProxy.messages.in
>@@ -311,7 +311,7 @@ messages -> WebPageProxy {
> 
>     # Drag and drop messages
> #if ENABLE(DRAG_SUPPORT)
>-    DidPerformDragControllerAction(uint64_t dragOperation, enum:uint8_t WebCore::DragHandlingMethod dragHandlingMethod, bool mouseIsOverFileInput, unsigned numberOfItemsToBeAccepted, WebCore::IntRect insertionRect)
>+    DidPerformDragControllerAction(uint64_t dragOperation, enum:uint8_t WebCore::DragHandlingMethod dragHandlingMethod, bool mouseIsOverFileInput, unsigned numberOfItemsToBeAccepted, WebCore::IntRect insertionRect, WebCore::IntRect editableElementRect)
>     DidEndDragging();
> #endif
> #if PLATFORM(COCOA) && ENABLE(DRAG_SUPPORT)
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index 51a0d875310cedd519aa4ecfb418ffcf6d238ed7..3e5292a78c2acf461a68293b3aac422c90d7b3c5 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -5989,7 +5989,16 @@ - (UIDropProposal *)dropInteraction:(UIDropInteraction *)interaction sessionDidU
> 
>     auto proposal = adoptNS([[UIDropProposal alloc] initWithDropOperation:static_cast<UIDropOperation>(operation)]);
>     auto dragHandlingMethod = _page->currentDragHandlingMethod();
>-    [proposal setPrecise:dragHandlingMethod == WebCore::DragHandlingMethod::EditPlainText || dragHandlingMethod == WebCore::DragHandlingMethod::EditRichText];
>+    if (dragHandlingMethod == WebCore::DragHandlingMethod::EditPlainText || dragHandlingMethod == WebCore::DragHandlingMethod::EditRichText) {
>+        // When dragging near the top or bottom edges of an editable element, enabling precision drop mode may result in the drag session hit-testing outside of the editable
>+        // element, causing the drag to no longer be accepted. This in turn disables precision drop mode, which causes the drag session to hit-test inside of the editable
>+        // element again, which enables precision mode, thus continuing the cycle. To avoid precision mode thrashing, we forbid precision mode when dragging near the top or
>+        // bottom of the editable element.
>+        auto minimumDistanceFromVerticalEdgeForPreciseDrop = 25 / _webView.scrollView.zoomScale;
>+        [proposal setPrecise:CGRectContainsPoint(CGRectInset(_page->currentDragCaretEditableElementRect(), 0, minimumDistanceFromVerticalEdgeForPreciseDrop), [session locationInView:self])];
>+    } else
>+        [proposal setPrecise:NO];
>+
>     if ([delegate respondsToSelector:@selector(_webView:willUpdateDropProposalToProposal:forSession:)])
>         proposal = [delegate _webView:_webView willUpdateDropProposalToProposal:proposal.get() forSession:session];
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index 25d6dfb71d03a496644c6232f6fb4218fa8d2fbb..803ba8b995ed3ec831ab46b5397c7662e6a81c65 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -3671,7 +3671,7 @@ NotificationPermissionRequestManager* WebPage::notificationPermissionRequestMana
> void WebPage::performDragControllerAction(DragControllerAction action, const IntPoint& clientPosition, const IntPoint& globalPosition, uint64_t draggingSourceOperationMask, WebSelectionData&& selection, uint32_t flags)
> {
>     if (!m_page) {
>-        send(Messages::WebPageProxy::DidPerformDragControllerAction(DragOperationNone, DragHandlingMethod::None, false, 0, { }));
>+        send(Messages::WebPageProxy::DidPerformDragControllerAction(DragOperationNone, DragHandlingMethod::None, false, 0, { }, { }));
>         return;
>     }
> 
>@@ -3679,12 +3679,12 @@ void WebPage::performDragControllerAction(DragControllerAction action, const Int
>     switch (action) {
>     case DragControllerAction::Entered: {
>         DragOperation resolvedDragOperation = m_page->dragController().dragEntered(dragData);
>-        send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), { }));
>+        send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), { }, { }));
>         return;
>     }
>     case DragControllerAction::Updated: {
>         DragOperation resolvedDragOperation = m_page->dragController().dragEntered(dragData);
>-        send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), { }));
>+        send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), { }, { }));
>         return;
>     }
>     case DragControllerAction::Exited:
>@@ -3702,24 +3702,24 @@ void WebPage::performDragControllerAction(DragControllerAction action, const Int
> void WebPage::performDragControllerAction(DragControllerAction action, const WebCore::DragData& dragData, SandboxExtension::Handle&& sandboxExtensionHandle, SandboxExtension::HandleArray&& sandboxExtensionsHandleArray)
> {
>     if (!m_page) {
>-        send(Messages::WebPageProxy::DidPerformDragControllerAction(DragOperationNone, DragHandlingMethod::None, false, 0, { }));
>+        send(Messages::WebPageProxy::DidPerformDragControllerAction(DragOperationNone, DragHandlingMethod::None, false, 0, { }, { }));
>         return;
>     }
> 
>     switch (action) {
>     case DragControllerAction::Entered: {
>         DragOperation resolvedDragOperation = m_page->dragController().dragEntered(dragData);
>-        send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), m_page->dragCaretController().caretRectInRootViewCoordinates()));
>+        send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), m_page->dragCaretController().caretRectInRootViewCoordinates(), m_page->dragCaretController().editableElementRectInRootViewCoordinates()));
>         return;
>     }
>     case DragControllerAction::Updated: {
>         DragOperation resolvedDragOperation = m_page->dragController().dragUpdated(dragData);
>-        send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), m_page->dragCaretController().caretRectInRootViewCoordinates()));
>+        send(Messages::WebPageProxy::DidPerformDragControllerAction(resolvedDragOperation, m_page->dragController().dragHandlingMethod(), m_page->dragController().mouseIsOverFileInput(), m_page->dragController().numberOfItemsToBeAccepted(), m_page->dragCaretController().caretRectInRootViewCoordinates(), m_page->dragCaretController().editableElementRectInRootViewCoordinates()));
>         return;
>     }
>     case DragControllerAction::Exited:
>         m_page->dragController().dragExited(dragData);
>-        send(Messages::WebPageProxy::DidPerformDragControllerAction(DragOperationNone, DragHandlingMethod::None, false, 0, { }));
>+        send(Messages::WebPageProxy::DidPerformDragControllerAction(DragOperationNone, DragHandlingMethod::None, false, 0, { }, { }));
>         return;
>         
>     case DragControllerAction::PerformDragOperation: {
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index e22f99f30e75cf9dd34f6974fbc8fdfcdbc99b08..62e77903e39f7b19352c89c79125eff0cdec725b 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Precision drop state thrashes when dragging near the top edge of an editable element
>+        https://bugs.webkit.org/show_bug.cgi?id=193364
>+        <rdar://problem/47214117>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a test to verify that dragging near the top of a textarea element does not flag the drop proposal as
>+        precise, whereas dragging near the middle of the textarea does.
>+
>+        * TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
>+        (TestWebKitAPI::TEST):
>+
> 2019-01-11  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Introduce IDL files for runtime-enabled UndoManager and UndoItem JavaScript API
>diff --git a/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm b/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
>index 44bc15094c62f542f011124b36830608e9e13cc8..f1a45db507bd2d34932162568f6013b101efc9ed 100644
>--- a/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
>+++ b/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
>@@ -326,6 +326,27 @@ TEST(DragAndDropTests, ImageInLinkToInput)
>     EXPECT_TRUE([simulator lastKnownDropProposal].precise);
> }
> 
>+TEST(DragAndDropTests, AvoidPreciseDropNearTopOfTextArea)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>+    [webView synchronouslyLoadHTMLString:@"<meta name='viewport' content='width=device-width, initial-scale=1'><body style='margin: 0'><textarea style='height: 100px'></textarea></body>"];
>+
>+    auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
>+    auto firstItem = adoptNS([[NSItemProvider alloc] initWithObject:[NSURL URLWithString:@"https://webkit.org"]]);
>+    [simulator setExternalItemProviders:@[ firstItem.get() ]];
>+    [simulator runFrom:CGPointMake(320, 10) to:CGPointMake(20, 10)];
>+
>+    EXPECT_WK_STREQ("https://webkit.org/", [webView stringByEvaluatingJavaScript:@"document.querySelector('textarea').value"]);
>+    EXPECT_FALSE([simulator lastKnownDropProposal].precise);
>+    [webView evaluateJavaScript:@"document.querySelector('textarea').value = ''" completionHandler:nil];
>+
>+    auto secondItem = adoptNS([[NSItemProvider alloc] initWithObject:[NSURL URLWithString:@"https://apple.com"]]);
>+    [simulator setExternalItemProviders:@[ secondItem.get() ]];
>+    [simulator runFrom:CGPointMake(320, 50) to:CGPointMake(20, 50)];
>+    EXPECT_WK_STREQ("https://apple.com/", [webView stringByEvaluatingJavaScript:@"document.querySelector('textarea').value"]);
>+    EXPECT_TRUE([simulator lastKnownDropProposal].precise);
>+}
>+
> TEST(DragAndDropTests, ImageInLinkWithoutHREFToInput)
> {
>     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);

Actions: View | Formatted Diff | Diff

Attachments on bug 193364: 358952