Changeset 142861 in webkit


Ignore:
Timestamp:
Feb 14, 2013 2:14:24 AM (11 years ago)
Author:
commit-queue@webkit.org
Message:

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 <aivopaas@gmail.com> on 2013-02-14
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); it has extra overhead of
using a timer which was introduced to make scrolling smoother.

The new approach does not use the fake mousemove event. Instead, it uses only the logic
needed for the actual cursor change to happen. This bypasses all the mousemove event related
overhead. The remaining code is a stripped version of what was run through the mousemove
event path. Everything that was not needed for changing a cursor is stripped off, everything
that is needed, remains the same.

The call to update cursor was moved up in the call tree from RenderObject::StyleDidChange
to RenderObject::SetStyle right after the StyleDidChange call. This allows to any updates
and style propagations in StyleDidChange to happen and makes sure that a cursor change is
not missed. Previous place was at the end of RenderObject::StyleDidChange, where it could
have been missed because of an early exit. For example, cursor change on mousedown/up on
a text node missed the correct cursor in the first pass.

Refactored EventHandler::selectCursor 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::updateCursor): Newly added method for updating mouse cursor
(WebCore):
(WebCore::EventHandler::selectCursor):
(WebCore::EventHandler::handleMouseMoveEvent):

  • page/EventHandler.h:

(EventHandler):

  • rendering/RenderObject.cpp:

(WebCore::areNonIdenticalCursorListsEqual):
(WebCore):
(WebCore::areCursorsEqual):
(WebCore::RenderObject::setStyle):
(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 <aivopaas@gmail.com> on 2013-02-14
Reviewed by Allan Sandfeld Jensen.

Added tests for changing cursor on mousemove, mousedown, mouseup and mousemove
while mouse button being hold down. Also added test to verify that a mousemove
event is not fired for changing cursor while 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:
Location:
trunk
Files:
4 added
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r142856 r142861  
     12013-02-14  Aivo Paas  <aivopaas@gmail.com>
     2
     3        Updating mouse cursor on style changes without emitting fake mousemove event
     4        https://bugs.webkit.org/show_bug.cgi?id=101857
     5        Changing CSS cursor should work no matter is mouse button is pressed or not
     6        https://bugs.webkit.org/show_bug.cgi?id=53341
     7
     8        Reviewed by Allan Sandfeld Jensen.
     9
     10        Added tests for changing cursor on mousemove, mousedown, mouseup and mousemove
     11        while mouse button being hold down. Also added test to verify that a mousemove
     12        event is not fired for changing cursor while mouse is not moving.
     13
     14        * fast/events/mouse-cursor-change-expected.txt: Added.
     15        * fast/events/mouse-cursor-change.html: Added.
     16        * fast/events/mouse-cursor-no-mousemove-expected.txt: Added.
     17        * fast/events/mouse-cursor-no-mousemove.html: Added.
     18        * platform/mac/TestExpectations:
     19
    1202013-02-06  Gregg Tavares  <gman@chromium.org>
    221
  • trunk/LayoutTests/platform/mac/TestExpectations

    r142856 r142861  
    14131413# Crashing after https://webkit.org/b/105667
    14141414webkit.org/b/109232 [ Debug ] inspector/debugger/debugger-reload-on-pause.html [ Crash ]
     1415
     1416# Mac fails cursor change test for unknown reasons
     1417webkit.org/b/103857 fast/events/mouse-cursor-change.html
  • trunk/Source/WebCore/ChangeLog

    r142858 r142861  
     12013-02-14  Aivo Paas  <aivopaas@gmail.com>
     2
     3        Updating mouse cursor on style changes without emitting fake mousemove event
     4        https://bugs.webkit.org/show_bug.cgi?id=101857
     5
     6        Reviewed by Allan Sandfeld Jensen.
     7
     8        Mouse cursor changes in styles used to be reflected in UI through dispatching a fake
     9        mousemove event. The old approach has some flaws: it emits a mousemove event in
     10        javascript when there is no mouse movement involved (bug 85343); the fake mousemove
     11        event is cancelled while there is a mouse button held down - cursor won't change
     12        until mouse is moved or the button released (bug 53341); it has extra overhead of
     13        using a timer which was introduced to make scrolling smoother.
     14
     15        The new approach does not use the fake mousemove event. Instead, it uses only the logic
     16        needed for the actual cursor change to happen. This bypasses all the mousemove event related
     17        overhead. The remaining code is a stripped version of what was run through the mousemove
     18        event path. Everything that was not needed for changing a cursor is stripped off, everything
     19        that is needed, remains the same.
     20
     21        The call to update cursor was moved up in the call tree from RenderObject::StyleDidChange
     22        to RenderObject::SetStyle right after the StyleDidChange call. This allows to any updates
     23        and style propagations in StyleDidChange to happen and makes sure that a cursor change is
     24        not missed. Previous place was at the end of RenderObject::StyleDidChange, where it could
     25        have been missed because of an early exit. For example, cursor change on mousedown/up on
     26        a text node missed the correct cursor in the first pass.
     27
     28        Refactored EventHandler::selectCursor to not take a whole mouse event but instead work with
     29        HitTestResult so that EventHandler::updateCursor must not create a useless PlatformEvent.
     30
     31        Fixes: https://bugs.webkit.org/show_bug.cgi?id=85343 (mousemove event on cursor change)
     32               https://bugs.webkit.org/show_bug.cgi?id=53341 (no cursor change when mouse button down)
     33
     34        Tests: fast/events/mouse-cursor-change.html
     35               fast/events/mouse-cursor-no-mousemove.html
     36
     37        * page/EventHandler.cpp:
     38        (WebCore::EventHandler::updateCursor): Newly added method for updating mouse cursor
     39        (WebCore):
     40        (WebCore::EventHandler::selectCursor):
     41        (WebCore::EventHandler::handleMouseMoveEvent):
     42        * page/EventHandler.h:
     43        (EventHandler):
     44        * rendering/RenderObject.cpp:
     45        (WebCore::areNonIdenticalCursorListsEqual):
     46        (WebCore):
     47        (WebCore::areCursorsEqual):
     48        (WebCore::RenderObject::setStyle):
     49        (WebCore::RenderObject::styleDidChange):
     50
    1512013-02-13  Ilya Tikhonovsky  <loislo@chromium.org>
    252
  • trunk/Source/WebCore/page/EventHandler.cpp

    r142375 r142861  
    12381238}
    12391239
    1240 OptionalCursor EventHandler::selectCursor(const MouseEventWithHitTestResults& event, Scrollbar* scrollbar)
     1240void EventHandler::updateCursor()
     1241{
     1242    if (m_mousePositionIsUnknown)
     1243        return;
     1244
     1245    Settings* settings = m_frame->settings();
     1246    if (settings && !settings->deviceSupportsMouse())
     1247        return;
     1248
     1249    FrameView* view = m_frame->view();
     1250    if (!view)
     1251        return;
     1252
     1253    if (!m_frame->page() || !m_frame->page()->isOnscreen() || !m_frame->page()->focusController()->isActive())
     1254        return;
     1255
     1256    bool shiftKey;
     1257    bool ctrlKey;
     1258    bool altKey;
     1259    bool metaKey;
     1260    PlatformKeyboardEvent::getCurrentModifierState(shiftKey, ctrlKey, altKey, metaKey);
     1261
     1262    HitTestRequest request(HitTestRequest::ReadOnly);
     1263    HitTestResult result(view->windowToContents(m_lastKnownMousePosition));
     1264    m_frame->document()->renderView()->hitTest(request, result);
     1265
     1266    OptionalCursor optionalCursor = selectCursor(result, shiftKey);
     1267    if (optionalCursor.isCursorChange()) {
     1268        m_currentMouseCursor = optionalCursor.cursor();
     1269        view->setCursor(m_currentMouseCursor);
     1270    }
     1271}
     1272
     1273OptionalCursor EventHandler::selectCursor(const HitTestResult& result, bool shiftKey)
    12411274{
    12421275    if (m_resizeLayer && m_resizeLayer->inResizeMode())
     
    12511284#endif
    12521285
    1253     Node* node = event.targetNode();
    1254     RenderObject* renderer = node ? node->renderer() : 0;
     1286    Node* node = result.targetNode();
     1287    if (!node)
     1288        return NoCursorChange;
     1289    bool originalIsText = node->isTextNode();
     1290    if (node && originalIsText)
     1291        node = node->parentNode();
     1292    if (!node)
     1293        return NoCursorChange;
     1294
     1295    RenderObject* renderer = node->renderer();
    12551296    RenderStyle* style = renderer ? renderer->style() : 0;
    12561297    bool horizontalText = !style || style->isHorizontalWritingMode();
     
    12681309    if (renderer) {
    12691310        Cursor overrideCursor;
    1270         switch (renderer->getCursor(roundedIntPoint(event.localPoint()), overrideCursor)) {
     1311        switch (renderer->getCursor(roundedIntPoint(result.localPoint()), overrideCursor)) {
    12711312        case SetCursorBasedOnStyle:
    12721313            break;
     
    13151356    switch (style ? style->cursor() : CURSOR_AUTO) {
    13161357    case CURSOR_AUTO: {
    1317         bool editable = (node && node->rendererIsEditable());
    1318 
    1319         if (useHandCursor(node, event.isOverLink(), event.event().shiftKey()))
     1358        bool editable = (node->rendererIsEditable());
     1359
     1360        if (useHandCursor(node, result.URLElement() && result.URLElement()->isLink(), shiftKey))
    13201361            return handCursor();
    13211362
     
    13241365            if (RenderLayer* layer = renderer->enclosingLayer()) {
    13251366                if (FrameView* view = m_frame->view())
    1326                     inResizer = layer->isPointInResizeControl(view->windowToContents(event.event().position()));
     1367                    inResizer = layer->isPointInResizeControl(view->windowToContents(roundedIntPoint(result.localPoint())));
    13271368            }
    13281369        }
    1329         if ((editable || (renderer && renderer->isText() && node->canStartSelection())) && !inResizer && !scrollbar)
     1370        if ((editable || (originalIsText && node->canStartSelection())) && !inResizer && !result.scrollbar())
    13301371            return iBeam;
    13311372        return pointerCursor();
     
    17381779            scrollbar->mouseMoved(mouseEvent); // Handle hover effects on platforms that support visual feedback on scrollbar hovering.
    17391780        if (FrameView* view = m_frame->view()) {
    1740             OptionalCursor optionalCursor = selectCursor(mev, scrollbar);
     1781            OptionalCursor optionalCursor = selectCursor(mev.hitTestResult(), mouseEvent.shiftKey());
    17411782            if (optionalCursor.isCursorChange()) {
    17421783                m_currentMouseCursor = optionalCursor.cursor();
  • trunk/Source/WebCore/page/EventHandler.h

    r140177 r142861  
    252252
    253253    bool useHandCursor(Node*, bool isOverLink, bool shiftKey);
     254    void updateCursor();
    254255
    255256private:
     
    278279    bool handleMouseReleaseEvent(const MouseEventWithHitTestResults&);
    279280
    280     OptionalCursor selectCursor(const MouseEventWithHitTestResults&, Scrollbar*);
     281    OptionalCursor selectCursor(const HitTestResult&, bool shiftKey);
     282
    281283    void hoverTimerFired(Timer<EventHandler>*);
    282284
  • trunk/Source/WebCore/rendering/RenderObject.cpp

    r142500 r142861  
    17591759}
    17601760
     1761static bool areNonIdenticalCursorListsEqual(const RenderStyle* a, const RenderStyle* b)
     1762{
     1763    ASSERT(a->cursors() != b->cursors());
     1764    return a->cursors() && b->cursors() && *a->cursors() == *b->cursors();
     1765}
     1766
     1767static inline bool areCursorsEqual(const RenderStyle* a, const RenderStyle* b)
     1768{
     1769    return a->cursor() == b->cursor() && (a->cursors() == b->cursors() || areNonIdenticalCursorListsEqual(a, b));
     1770}
     1771
    17611772void RenderObject::setStyle(PassRefPtr<RenderStyle> style)
    17621773{
     
    17961807
    17971808    styleDidChange(diff, oldStyle.get());
     1809
     1810    if (oldStyle.get() && !areCursorsEqual(oldStyle.get(), this->style())) {
     1811        if (Frame* frame = this->frame())
     1812            frame->eventHandler()->updateCursor();
     1813    }
    17981814
    17991815    // FIXME: |this| might be destroyed here. This can currently happen for a RenderTextFragment when
     
    19221938}
    19231939
    1924 static bool areNonIdenticalCursorListsEqual(const RenderStyle* a, const RenderStyle* b)
    1925 {
    1926     ASSERT(a->cursors() != b->cursors());
    1927     return a->cursors() && b->cursors() && *a->cursors() == *b->cursors();
    1928 }
    1929 
    1930 static inline bool areCursorsEqual(const RenderStyle* a, const RenderStyle* b)
    1931 {
    1932     return a->cursor() == b->cursor() && (a->cursors() == b->cursors() || areNonIdenticalCursorListsEqual(a, b));
    1933 }
    1934 
    19351940void RenderObject::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
    19361941{
     
    19661971    // Don't check for repaint here; we need to wait until the layer has been
    19671972    // updated by subclasses before we know if we have to repaint (in setStyle()).
    1968 
    1969     if (oldStyle && !areCursorsEqual(oldStyle, style())) {
    1970         if (Frame* frame = this->frame())
    1971             frame->eventHandler()->dispatchFakeMouseMoveEventSoon();
    1972     }
    19731973}
    19741974
Note: See TracChangeset for help on using the changeset viewer.