From e095b6eb4b18f022ade46f6f002711f5255a6236 Mon Sep 17 00:00:00 2001 From: "commit-queue@webkit.org" Date: Fri, 5 Apr 2013 12:38:41 +0000 Subject: [PATCH] Source/WebCore: Updating mouse cursor on style changes without emitting fake mousemove event https://bugs.webkit.org/show_bug.cgi?id=101857 Patch by Aivo Paas on 2013-04-05 Reviewed by Allan Sandfeld Jensen. Mouse cursor changes in styles used to be reflected in UI through dispatching a fake mousemove event. The old approach has some flaws: it emits a mousemove event in javascript when there is no mouse movement involved (bug 85343); the fake mousemove event is cancelled while there is a mouse button held down - cursor won't change until mouse is moved or the button released (bug 53341). The new approach does not use the fake mousemove event. Instead, it uses only the logic needed for the actual cursor change to happen. EventHandler::selectCursor was refactored to not take a whole mouse event but instead work with HitTestResult so that EventHandler::updateCursor must not create a useless PlatformEvent. Fixes: https://bugs.webkit.org/show_bug.cgi?id=85343 (mousemove event on cursor change) https://bugs.webkit.org/show_bug.cgi?id=53341 (no cursor change when mouse button down) Tests: fast/events/mouse-cursor-change.html fast/events/mouse-cursor-no-mousemove.html * page/EventHandler.cpp: (WebCore::EventHandler::EventHandler): (WebCore::EventHandler::clear): (WebCore::EventHandler::cursorUpdateTimerFired): (WebCore::EventHandler::updateCursor): (WebCore::EventHandler::selectCursor): (WebCore::EventHandler::handleMouseMoveEvent): (WebCore::EventHandler::scheduleCursorUpdate): * page/EventHandler.h: * page/FrameView.cpp: (WebCore::FrameView::shouldSetCursor): * page/FrameView.h: * page/MouseEventWithHitTestResults.cpp: (WebCore::MouseEventWithHitTestResults::isOverLink): * rendering/HitTestResult.cpp: (WebCore::HitTestResult::isOverLink): * rendering/HitTestResult.h: * rendering/RenderObject.cpp: (WebCore::RenderObject::styleDidChange): LayoutTests: Updating mouse cursor on style changes without emitting fake mousemove event https://bugs.webkit.org/show_bug.cgi?id=101857 Changing CSS cursor should work no matter is mouse button is pressed or not https://bugs.webkit.org/show_bug.cgi?id=53341 Patch by Aivo Paas on 2013-04-05 Reviewed by Allan Sandfeld Jensen. Added tests for changing cursor on mousemove, mousedown, mouseup and mousemove while mouse button being held down. Also added test to verify that a mousemove event is not fired for changing cursor when mouse is not moving. * fast/events/mouse-cursor-change-expected.txt: Added. * fast/events/mouse-cursor-change.html: Added. * fast/events/mouse-cursor-no-mousemove-expected.txt: Added. * fast/events/mouse-cursor-no-mousemove.html: Added. * platform/mac/TestExpectations: git-svn-id: https://svn.webkit.org/repository/webkit/trunk@147739 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 19 +++++ .../fast/events/mouse-cursor-change-expected.txt | 24 +++++++ LayoutTests/fast/events/mouse-cursor-change.html | 80 ++++++++++++++++++++++ .../events/mouse-cursor-no-mousemove-expected.txt | 16 +++++ .../fast/events/mouse-cursor-no-mousemove.html | 57 +++++++++++++++ LayoutTests/platform/mac/TestExpectations | 2 + Source/WebCore/ChangeLog | 45 ++++++++++++ Source/WebCore/page/EventHandler.cpp | 78 ++++++++++++++++++--- Source/WebCore/page/EventHandler.h | 7 +- Source/WebCore/page/FrameView.cpp | 6 ++ Source/WebCore/page/FrameView.h | 1 + .../WebCore/page/MouseEventWithHitTestResults.cpp | 2 +- Source/WebCore/rendering/HitTestResult.cpp | 5 ++ Source/WebCore/rendering/HitTestResult.h | 1 + Source/WebCore/rendering/RenderObject.cpp | 2 +- 15 files changed, 333 insertions(+), 12 deletions(-) create mode 100644 LayoutTests/fast/events/mouse-cursor-change-expected.txt create mode 100644 LayoutTests/fast/events/mouse-cursor-change.html create mode 100644 LayoutTests/fast/events/mouse-cursor-no-mousemove-expected.txt create mode 100644 LayoutTests/fast/events/mouse-cursor-no-mousemove.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index a9f0a29..a654505 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,22 @@ +2013-04-05 Aivo Paas + + Updating mouse cursor on style changes without emitting fake mousemove event + https://bugs.webkit.org/show_bug.cgi?id=101857 + Changing CSS cursor should work no matter is mouse button is pressed or not + https://bugs.webkit.org/show_bug.cgi?id=53341 + + Reviewed by Allan Sandfeld Jensen. + + Added tests for changing cursor on mousemove, mousedown, mouseup and mousemove + while mouse button being held down. Also added test to verify that a mousemove + event is not fired for changing cursor when mouse is not moving. + + * fast/events/mouse-cursor-change-expected.txt: Added. + * fast/events/mouse-cursor-change.html: Added. + * fast/events/mouse-cursor-no-mousemove-expected.txt: Added. + * fast/events/mouse-cursor-no-mousemove.html: Added. + * platform/mac/TestExpectations: + 2013-04-05 Zan Dobersek Unreviewed GTK gardening. diff --git a/LayoutTests/fast/events/mouse-cursor-change-expected.txt b/LayoutTests/fast/events/mouse-cursor-change-expected.txt new file mode 100644 index 0000000..48b7abe --- /dev/null +++ b/LayoutTests/fast/events/mouse-cursor-change-expected.txt @@ -0,0 +1,24 @@ +Test that mouse cursors are changed correctly on mouse events. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +Bug 53341 + + +Mouse move +Cursor Info: type=Hand hotSpot=0,0 + +Mouse down +Cursor Info: type=Progress hotSpot=0,0 + +Mouse hold down, move +Cursor Info: type=Hand hotSpot=0,0 + +Mouse up +Cursor Info: type=Help hotSpot=0,0 + +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/LayoutTests/fast/events/mouse-cursor-change.html b/LayoutTests/fast/events/mouse-cursor-change.html new file mode 100644 index 0000000..194772a --- /dev/null +++ b/LayoutTests/fast/events/mouse-cursor-change.html @@ -0,0 +1,80 @@ + + + + + + + +

