Changeset 87096 in webkit


Ignore:
Timestamp:
May 23, 2011 1:49:37 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

2011-05-23 Ryosuke Niwa <rniwa@webkit.org>

Reviewed by Alexey Proskuryakov.

selectstart is fired for every mouse move
https://bugs.webkit.org/show_bug.cgi?id=19489

Added tests to ensure selectstart is dispatches exactly once when selecting text by a mouse drag,
a mouse click, a double click, or a triple click. Also rebaselined a test that previously logged
extra selectstart dispatches.

  • fast/events/selectstart-by-double-triple-clicks-expected.txt: Added.
  • fast/events/selectstart-by-double-triple-clicks.html: Added.
  • fast/events/selectstart-by-drag.html:
  • fast/events/selectstart-by-single-click-with-shift-expected.txt: Added.
  • fast/events/selectstart-by-single-click-with-shift.html: Added.

2011-05-23 Ryosuke Niwa <rniwa@webkit.org>

Reviewed by Alexey Proskuryakov.

selectstart is fired for every mouse move
https://bugs.webkit.org/show_bug.cgi?id=19489

Fixed the bug by dispatching selectstart event immediately before modifying selection in
handleMousePressEventSingleClick and updateSelectionForMouseDrag.

Also replaced a boolean EventHandler::m_beganSelectingText by an enum-valued m_selectionInitiationState
to retain 3 states:

  1. HaveNotStartedSelection - Selection has not been set by a mouse drag or a mouse click
  2. PlacedCaret - A caret was placed by a mouse click, double click, or triple click, and is about to replace selection if a mouse drag never occurs.
  3. ExtendedSelection - A range selection was set by a mouse click, a double click, a triple click, or a mouse drag; otherwise a caret selection was set by a mouse drag.

State 1 corresponds to m_beganSelectingText being false and state 3 corresponds to m_beganSelectingText
being true. State 2 is used in updateSelectionForMouseDrag to avoid dispatching selectstart twice.

States 1 and 2 are set by updateSelectionForMouseDownDispatchingSelectStart and state 3 is set by
updateSelectionForMouseDrag.

Test: fast/events/selectstart-by-double-triple-clicks.html

fast/events/selectstart-by-drag.html
fast/events/selectstart-by-single-click-with-shift.html

  • page/EventHandler.cpp: Removed canMouseDragExtendSelect. (WebCore::EventHandler::EventHandler): Initializes m_selectionInitiationState. (WebCore::dispatchSelectStart): Returns true only if selectstart was successfully fired and default action was not prevented. (WebCore::EventHandler::updateSelectionForMouseDownDispatchingSelectStart): Updates m_selectionInitiationState and modifies the selection if dispatchSelectStart returns true. (WebCore::EventHandler::selectClosestWordFromMouseEvent): Calls updateSelectionForMouseDownDispatchingSelectStart. (WebCore::EventHandler::selectClosestWordOrLinkFromMouseEvent): Ditto. (WebCore::EventHandler::handleMousePressEventDoubleClick): (WebCore::EventHandler::handleMousePressEventTripleClick): Ditto. (WebCore::EventHandler::handleMousePressEventSingleClick): Ditto. (WebCore::canMouseDownStartSelect): No longer dispatches startselect; also renamed from EventHandler::canMouseDownStartSelect. (WebCore::EventHandler::handleMousePressEvent): No longer calls canMouseDragExtendSelect. (WebCore::EventHandler::handleMouseDraggedEvent): (WebCore::EventHandler::updateSelectionForMouseDrag): Exit early if m_selectionInitiationState is HaveNotStartedSelection and dispatchSelectStart returns false. Since setSelectionIfPossible dispatches selectstart event before assigning PlacedCaret or ExtendedSelection to m_selectionInitiationState, there is no need to dispatch event for those two cases. (WebCore::EventHandler::handleMouseReleaseEvent):
  • page/EventHandler.h: Removed canMouseDownStartSelect and canMouseDragExtendSelect from EventHandler and added setSelectionIfPossible.
Location:
trunk
Files:
6 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r87092 r87096  
     12011-05-23  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Alexey Proskuryakov.
     4
     5        selectstart is fired for every mouse move
     6        https://bugs.webkit.org/show_bug.cgi?id=19489
     7
     8        Added tests to ensure selectstart is dispatches exactly once when selecting text by a mouse drag,
     9        a mouse click, a double click, or a triple click. Also rebaselined a test that previously logged
     10        extra selectstart dispatches.
     11
     12        * fast/events/selectstart-by-double-triple-clicks-expected.txt: Added.
     13        * fast/events/selectstart-by-double-triple-clicks.html: Added.
     14        * fast/events/selectstart-by-drag.html:
     15        * fast/events/selectstart-by-single-click-with-shift-expected.txt: Added.
     16        * fast/events/selectstart-by-single-click-with-shift.html: Added.
     17
    1182011-05-22  Robert Hogan  <robert@webkit.org>
    219
  • trunk/LayoutTests/platform/mac/fast/events/objc-event-api-expected.txt

    r30635 r87096  
    342342  screenY:       -9999
    343343  modifier keys: c:0 s:0 a:0 m:0
    344 event type:      selectstart
    345   target:        <html>
    346   eventPhase:    3
    347   bubbles:       1
    348   cancelable:    1
    349344event type:      mouseup
    350345  target:        <html>
  • trunk/Source/WebCore/ChangeLog

    r87095 r87096  
     12011-05-23  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Alexey Proskuryakov.
     4
     5        selectstart is fired for every mouse move
     6        https://bugs.webkit.org/show_bug.cgi?id=19489
     7
     8        Fixed the bug by dispatching selectstart event immediately before modifying selection in
     9        handleMousePressEventSingleClick and updateSelectionForMouseDrag.
     10
     11        Also replaced a boolean EventHandler::m_beganSelectingText by an enum-valued m_selectionInitiationState
     12        to retain 3 states:
     13        1. HaveNotStartedSelection - Selection has not been set by a mouse drag or a mouse click
     14        2. PlacedCaret - A caret was placed by a mouse click, double click, or triple click, and is about
     15        to replace selection if a mouse drag never occurs.
     16        3. ExtendedSelection - A range selection was set by a mouse click, a double click, a triple click,
     17        or a mouse drag; otherwise a caret selection was set by a mouse drag.
     18
     19        State 1 corresponds to m_beganSelectingText being false and state 3 corresponds to m_beganSelectingText
     20        being true. State 2 is used in updateSelectionForMouseDrag to avoid dispatching selectstart twice.
     21
     22        States 1 and 2 are set by updateSelectionForMouseDownDispatchingSelectStart and state 3 is set by
     23        updateSelectionForMouseDrag.
     24
     25        Test: fast/events/selectstart-by-double-triple-clicks.html
     26              fast/events/selectstart-by-drag.html
     27              fast/events/selectstart-by-single-click-with-shift.html
     28
     29        * page/EventHandler.cpp: Removed canMouseDragExtendSelect.
     30        (WebCore::EventHandler::EventHandler): Initializes m_selectionInitiationState.
     31        (WebCore::dispatchSelectStart): Returns true only if selectstart was successfully fired
     32        and default action was not prevented.
     33        (WebCore::EventHandler::updateSelectionForMouseDownDispatchingSelectStart): Updates m_selectionInitiationState
     34        and modifies the selection if dispatchSelectStart returns true.
     35        (WebCore::EventHandler::selectClosestWordFromMouseEvent): Calls updateSelectionForMouseDownDispatchingSelectStart.
     36        (WebCore::EventHandler::selectClosestWordOrLinkFromMouseEvent): Ditto.
     37        (WebCore::EventHandler::handleMousePressEventDoubleClick):
     38        (WebCore::EventHandler::handleMousePressEventTripleClick): Ditto.
     39        (WebCore::EventHandler::handleMousePressEventSingleClick): Ditto.
     40        (WebCore::canMouseDownStartSelect): No longer dispatches startselect; also renamed from
     41        EventHandler::canMouseDownStartSelect.
     42        (WebCore::EventHandler::handleMousePressEvent): No longer calls canMouseDragExtendSelect.
     43        (WebCore::EventHandler::handleMouseDraggedEvent):
     44        (WebCore::EventHandler::updateSelectionForMouseDrag): Exit early if m_selectionInitiationState is
     45        HaveNotStartedSelection and dispatchSelectStart returns false. Since setSelectionIfPossible dispatches
     46        selectstart event before assigning PlacedCaret or ExtendedSelection to m_selectionInitiationState,
     47        there is no need to dispatch event for those two cases.
     48        (WebCore::EventHandler::handleMouseReleaseEvent):
     49        * page/EventHandler.h: Removed canMouseDownStartSelect and canMouseDragExtendSelect from EventHandler
     50        and added setSelectionIfPossible.
     51
    1522011-05-23  Adam Klein  <adamk@chromium.org>
    253
  • trunk/Source/WebCore/page/EventHandler.cpp

    r86741 r87096  
    183183#endif
    184184    , m_mouseDownWasSingleClickInSelection(false)
    185     , m_beganSelectingText(false)
     185    , m_selectionInitiationState(HaveNotStartedSelection)
    186186    , m_panScrollInProgress(false)
    187187    , m_panScrollButtonPressed(false)
     
    282282}
    283283
     284static inline bool dispatchSelectStart(Node* node)
     285{
     286    if (!node || !node->renderer())
     287        return true;
     288
     289    return node->dispatchEvent(Event::create(eventNames().selectstartEvent, true, true));
     290}
     291
     292bool EventHandler::updateSelectionForMouseDownDispatchingSelectStart(Node* targetNode, const VisibleSelection& newSelection, TextGranularity granularity)
     293{
     294    if (!dispatchSelectStart(targetNode))
     295        return false;
     296
     297    if (newSelection.isRange())
     298        m_selectionInitiationState = ExtendedSelection;
     299    else {
     300        granularity = CharacterGranularity;
     301        m_selectionInitiationState = PlacedCaret;
     302    }
     303
     304    setNonDirectionalSelectionIfNeeded(m_frame->selection(), newSelection, granularity);
     305
     306    return true;
     307}
     308
    284309void EventHandler::selectClosestWordFromMouseEvent(const MouseEventWithHitTestResults& result)
    285310{
     
    289314    if (innerNode && innerNode->renderer() && m_mouseDownMayStartSelect) {
    290315        VisiblePosition pos(innerNode->renderer()->positionForPoint(result.localPoint()));
    291         TextGranularity granularity = CharacterGranularity;
    292316        if (pos.isNotNull()) {
    293317            newSelection = VisibleSelection(pos);
    294318            newSelection.expandUsingGranularity(WordGranularity);
    295319        }
    296    
    297         if (newSelection.isRange()) {
    298             granularity = WordGranularity;
    299             m_beganSelectingText = true;
    300             if (result.event().clickCount() == 2 && m_frame->editor()->isSelectTrailingWhitespaceEnabled())
    301                 newSelection.appendTrailingWhitespace();           
    302         }
    303 
    304         setNonDirectionalSelectionIfNeeded(m_frame->selection(), newSelection, granularity);
     320
     321        if (newSelection.isRange() && result.event().clickCount() == 2 && m_frame->editor()->isSelectTrailingWhitespaceEnabled())
     322            newSelection.appendTrailingWhitespace();
     323
     324        updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, WordGranularity);
    305325    }
    306326}
     
    319339        if (pos.isNotNull() && pos.deepEquivalent().deprecatedNode()->isDescendantOf(URLElement))
    320340            newSelection = VisibleSelection::selectionFromContentsOfNode(URLElement);
    321    
    322         TextGranularity granularity = CharacterGranularity;
    323         if (newSelection.isRange()) {
    324             granularity = WordGranularity;
    325             m_beganSelectingText = true;
    326         }
    327 
    328         setNonDirectionalSelectionIfNeeded(m_frame->selection(), newSelection, granularity);
     341
     342        updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, WordGranularity);
    329343    }
    330344}
     
    341355        // m_beganSelectingText to prevent handleMouseReleaseEvent
    342356        // from setting caret selection.
    343         m_beganSelectingText = true;
     357        m_selectionInitiationState = ExtendedSelection;
    344358    else
    345359        selectClosestWordFromMouseEvent(event);
     
    363377        newSelection.expandUsingGranularity(ParagraphGranularity);
    364378    }
    365    
    366     TextGranularity granularity = CharacterGranularity;
    367     if (newSelection.isRange()) {
    368         granularity = ParagraphGranularity;
    369         m_beganSelectingText = true;
    370     }
    371 
    372     setNonDirectionalSelectionIfNeeded(m_frame->selection(), newSelection, granularity);
    373 
    374     return true;
     379
     380    return updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, ParagraphGranularity);
    375381}
    376382
     
    423429            else
    424430                newSelection = VisibleSelection(start, pos);
    425         } else {
     431        } else
    426432            newSelection.setExtent(pos);
    427         }
    428433
    429434        if (m_frame->selection()->granularity() != CharacterGranularity) {
     
    431436            newSelection.expandUsingGranularity(m_frame->selection()->granularity());
    432437        }
    433 
    434         m_beganSelectingText = true;
    435438    } else
    436439        newSelection = VisibleSelection(visiblePos);
    437 
    438     setNonDirectionalSelectionIfNeeded(m_frame->selection(), newSelection, granularity);
     440   
     441    return updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, granularity);
     442}
     443
     444static inline bool canMouseDownStartSelect(Node* node)
     445{
     446    if (!node || !node->renderer())
     447        return true;
     448
     449    if (!node->canStartSelection())
     450        return false;
    439451
    440452    return true;
     
    498510    bool swallowEvent = false;
    499511    m_mousePressed = true;
    500     m_beganSelectingText = false;
     512    m_selectionInitiationState = HaveNotStartedSelection;
    501513
    502514    if (event.event().clickCount() == 2)
     
    574586    }
    575587
    576     if (!m_beganSelectingText) {
     588    if (m_selectionInitiationState != ExtendedSelection) {
    577589        HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::Active);
    578590        HitTestResult result(m_mouseDownPos);
     
    662674        return;
    663675
    664     if (!canMouseDragExtendSelect(target))
    665         return;
    666 
    667676    VisiblePosition targetPosition = selectionExtentRespectingEditingBoundary(m_frame->selection()->selection(), hitTestResult.localPoint(), target);
    668677
     
    685694#endif
    686695
    687     if (!m_beganSelectingText) {
    688         m_beganSelectingText = true;
     696    if (m_selectionInitiationState == HaveNotStartedSelection && !dispatchSelectStart(target))
     697        return;
     698
     699    if (m_selectionInitiationState != ExtendedSelection) {
     700        // Always extend selection here because it's caused by a mouse drag
     701        m_selectionInitiationState = ExtendedSelection;
    689702        newSelection = VisibleSelection(targetPosition);
    690703    }
     
    742755    // on the selection, the selection goes away.  However, if we are
    743756    // editing, place the caret.
    744     if (m_mouseDownWasSingleClickInSelection && !m_beganSelectingText
     757    if (m_mouseDownWasSingleClickInSelection && m_selectionInitiationState != ExtendedSelection
    745758#if ENABLE(DRAG_SUPPORT)
    746759            && m_dragStartPos == event.event().pos()
     
    23352348}
    23362349
    2337 // Whether or not a mouse down can begin the creation of a selection.  Fires the selectStart event.
    2338 bool EventHandler::canMouseDownStartSelect(Node* node)
    2339 {
    2340     if (!node || !node->renderer())
    2341         return true;
    2342    
    2343     // Some controls and images can't start a select on a mouse down.
    2344     if (!node->canStartSelection())
    2345         return false;
    2346            
    2347     return node->dispatchEvent(Event::create(eventNames().selectstartEvent, true, true));
    2348 }
    2349 
    2350 #if ENABLE(DRAG_SUPPORT)
    2351 bool EventHandler::canMouseDragExtendSelect(Node* node)
    2352 {
    2353     if (!node || !node->renderer())
    2354         return true;
    2355            
    2356     return node->dispatchEvent(Event::create(eventNames().selectstartEvent, true, true));
    2357 }
    2358 #endif // ENABLE(DRAG_SUPPORT)
    2359 
    23602350void EventHandler::setResizingFrameSet(HTMLFrameSetElement* frameSet)
    23612351{
  • trunk/Source/WebCore/page/EventHandler.h

    r86741 r87096  
    3434#include "ScrollTypes.h"
    3535#include "TextEventInputType.h"
     36#include "TextGranularity.h"
    3637#include "Timer.h"
    3738#include <wtf/Forward.h>
     
    7273class TextEvent;
    7374class TouchEvent;
     75class VisibleSelection;
    7476class WheelEvent;
    7577class Widget;
     
    231233
    232234    bool eventActivatedView(const PlatformMouseEvent&) const;
     235    bool updateSelectionForMouseDownDispatchingSelectStart(Node*, const VisibleSelection&, TextGranularity);
    233236    void selectClosestWordFromMouseEvent(const MouseEventWithHitTestResults&);
    234237    void selectClosestWordOrLinkFromMouseEvent(const MouseEventWithHitTestResults&);
     
    256259
    257260    void hoverTimerFired(Timer<EventHandler>*);
    258 
    259     static bool canMouseDownStartSelect(Node*);
    260 #if ENABLE(DRAG_SUPPORT)
    261     static bool canMouseDragExtendSelect(Node*);
    262 #endif
    263261
    264262    void handleAutoscroll(RenderObject*);
     
    368366#endif
    369367    bool m_mouseDownWasSingleClickInSelection;
    370     bool m_beganSelectingText;
     368    enum SelectionInitiationState { HaveNotStartedSelection, PlacedCaret, ExtendedSelection };
     369    SelectionInitiationState m_selectionInitiationState;
    371370
    372371#if ENABLE(DRAG_SUPPORT)
Note: See TracChangeset for help on using the changeset viewer.