Commit 9e76e369 authored by rniwa@webkit.org's avatar rniwa@webkit.org

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:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@147022 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 6ead1357
2013-03-27 Ryosuke Niwa <rniwa@webkit.org>
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.
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:
2013-03-27 Zalan Bujtas <zalan@apple.com>
REGRESSION(r143102): iframe with percentage height within table with anonymous cell fails.
This tests shift + selecting two discontinuous elements with user-select: all. WebKit should select the both elements instead of moving the selection.
To manually test, click to select the first element and shift-click the second element. WebKit should select both elements.
After clicking on the first element (Mac):
| "
"
| <div>
| class="select-all"
| id="first"
| "<#selection-anchor>First element<#selection-focus>"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "Second element"
| "
"
After shift clicking on the second element (Mac):
| "
"
| <div>
| class="select-all"
| id="first"
| "<#selection-anchor>First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "Second element<#selection-focus>"
| "
"
After clicking on the second element (Mac):
| "
"
| <div>
| class="select-all"
| id="first"
| "First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "<#selection-anchor>Second element<#selection-focus>"
| "
"
After shift clicking on the first element (Mac):
| "
"
| <div>
| class="select-all"
| id="first"
| "<#selection-focus>First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "Second element<#selection-anchor>"
| "
"
After clicking on the first element (Win):
| "
"
| <div>
| class="select-all"
| id="first"
| "<#selection-focus>First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "Second element<#selection-anchor>"
| "
"
After shift clicking on the second element (Win):
| "
"
| <div>
| class="select-all"
| id="first"
| "First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "<#selection-focus>Second element<#selection-anchor>"
| "
"
After clicking on the second element (Win):
| "
"
| <div>
| class="select-all"
| id="first"
| "First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "<#selection-anchor>Second element<#selection-focus>"
| "
"
After shift clicking on the first element (Win):
| "
"
| <div>
| class="select-all"
| id="first"
| "<#selection-focus>First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "Second<#selection-anchor> element"
| "
"
After clicking on the first element (Unix):
| "
"
| <div>
| class="select-all"
| id="first"
| "<#selection-focus>First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "Second<#selection-anchor> element"
| "
"
After shift clicking on the second element (Unix):
| "
"
| <div>
| class="select-all"
| id="first"
| "First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "<#selection-anchor>Second element<#selection-focus>"
| "
"
After clicking on the second element (Unix):
| "
"
| <div>
| class="select-all"
| id="first"
| "First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "<#selection-anchor>Second element<#selection-focus>"
| "
"
After shift clicking on the first element (Unix):
| "
"
| <div>
| class="select-all"
| id="first"
| "<#selection-focus>First element"
| "
Some other text.
"
| <div>
| class="select-all"
| id="second"
| "Second<#selection-anchor> element"
| "
"
<!DOCTYPE html>
<html>
<body>
<style>
.select-all {
border: 1px solid black;
height: 100px;
width: 100px;
-webkit-user-select: all;
-moz-user-select: all;
}
</style>
<p id="description">This tests shift + selecting two discontinuous elements with user-select: all. WebKit should select the both elements instead of moving the selection.
To manually test, click to select the first element and shift-click the second element. WebKit should select both elements.</p>
<div id="test">
<div id="first" class="select-all">First element</div>
Some other text.
<div id="second" class="select-all">Second element</div>
</div>
<script src="../../resources/dump-as-markup.js"></script>
<script>
Markup.description(document.getElementById('description').textContent);
function clickOnElement(element, keys) {
eventSender.mouseMoveTo(element.offsetLeft + 10, element.offsetTop + 10);
eventSender.mouseDown(0, keys);
eventSender.mouseUp(0, keys);
}
function runTest(editingBehavior) {
internals.settings.setEditingBehavior(editingBehavior);
var postfix = ' (' + editingBehavior + ')';
clickOnElement(document.getElementById('first'));
Markup.dump('test', 'After clicking on the first element' + postfix);
eventSender.leapForward(300);
clickOnElement(document.getElementById('second'), ['shiftKey']);
Markup.dump('test', 'After shift clicking on the second element' + postfix);
getSelection().removeAllRanges();
clickOnElement(document.getElementById('second'));
Markup.dump('test', 'After clicking on the second element' + postfix);
eventSender.leapForward(300);
clickOnElement(document.getElementById('first'), ['shiftKey']);
Markup.dump('test', 'After shift clicking on the first element' + postfix);
}
if (window.eventSender) {
runTest('Mac');
runTest('Win');
runTest('Unix');
} else
Markup.noAutoDump();
</script>
</body>
</html>
......@@ -3477,6 +3477,7 @@ webkit.org/b/100023 [ Mac ] fast/sub-pixel/file-upload-control-at-fractional-off
webkit.org/b/100478 [ Mac ] css3/filters/custom/effect-custom.html [ ImageOnlyFailure ]
webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
webkit.org/b/100475 compositing/tiling/tile-cache-zoomed.html [ Failure ]
webkit.org/b/100475 platform/chromium/virtual/softwarecompositing/tiling/tile-cache-zoomed.html [ Failure ]
......
......@@ -1530,6 +1530,7 @@ webkit.org/b/99691 gamepad/gamepad-out-of-range-crasher.html [ Failure ]
webkit.org/b/61138 http/tests/w3c/webperf/submission/Intel/resource-timing [ Skip ]
webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
# Entering into full screen playback mode fails when triggered by context menu
Bug(EFL) media/context-menu-actions.html [ Failure ]
......
......@@ -1234,6 +1234,7 @@ webkit.org/b/99825 accessibility/title-ui-element-correctness.html [ Failure ]
webkit.org/b/98718 svg/animations/animate-css-xml-attributeType.html [ Failure ]
webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
webkit.org/b/100846 inspector-protocol/debugger-pause-dedicated-worker.html [ Skip ]
webkit.org/b/100846 inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html [ Skip ]
......
......@@ -2569,6 +2569,7 @@ webkit.org/b/102226 fast/dom/Window/open-window-min-size.html
webkit.org/b/100196 http/tests/inspector/network/image-as-text-loading-data-url.html
webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
webkit.org/b/101539 editing/execCommand/switch-list-type-with-orphaned-li.html [ Failure ]
......
......@@ -2445,6 +2445,7 @@ webkit.org/b/84893 http/tests/w3c/webperf/submission/Intel/user-timing
webkit.org/b/61138 http/tests/w3c/webperf/submission/Intel/resource-timing
webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
# https://bugs.webkit.org/show_bug.cgi?id=99567
# Not supported on Mac or Windows ports
......
......@@ -2930,6 +2930,7 @@ webkit.org/b/84893 http/tests/w3c/webperf/submission/Intel/user-timing
webkit.org/b/61138 http/tests/w3c/webperf/submission/Intel/resource-timing
webkit.org/b/100424 editing/selection/user-select-all-selection.html [ Failure ]
webkit.org/b/100424 editing/selection/user-select-all-with-shift.html [ Failure ]
# https://bugs.webkit.org/show_bug.cgi?id=99567
# Not supported on Mac or Windows ports
......
2013-03-27 Ryosuke Niwa <rniwa@webkit.org>
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.
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.
2013-03-27 Zalan Bujtas <zalan@apple.com>
REGRESSION(r143102): iframe with percentage height within table with anonymous cell fails.
......@@ -442,7 +442,24 @@ static inline bool dispatchSelectStart(Node* node)
return node->dispatchEvent(Event::create(eventNames().selectstartEvent, true, true));
}
bool EventHandler::updateSelectionForMouseDownDispatchingSelectStart(Node* targetNode, const VisibleSelection& newSelection, TextGranularity granularity)
static VisibleSelection expandSelectionToRespectUserSelectAll(Node* targetNode, const VisibleSelection& selection)
{
#if ENABLE(USERSELECT_ALL)
Node* rootUserSelectAll = Position::rootUserSelectAllForNode(targetNode);
if (!rootUserSelectAll)
return selection;
VisibleSelection newSelection(selection);
newSelection.setBase(positionBeforeNode(rootUserSelectAll).upstream(CanCrossEditingBoundary));
newSelection.setExtent(positionAfterNode(rootUserSelectAll).downstream(CanCrossEditingBoundary));
return newSelection;
#else
return selection;
#endif
}
bool EventHandler::updateSelectionForMouseDownDispatchingSelectStart(Node* targetNode, const VisibleSelection& selection, TextGranularity granularity)
{
if (Position::nodeIsUserSelectNone(targetNode))
return false;
......@@ -450,13 +467,6 @@ bool EventHandler::updateSelectionForMouseDownDispatchingSelectStart(Node* targe
if (!dispatchSelectStart(targetNode))
return false;
VisibleSelection selection(newSelection);
#if ENABLE(USERSELECT_ALL)
if (Node* rootUserSelectAll = Position::rootUserSelectAllForNode(targetNode)) {
selection.setBase(positionBeforeNode(rootUserSelectAll).upstream(CanCrossEditingBoundary));
selection.setExtent(positionAfterNode(rootUserSelectAll).downstream(CanCrossEditingBoundary));
}
#endif
if (selection.isRange())
m_selectionInitiationState = ExtendedSelection;
else {
......@@ -484,7 +494,7 @@ void EventHandler::selectClosestWordFromHitTestResult(const HitTestResult& resul
if (appendTrailingWhitespace == ShouldAppendTrailingWhitespace && newSelection.isRange())
newSelection.appendTrailingWhitespace();
updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, WordGranularity);
updateSelectionForMouseDownDispatchingSelectStart(innerNode, expandSelectionToRespectUserSelectAll(innerNode, newSelection), WordGranularity);
}
}
......@@ -510,7 +520,7 @@ void EventHandler::selectClosestWordOrLinkFromMouseEvent(const MouseEventWithHit
if (pos.isNotNull() && pos.deepEquivalent().deprecatedNode()->isDescendantOf(URLElement))
newSelection = VisibleSelection::selectionFromContentsOfNode(URLElement);
updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, WordGranularity);
updateSelectionForMouseDownDispatchingSelectStart(innerNode, expandSelectionToRespectUserSelectAll(innerNode, newSelection), WordGranularity);
}
}
......@@ -548,7 +558,7 @@ bool EventHandler::handleMousePressEventTripleClick(const MouseEventWithHitTestR
newSelection.expandUsingGranularity(ParagraphGranularity);
}
return updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, ParagraphGranularity);
return updateSelectionForMouseDownDispatchingSelectStart(innerNode, expandSelectionToRespectUserSelectAll(innerNode, newSelection), ParagraphGranularity);
}
static int textDistance(const Position& start, const Position& end)
......@@ -586,8 +596,15 @@ bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestR
TextGranularity granularity = CharacterGranularity;
if (extendSelection && newSelection.isCaretOrRange()) {
ASSERT(m_frame->settings());
if (m_frame->settings()->editingBehaviorType() == EditingMacBehavior) {
VisibleSelection selectionInUserSelectAll = expandSelectionToRespectUserSelectAll(innerNode, VisibleSelection(pos));
if (selectionInUserSelectAll.isRange()) {
if (comparePositions(selectionInUserSelectAll.start(), newSelection.start()) < 0)
pos = selectionInUserSelectAll.start();
else if (comparePositions(newSelection.end(), selectionInUserSelectAll.end()) < 0)
pos = selectionInUserSelectAll.end();
}
if (!m_frame->editor()->behavior().shouldConsiderSelectionAsDirectional()) {
// See <rdar://problem/3668157> REGRESSION (Mail): shift-click deselects when selection
// was created right-to-left
Position start = newSelection.start();
......@@ -606,8 +623,8 @@ bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestR
newSelection.expandUsingGranularity(m_frame->selection()->granularity());
}
} else
newSelection = VisibleSelection(visiblePos);
newSelection = expandSelectionToRespectUserSelectAll(innerNode, visiblePos);
bool handled = updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, granularity);
if (event.event().button() == MiddleButton) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment