Title: [198909] trunk/Source/WebCore
- Revision
- 198909
- Author
- [email protected]
- Date
- 2016-03-31 12:25:45 -0700 (Thu, 31 Mar 2016)
Log Message
eventMayStartDrag() does not check for shiftKey or isOverLink
https://bugs.webkit.org/show_bug.cgi?id=155746
Reviewed by Darin Adler.
There is currently a mismatch between the logic that checks whether
an event can start a dragging action in EventHandler::handleMousePressEvent
and in EventHandler::eventMayStartDrag
Basically the former checks for event's count, type, button, target and modifier.
The later, on the other hand, only checks for event's button and count.
In order to sync them up again, as per the comment in the code,
patch factors out the logic in ::handleMousePressEvent into a helper function,
and ultimately reuses it in ::eventMayStartDrag.
No new tests for two reasons:
1) ::handleMousePressEvent will not have any behavior change. Code is factored out
into a helper.
2) The callees of ::eventMayStartDrag are currently WebPage::shouldDelayWindowOrderingEvent
and ::acceptsFirstMouse. Based on my understanding of [1], these methods
are called when there is a (say) browser window in background with some text selected,
and user starts to drag the selected content. Dragging happens without bringing the window
foreground. This is called "non-activating click".
Such behavior will not change with the patch.
[1] https://bugs.webkit.org/show_bug.cgi?id=55053 - "Implement non-activating clicks to allow dragging out of a background window"
* page/EventHandler.cpp:
(WebCore::isSingleMouseDownOnLinkOrImage):
(WebCore::canMouseEventStartDragInternal):
(WebCore::EventHandler::handleMousePressEvent):
(WebCore::documentPointForWindowPoint): Moved up in the file to be used elsewhere.
(WebCore::EventHandler::eventMayStartDrag):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (198908 => 198909)
--- trunk/Source/WebCore/ChangeLog 2016-03-31 19:12:29 UTC (rev 198908)
+++ trunk/Source/WebCore/ChangeLog 2016-03-31 19:25:45 UTC (rev 198909)
@@ -1,5 +1,43 @@
2016-03-31 Antonio Gomes <[email protected]>
+ eventMayStartDrag() does not check for shiftKey or isOverLink
+ https://bugs.webkit.org/show_bug.cgi?id=155746
+
+ Reviewed by Darin Adler.
+
+ There is currently a mismatch between the logic that checks whether
+ an event can start a dragging action in EventHandler::handleMousePressEvent
+ and in EventHandler::eventMayStartDrag
+ Basically the former checks for event's count, type, button, target and modifier.
+ The later, on the other hand, only checks for event's button and count.
+
+ In order to sync them up again, as per the comment in the code,
+ patch factors out the logic in ::handleMousePressEvent into a helper function,
+ and ultimately reuses it in ::eventMayStartDrag.
+
+ No new tests for two reasons:
+
+ 1) ::handleMousePressEvent will not have any behavior change. Code is factored out
+ into a helper.
+
+ 2) The callees of ::eventMayStartDrag are currently WebPage::shouldDelayWindowOrderingEvent
+ and ::acceptsFirstMouse. Based on my understanding of [1], these methods
+ are called when there is a (say) browser window in background with some text selected,
+ and user starts to drag the selected content. Dragging happens without bringing the window
+ foreground. This is called "non-activating click".
+ Such behavior will not change with the patch.
+
+ [1] https://bugs.webkit.org/show_bug.cgi?id=55053 - "Implement non-activating clicks to allow dragging out of a background window"
+
+ * page/EventHandler.cpp:
+ (WebCore::isSingleMouseDownOnLinkOrImage):
+ (WebCore::canMouseEventStartDragInternal):
+ (WebCore::EventHandler::handleMousePressEvent):
+ (WebCore::documentPointForWindowPoint): Moved up in the file to be used elsewhere.
+ (WebCore::EventHandler::eventMayStartDrag):
+
+2016-03-31 Antonio Gomes <[email protected]>
+
SelectionController::positionForPlatform should ask EditingBehavior for platform specific behavior
https://bugs.webkit.org/show_bug.cgi?id=41976
Modified: trunk/Source/WebCore/page/EventHandler.cpp (198908 => 198909)
--- trunk/Source/WebCore/page/EventHandler.cpp 2016-03-31 19:12:29 UTC (rev 198908)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2016-03-31 19:25:45 UTC (rev 198909)
@@ -737,6 +737,22 @@
return node->canStartSelection() || Position::nodeIsUserSelectAll(node);
}
+#if ENABLE(DRAG_SUPPORT)
+static bool isSingleMouseDownOnLinkOrImage(const MouseEventWithHitTestResults& event)
+{
+ auto& platformEvent = event.event();
+ if (platformEvent.type() != PlatformEvent::MousePressed || platformEvent.button() != LeftButton || platformEvent.clickCount() != 1)
+ return false;
+
+ return event.isOverLink() || event.hitTestResult().image();
+}
+
+static bool eventMayStartDragInternal(const MouseEventWithHitTestResults& event)
+{
+ return !event.event().shiftKey() || isSingleMouseDownOnLinkOrImage(event);
+}
+#endif
+
bool EventHandler::handleMousePressEvent(const MouseEventWithHitTestResults& event)
{
#if ENABLE(DRAG_SUPPORT)
@@ -762,13 +778,7 @@
m_mouseDownMayStartSelect = canMouseDownStartSelect(event.targetNode()) && !event.scrollbar();
#if ENABLE(DRAG_SUPPORT)
- // Careful that the drag starting logic stays in sync with eventMayStartDrag()
- // FIXME: eventMayStartDrag() does not check for shift key press, link or image event targets.
- // Bug: https://bugs.webkit.org/show_bug.cgi?id=155390
-
- // Single mouse down on links or images can always trigger drag-n-drop.
- bool isMouseDownOnLinkOrImage = event.isOverLink() || event.hitTestResult().image();
- m_mouseDownMayStartDrag = singleClick && (!event.event().shiftKey() || isMouseDownOnLinkOrImage);
+ m_mouseDownMayStartDrag = eventMayStartDragInternal(event);
#endif
m_mouseDownWasSingleClickInSelection = false;
@@ -816,6 +826,14 @@
return swallowEvent;
}
+static LayoutPoint documentPointForWindowPoint(Frame& frame, const IntPoint& windowPoint)
+{
+ FrameView* view = frame.view();
+ // FIXME: Is it really OK to use the wrong coordinates here when view is 0?
+ // Historically the code would just crash; this is clearly no worse than that.
+ return view ? view->windowToContents(windowPoint) : windowPoint;
+}
+
#if ENABLE(DRAG_SUPPORT)
bool EventHandler::handleMouseDraggedEvent(const MouseEventWithHitTestResults& event)
{
@@ -860,7 +878,7 @@
updateSelectionForMouseDrag(event.hitTestResult());
return true;
}
-
+
bool EventHandler::eventMayStartDrag(const PlatformMouseEvent& event) const
{
// This is a pre-flight check of whether the event might lead to a drag being started. Be careful
@@ -870,9 +888,6 @@
if (!renderView)
return false;
- if (event.button() != LeftButton || event.clickCount() != 1)
- return false;
-
FrameView* view = m_frame.view();
if (!view)
return false;
@@ -881,10 +896,15 @@
if (!page)
return false;
+ HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::DisallowShadowContent);
+ LayoutPoint documentPoint = documentPointForWindowPoint(m_frame, event.position());
+ MouseEventWithHitTestResults mouseEvent = m_frame.document()->prepareMouseEvent(request, documentPoint, event);
+ if (!eventMayStartDragInternal(mouseEvent))
+ return false;
+
updateDragSourceActionsAllowed();
- HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::DisallowShadowContent);
- HitTestResult result(view->windowToContents(event.position()));
- renderView->hitTest(request, result);
+
+ HitTestResult result = mouseEvent.hitTestResult();
DragState state;
return result.innerElement() && page->dragController().draggableElement(&m_frame, result.innerElement(), result.roundedPointInInnerNodeFrame(), state);
}
@@ -1577,14 +1597,6 @@
}
#endif
-static LayoutPoint documentPointForWindowPoint(Frame& frame, const IntPoint& windowPoint)
-{
- FrameView* view = frame.view();
- // FIXME: Is it really OK to use the wrong coordinates here when view is 0?
- // Historically the code would just crash; this is clearly no worse than that.
- return view ? view->windowToContents(windowPoint) : windowPoint;
-}
-
static Scrollbar* scrollbarForMouseEvent(const MouseEventWithHitTestResults& mouseEvent, FrameView* view)
{
if (view) {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
