Changeset 147022 in webkit


Ignore:
Timestamp:
Mar 27, 2013 5:01:41 PM (11 years ago)
Author:
rniwa@webkit.org
Message:

Shift clicking on an element with -webkit-user-select: all doesn't extend selection
​https://bugs.webkit.org/show_bug.cgi?id=113270

Reviewed by Enrica Casucci.

Source/WebCore:

The bug was caused by updateSelectionForMouseDownDispatchingSelectStart always replacing selection whenever
the target node was inside an element with -webkit-suer-select even when we were attemping to extend selection
in handleMousePressEventSingleClick.

Fixed the bug by extracting this logic as a separate function (expandSelectionToRespectUserSelectAll) and deploying
it in handleMousePressEventSingleClick to extend selection as needed.

Test: editing/selection/user-select-all-with-shift.html

  • page/EventHandler.cpp:

(WebCore::expandSelectionToRespectUserSelectAll): Extracted from updateSelectionForMouseDownDispatchingSelectStart.
(WebCore::EventHandler::updateSelectionForMouseDownDispatchingSelectStart):
(WebCore::EventHandler::selectClosestWordFromHitTestResult):
(WebCore::EventHandler::selectClosestWordOrLinkFromMouseEvent):
(WebCore::EventHandler::handleMousePressEventTripleClick):
(WebCore::EventHandler::handleMousePressEventSingleClick): Adjust "pos" as needed when extending selection.
Also use shouldConsiderSelectionAsDirectional() instead of manually peeking editingBehaviorType while we're at it.

LayoutTests:

Added a regression test for shift clicking on an element with -webkit-user-select: all.
Skip it on non-Mac platforms as -webkit-user-select: all hasn't been enabled on them.

  • editing/selection/user-select-all-with-shift-expected.txt: Added.
  • editing/selection/user-select-all-with-shift.html: Added.
  • platform/chromium/TestExpectations:
  • platform/efl/TestExpectations:
  • platform/gtk/TestExpectations:
  • platform/qt/TestExpectations:
  • platform/win/TestExpectations:
  • platform/wincairo/TestExpectations:
Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r147021 r147022  
     12013-03-27  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Shift clicking on an element with -webkit-user-select: all doesn't extend selection
     4        https://bugs.webkit.org/show_bug.cgi?id=113270
     5
     6        Reviewed by Enrica Casucci.
     7
     8        Added a regression test for shift clicking on an element with -webkit-user-select: all.
     9        Skip it on non-Mac platforms as -webkit-user-select: all hasn't been enabled on them.
     10
     11        * editing/selection/user-select-all-with-shift-expected.txt: Added.
     12        * editing/selection/user-select-all-with-shift.html: Added.
     13        * platform/chromium/TestExpectations:
     14        * platform/efl/TestExpectations:
     15        * platform/gtk/TestExpectations:
     16        * platform/qt/TestExpectations:
     17        * platform/win/TestExpectations:
     18        * platform/wincairo/TestExpectations:
     19
    1202013-03-27  Zalan Bujtas  <zalan@apple.com>
    221

  • trunk/LayoutTests/platform/chromium/TestExpectations

    r147001 r147022  
    34783478
    34793479webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
     3480webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
    34803481webkit.org/b/100475 compositing/tiling/tile-cache-zoomed.html [ Failure ]
    34813482webkit.org/b/100475 platform/chromium/virtual/softwarecompositing/tiling/tile-cache-zoomed.html [ Failure ]

  • trunk/LayoutTests/platform/efl/TestExpectations

    r147001 r147022  
    15311531
    15321532webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
     1533webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
    15331534
    15341535# Entering into full screen playback mode fails when triggered by context menu

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r147001 r147022  
    12351235
    12361236webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
     1237webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
    12371238
    12381239webkit.org/b/100846 inspector-protocol/debugger-pause-dedicated-worker.html [ Skip ]

  • trunk/LayoutTests/platform/qt/TestExpectations

    r147020 r147022  
    25702570
    25712571webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
     2572webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
    25722573
    25732574webkit.org/b/101539 editing/execCommand/switch-list-type-with-orphaned-li.html [ Failure ]

  • trunk/LayoutTests/platform/win/TestExpectations

    r147001 r147022  
    24462446
    24472447webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
     2448webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
    24482449
    24492450# https://bugs.webkit.org/show_bug.cgi?id=99567

  • trunk/LayoutTests/platform/wincairo/TestExpectations

    r146764 r147022  
    29312931
    29322932webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
     2933webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
    29332934
    29342935# https://bugs.webkit.org/show_bug.cgi?id=99567

  • trunk/Source/WebCore/ChangeLog

    r147021 r147022  
     12013-03-27  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Shift clicking on an element with -webkit-user-select: all doesn't extend selection
     4        https://bugs.webkit.org/show_bug.cgi?id=113270
     5
     6        Reviewed by Enrica Casucci.
     7
     8        The bug was caused by updateSelectionForMouseDownDispatchingSelectStart always replacing selection whenever
     9        the target node was inside an element with -webkit-suer-select even when we were attemping to extend selection
     10        in handleMousePressEventSingleClick.
     11
     12        Fixed the bug by extracting this logic as a separate function (expandSelectionToRespectUserSelectAll) and deploying
     13        it in handleMousePressEventSingleClick to extend selection as needed.
     14
     15        Test: editing/selection/user-select-all-with-shift.html
     16
     17        * page/EventHandler.cpp:
     18        (WebCore::expandSelectionToRespectUserSelectAll): Extracted from updateSelectionForMouseDownDispatchingSelectStart.
     19        (WebCore::EventHandler::updateSelectionForMouseDownDispatchingSelectStart):
     20        (WebCore::EventHandler::selectClosestWordFromHitTestResult):
     21        (WebCore::EventHandler::selectClosestWordOrLinkFromMouseEvent):
     22        (WebCore::EventHandler::handleMousePressEventTripleClick):
     23        (WebCore::EventHandler::handleMousePressEventSingleClick): Adjust "pos" as needed when extending selection.
     24        Also use shouldConsiderSelectionAsDirectional() instead of manually peeking editingBehaviorType while we're at it.
     25
    1262013-03-27  Zalan Bujtas  <zalan@apple.com>
    227

  • trunk/Source/WebCore/page/EventHandler.cpp

    r146961 r147022  
    443443}
    444444
    445 bool EventHandler::updateSelectionForMouseDownDispatchingSelectStart(Node* targetNode, const VisibleSelection& newSelection, TextGranularity granularity)
     445static VisibleSelection expandSelectionToRespectUserSelectAll(Node* targetNode, const VisibleSelection& selection)
     446{
     447#if ENABLE(USERSELECT_ALL)
     448    Node* rootUserSelectAll = Position::rootUserSelectAllForNode(targetNode);
     449    if (!rootUserSelectAll)
     450        return selection;
     451
     452    VisibleSelection newSelection(selection);
     453    newSelection.setBase(positionBeforeNode(rootUserSelectAll).upstream(CanCrossEditingBoundary));
     454    newSelection.setExtent(positionAfterNode(rootUserSelectAll).downstream(CanCrossEditingBoundary));
     455
     456    return newSelection;
     457#else
     458    return selection;
     459#endif
     460}
     461
     462bool EventHandler::updateSelectionForMouseDownDispatchingSelectStart(Node* targetNode, const VisibleSelection& selection, TextGranularity granularity)
    446463{
    447464    if (Position::nodeIsUserSelectNone(targetNode))
    … …  
    451468        return false;
    452469
    453     VisibleSelection selection(newSelection);
    454 #if ENABLE(USERSELECT_ALL)
    455     if (Node* rootUserSelectAll = Position::rootUserSelectAllForNode(targetNode)) {
    456         selection.setBase(positionBeforeNode(rootUserSelectAll).upstream(CanCrossEditingBoundary));
    457         selection.setExtent(positionAfterNode(rootUserSelectAll).downstream(CanCrossEditingBoundary));
    458     }
    459 #endif
    460470    if (selection.isRange())
    461471        m_selectionInitiationState = ExtendedSelection;
    … …  
    485495            newSelection.appendTrailingWhitespace();
    486496
    487         updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, WordGranularity);
     497        updateSelectionForMouseDownDispatchingSelectStart(innerNode, expandSelectionToRespectUserSelectAll(innerNode, newSelection), WordGranularity);
    488498    }
    489499}
    … …  
    511521            newSelection = VisibleSelection::selectionFromContentsOfNode(URLElement);
    512522
    513         updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, WordGranularity);
     523        updateSelectionForMouseDownDispatchingSelectStart(innerNode, expandSelectionToRespectUserSelectAll(innerNode, newSelection), WordGranularity);
    514524    }
    515525}
    … …  
    549559    }
    550560
    551     return updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, ParagraphGranularity);
     561    return updateSelectionForMouseDownDispatchingSelectStart(innerNode, expandSelectionToRespectUserSelectAll(innerNode, newSelection), ParagraphGranularity);
    552562}
    553563
    … …  
    587597
    588598    if (extendSelection && newSelection.isCaretOrRange()) {
    589         ASSERT(m_frame->settings());
    590         if (m_frame->settings()->editingBehaviorType() == EditingMacBehavior) {
     599        VisibleSelection selectionInUserSelectAll = expandSelectionToRespectUserSelectAll(innerNode, VisibleSelection(pos));
     600        if (selectionInUserSelectAll.isRange()) {
     601            if (comparePositions(selectionInUserSelectAll.start(), newSelection.start()) < 0)
     602                pos = selectionInUserSelectAll.start();
     603            else if (comparePositions(newSelection.end(), selectionInUserSelectAll.end()) < 0)
     604                pos = selectionInUserSelectAll.end();
     605        }
     606
     607        if (!m_frame->editor()->behavior().shouldConsiderSelectionAsDirectional()) {
    591608            // See <rdar://problem/3668157> REGRESSION (Mail): shift-click deselects when selection
    592609            // was created right-to-left
    … …  
    607624        }
    608625    } else
    609         newSelection = VisibleSelection(visiblePos);
    610    
     626        newSelection = expandSelectionToRespectUserSelectAll(innerNode, visiblePos);
     627
    611628    bool handled = updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, granularity);
    612629
Note: See TracChangeset for help on using the changeset viewer.