WebKit Bugzilla
Attachment 358952 Details for
Bug 193364
: [iOS] Precision drop state thrashes when dragging near the top edge of an editable element
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[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:
Patch
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)]);
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193364
: 358952