+

Bug 53341

+
+
Play with mouse on this element. Cursors change on events - mousemove: pointer(hand), mousedown: progress, mouseup: help.
+
+
+
+ + + + diff --git a/LayoutTests/fast/events/mouse-cursor-no-mousemove-expected.txt b/LayoutTests/fast/events/mouse-cursor-no-mousemove-expected.txt new file mode 100644 index 0000000..a5d0645 --- /dev/null +++ b/LayoutTests/fast/events/mouse-cursor-no-mousemove-expected.txt @@ -0,0 +1,16 @@ +Test that there is no mousemove event fired when changing cursor. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +Bug 85343 + + +TEST CASE: Mouse idle, change cursor should not fire mousemove event +Cursor Info: type=Pointer hotSpot=0,0 +Cursor Info: type=Help hotSpot=0,0 + +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/LayoutTests/fast/events/mouse-cursor-no-mousemove.html b/LayoutTests/fast/events/mouse-cursor-no-mousemove.html new file mode 100644 index 0000000..acb6fad --- /dev/null +++ b/LayoutTests/fast/events/mouse-cursor-no-mousemove.html @@ -0,0 +1,57 @@ + + + + + + + +

+

Bug 85343

+
+
Mouse idle, change cursor should not fire mousemove event
+
+
+
+ + + + diff --git a/LayoutTests/platform/mac/TestExpectations b/LayoutTests/platform/mac/TestExpectations index 10ab196..a526eab 100644 --- a/LayoutTests/platform/mac/TestExpectations +++ b/LayoutTests/platform/mac/TestExpectations @@ -1500,3 +1500,5 @@ webkit.org/b/113227 transitions/cross-fade-border-image.html [ Pass Failure ] webkit.org/b/113227 transitions/default-timing-function.html [ Pass Failure ] webkit.org/b/113227 transitions/svg-text-shadow-transition.html [ Pass Failure ] +# New test failing, needs investigation. +webkit.org/b/103857 fast/events/mouse-cursor-change.html diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 1356b8d..106bdca 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,48 @@ +2013-04-05 Aivo Paas + + Updating mouse cursor on style changes without emitting fake mousemove event + https://bugs.webkit.org/show_bug.cgi?id=101857 + + Reviewed by Allan Sandfeld Jensen. + + Mouse cursor changes in styles used to be reflected in UI through dispatching a fake + mousemove event. The old approach has some flaws: it emits a mousemove event in + javascript when there is no mouse movement involved (bug 85343); the fake mousemove + event is cancelled while there is a mouse button held down - cursor won't change + until mouse is moved or the button released (bug 53341). + + The new approach does not use the fake mousemove event. Instead, it uses only the logic + needed for the actual cursor change to happen. + + EventHandler::selectCursor was refactored to not take a whole mouse event but instead work with + HitTestResult so that EventHandler::updateCursor must not create a useless PlatformEvent. + + Fixes: https://bugs.webkit.org/show_bug.cgi?id=85343 (mousemove event on cursor change) + https://bugs.webkit.org/show_bug.cgi?id=53341 (no cursor change when mouse button down) + + Tests: fast/events/mouse-cursor-change.html + fast/events/mouse-cursor-no-mousemove.html + + * page/EventHandler.cpp: + (WebCore::EventHandler::EventHandler): + (WebCore::EventHandler::clear): + (WebCore::EventHandler::cursorUpdateTimerFired): + (WebCore::EventHandler::updateCursor): + (WebCore::EventHandler::selectCursor): + (WebCore::EventHandler::handleMouseMoveEvent): + (WebCore::EventHandler::scheduleCursorUpdate): + * page/EventHandler.h: + * page/FrameView.cpp: + (WebCore::FrameView::shouldSetCursor): + * page/FrameView.h: + * page/MouseEventWithHitTestResults.cpp: + (WebCore::MouseEventWithHitTestResults::isOverLink): + * rendering/HitTestResult.cpp: + (WebCore::HitTestResult::isOverLink): + * rendering/HitTestResult.h: + * rendering/RenderObject.cpp: + (WebCore::RenderObject::styleDidChange): + 2013-04-05 Jocelyn Turcotte [Qt] PluginsX11: exposedRect offset is applied twice when painting windowless diff --git a/Source/WebCore/page/EventHandler.cpp b/Source/WebCore/page/EventHandler.cpp index 30a6a5c..62ddca5 100644 --- a/Source/WebCore/page/EventHandler.cpp +++ b/Source/WebCore/page/EventHandler.cpp @@ -144,6 +144,10 @@ using namespace SVGNames; const double fakeMouseMoveShortInterval = 0.1; const double fakeMouseMoveLongInterval = 0.250; +// The amount of time to wait for a cursor update on style and layout changes +// Set to 50Hz, no need to be faster than common screen refresh rate +const double cursorUpdateInterval = 0.02; + const int maximumCursorSize = 128; #if ENABLE(MOUSE_CURSOR_SCALE) // It's pretty unlikely that a scale of less than one would ever be used. But all we really @@ -319,6 +323,7 @@ EventHandler::EventHandler(Frame* frame) , m_mouseDownWasSingleClickInSelection(false) , m_selectionInitiationState(HaveNotStartedSelection) , m_hoverTimer(this, &EventHandler::hoverTimerFired) + , m_cursorUpdateTimer(this, &EventHandler::cursorUpdateTimerFired) , m_autoscrollController(adoptPtr(new AutoscrollController)) , m_mouseDownMayStartAutoscroll(false) , m_mouseDownWasInSubframe(false) @@ -374,6 +379,7 @@ DragState& EventHandler::dragState() void EventHandler::clear() { m_hoverTimer.stop(); + m_cursorUpdateTimer.stop(); m_fakeMouseMoveEventTimer.stop(); #if ENABLE(CURSOR_VISIBILITY) cancelAutoHideCursorTimer(); @@ -1241,7 +1247,50 @@ bool EventHandler::useHandCursor(Node* node, bool isOverLink, bool shiftKey) return ((isOverLink || isSubmitImage(node)) && (!editable || editableLinkEnabled)); } -OptionalCursor EventHandler::selectCursor(const MouseEventWithHitTestResults& event, Scrollbar* scrollbar) +void EventHandler::cursorUpdateTimerFired(Timer*) +{ + ASSERT(m_frame); + ASSERT(m_frame->document()); + + updateCursor(); +} + +void EventHandler::updateCursor() +{ + if (m_mousePositionIsUnknown) + return; + + FrameView* view = m_frame->view(); + if (!view) + return; + + RenderView* renderView = view->renderView(); + if (!renderView) + return; + + if (!view->shouldSetCursor()) + return; + + bool shiftKey; + bool ctrlKey; + bool altKey; + bool metaKey; + PlatformKeyboardEvent::getCurrentModifierState(shiftKey, ctrlKey, altKey, metaKey); + + m_frame->document()->updateLayout(); + + HitTestRequest request(HitTestRequest::ReadOnly); + HitTestResult result(view->windowToContents(m_lastKnownMousePosition)); + renderView->hitTest(request, result); + + OptionalCursor optionalCursor = selectCursor(result, shiftKey); + if (optionalCursor.isCursorChange()) { + m_currentMouseCursor = optionalCursor.cursor(); + view->setCursor(m_currentMouseCursor); + } +} + +OptionalCursor EventHandler::selectCursor(const HitTestResult& result, bool shiftKey) { if (m_resizeLayer && m_resizeLayer->inResizeMode()) return NoCursorChange; @@ -1254,8 +1303,11 @@ OptionalCursor EventHandler::selectCursor(const MouseEventWithHitTestResults& ev return NoCursorChange; #endif - Node* node = event.targetNode(); - RenderObject* renderer = node ? node->renderer() : 0; + Node* node = result.targetNode(); + if (!node) + return NoCursorChange; + + RenderObject* renderer = node->renderer(); RenderStyle* style = renderer ? renderer->style() : 0; bool horizontalText = !style || style->isHorizontalWritingMode(); const Cursor& iBeam = horizontalText ? iBeamCursor() : verticalTextCursor(); @@ -1279,7 +1331,7 @@ OptionalCursor EventHandler::selectCursor(const MouseEventWithHitTestResults& ev if (renderer) { Cursor overrideCursor; - switch (renderer->getCursor(roundedIntPoint(event.localPoint()), overrideCursor)) { + switch (renderer->getCursor(roundedIntPoint(result.localPoint()), overrideCursor)) { case SetCursorBasedOnStyle: break; case SetCursor: @@ -1326,19 +1378,19 @@ OptionalCursor EventHandler::selectCursor(const MouseEventWithHitTestResults& ev switch (style ? style->cursor() : CURSOR_AUTO) { case CURSOR_AUTO: { - bool editable = (node && node->rendererIsEditable()); + bool editable = node->rendererIsEditable(); - if (useHandCursor(node, event.isOverLink(), event.event().shiftKey())) + if (useHandCursor(node, result.isOverLink(), shiftKey)) return handCursor(); bool inResizer = false; if (renderer) { if (RenderLayer* layer = renderer->enclosingLayer()) { if (FrameView* view = m_frame->view()) - inResizer = layer->isPointInResizeControl(view->windowToContents(event.event().position())); + inResizer = layer->isPointInResizeControl(view->windowToContents(roundedIntPoint(result.localPoint()))); } } - if ((editable || (renderer && renderer->isText() && node->canStartSelection())) && !inResizer && !scrollbar) + if ((editable || (renderer && renderer->isText() && node->canStartSelection())) && !inResizer && !result.scrollbar()) return iBeam; return pointerCursor(); } @@ -1706,6 +1758,8 @@ bool EventHandler::handleMouseMoveEvent(const PlatformMouseEvent& mouseEvent, Hi if (m_hoverTimer.isActive()) m_hoverTimer.stop(); + m_cursorUpdateTimer.stop(); + cancelFakeMouseMoveEvent(); #if ENABLE(SVG) @@ -1778,7 +1832,7 @@ bool EventHandler::handleMouseMoveEvent(const PlatformMouseEvent& mouseEvent, Hi if (scrollbar && !m_mousePressed) scrollbar->mouseMoved(mouseEvent); // Handle hover effects on platforms that support visual feedback on scrollbar hovering. if (FrameView* view = m_frame->view()) { - OptionalCursor optionalCursor = selectCursor(mev, scrollbar); + OptionalCursor optionalCursor = selectCursor(mev.hitTestResult(), mouseEvent.shiftKey()); if (optionalCursor.isCursorChange()) { m_currentMouseCursor = optionalCursor.cursor(); view->setCursor(m_currentMouseCursor); @@ -2998,6 +3052,12 @@ void EventHandler::scheduleHoverStateUpdate() m_hoverTimer.startOneShot(0); } +void EventHandler::scheduleCursorUpdate() +{ + if (!m_cursorUpdateTimer.isActive()) + m_cursorUpdateTimer.startOneShot(cursorUpdateInterval); +} + void EventHandler::dispatchFakeMouseMoveEventSoon() { if (m_mousePressed) diff --git a/Source/WebCore/page/EventHandler.h b/Source/WebCore/page/EventHandler.h index c7ed793..18532f3 100644 --- a/Source/WebCore/page/EventHandler.h +++ b/Source/WebCore/page/EventHandler.h @@ -147,6 +147,7 @@ public: #endif void scheduleHoverStateUpdate(); + void scheduleCursorUpdate(); void setResizingFrameSet(HTMLFrameSetElement*); @@ -254,6 +255,7 @@ public: #endif bool useHandCursor(Node*, bool isOverLink, bool shiftKey); + void updateCursor(); private: #if ENABLE(DRAG_SUPPORT) @@ -280,8 +282,10 @@ private: #endif bool handleMouseReleaseEvent(const MouseEventWithHitTestResults&); - OptionalCursor selectCursor(const MouseEventWithHitTestResults&, Scrollbar*); + OptionalCursor selectCursor(const HitTestResult&, bool shiftKey); + void hoverTimerFired(Timer*); + void cursorUpdateTimerFired(Timer*); bool logicalScrollOverflow(ScrollLogicalDirection, ScrollGranularity, Node* startingNode = 0); @@ -416,6 +420,7 @@ private: bool m_panScrollButtonPressed; Timer m_hoverTimer; + Timer m_cursorUpdateTimer; OwnPtr m_autoscrollController; bool m_mouseDownMayStartAutoscroll; diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp index d5aeb41..59fd78c 100644 --- a/Source/WebCore/page/FrameView.cpp +++ b/Source/WebCore/page/FrameView.cpp @@ -1624,6 +1624,12 @@ IntPoint FrameView::lastKnownMousePosition() const return m_frame ? m_frame->eventHandler()->lastKnownMousePosition() : IntPoint(); } +bool FrameView::shouldSetCursor() const +{ + Page* page = frame()->page(); + return page && page->isOnscreen() && page->focusController()->isActive(); +} + bool FrameView::scrollContentsFastPath(const IntSize& scrollDelta, const IntRect& rectToScroll, const IntRect& clipRect) { if (!m_viewportConstrainedObjects || m_viewportConstrainedObjects->isEmpty()) { diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h index bba3bae..2950410 100644 --- a/Source/WebCore/page/FrameView.h +++ b/Source/WebCore/page/FrameView.h @@ -340,6 +340,7 @@ public: static void setRepaintThrottlingDeferredRepaintDelayIncrementDuringLoading(double p); virtual IntPoint lastKnownMousePosition() const; + bool shouldSetCursor() const; virtual bool scrollbarsCanBeActive() const OVERRIDE; diff --git a/Source/WebCore/page/MouseEventWithHitTestResults.cpp b/Source/WebCore/page/MouseEventWithHitTestResults.cpp index e33ddce..e030568 100644 --- a/Source/WebCore/page/MouseEventWithHitTestResults.cpp +++ b/Source/WebCore/page/MouseEventWithHitTestResults.cpp @@ -35,7 +35,7 @@ MouseEventWithHitTestResults::MouseEventWithHitTestResults(const PlatformMouseEv bool MouseEventWithHitTestResults::isOverLink() const { - return m_hitTestResult.URLElement() && m_hitTestResult.URLElement()->isLink(); + return m_hitTestResult.isOverLink(); } } diff --git a/Source/WebCore/rendering/HitTestResult.cpp b/Source/WebCore/rendering/HitTestResult.cpp index 5ff19c0..5f477f8 100644 --- a/Source/WebCore/rendering/HitTestResult.cpp +++ b/Source/WebCore/rendering/HitTestResult.cpp @@ -512,6 +512,11 @@ bool HitTestResult::isLiveLink() const return false; } +bool HitTestResult::isOverLink() const +{ + return m_innerURLElement && m_innerURLElement->isLink(); +} + String HitTestResult::titleDisplayString() const { if (!m_innerURLElement) diff --git a/Source/WebCore/rendering/HitTestResult.h b/Source/WebCore/rendering/HitTestResult.h index 25a6955..5cc506c 100644 --- a/Source/WebCore/rendering/HitTestResult.h +++ b/Source/WebCore/rendering/HitTestResult.h @@ -108,6 +108,7 @@ public: KURL absoluteLinkURL() const; String textContent() const; bool isLiveLink() const; + bool isOverLink() const; bool isContentEditable() const; void toggleMediaControlsDisplay() const; void toggleMediaLoopPlayback() const; diff --git a/Source/WebCore/rendering/RenderObject.cpp b/Source/WebCore/rendering/RenderObject.cpp index fb9f139..07c2907 100644 --- a/Source/WebCore/rendering/RenderObject.cpp +++ b/Source/WebCore/rendering/RenderObject.cpp @@ -2003,7 +2003,7 @@ void RenderObject::styleDidChange(StyleDifference diff, const RenderStyle* oldSt if (oldStyle && !areCursorsEqual(oldStyle, style())) { if (Frame* frame = this->frame()) - frame->eventHandler()->dispatchFakeMouseMoveEventSoon(); + frame->eventHandler()->scheduleCursorUpdate(); } } -- 1.8.3.1