Commit 40285d7e authored by tonikitoo's avatar tonikitoo Committed by Commit bot

Selecting with shift+drag results in unexpected drag-n-drop

Whenever user tries to extend an existing text selection by dragging the mouse
(left button hold) with shift key pressed, Blink enters drag-n-drop mode.

This behavior does not match common editing behavior out there, including other
engines' (WebKit, Gecko/Firefox, Opera/Presto and IE).

Patch changes Blink so that whenever one extends a selection with mouse
and shift key pressed off of a #text node, it does not enter drag-n-drop mode.

Additionally, patch also adds some further tests to ensure that when
selection is extended off of either a link or an image, drag-n-drop does
get triggered, no matter if shift key is pressed.

Associated WebKit bug: https://bugs.webkit.org/show_bug.cgi?id=155314.

BUG=142023

Review-Url: https://codereview.chromium.org/2048733002
Cr-Commit-Position: refs/heads/master@{#403315}
parent febe3507
<!DOCTYPE html>
<script src='../../resources/testharness.js'></script>
<script src='../../resources/testharnessreport.js'></script>
<p>This test ensures drag-n-drop does not start when extending an existing selecting with shift + mouse drag, starting over a #text.</p>
<span id='span' style='font-size: 50px; padding: 10px;'>hello world</span>
<div id='log'></div>
<script>
test(function() {
assert_not_equals(window.testRunner, undefined, 'Requires testRunner.');
assert_not_equals(window.eventSender, undefined, 'Requires eventSender.');
var span = document.getElementById('span');
span.focus();
var dragStartCount = 0;
document.addEventListener('dragstart', function (event) { dragStartCount++; });
var y = span.offsetTop + span.offsetHeight / 2;
eventSender.mouseMoveTo(span.offsetLeft + 5, y);
eventSender.mouseDown();
eventSender.leapForward(200);
eventSender.mouseUp();
eventSender.mouseMoveTo(span.offsetLeft + span.offsetWidth / 4, y);
eventSender.mouseDown(0, ['shiftKey']);
eventSender.leapForward(200);
eventSender.mouseUp();
eventSender.mouseMoveTo(span.offsetLeft + span.offsetWidth / 2, y);
eventSender.mouseDown(0, ['shiftKey']);
eventSender.leapForward(200);
eventSender.mouseMoveTo(span.offsetLeft + span.offsetWidth, y);
eventSender.leapForward(200);
eventSender.mouseUp();
assert_equals(dragStartCount, 0);
});
</script>
<!DOCTYPE html>
<script src='../../resources/testharness.js'></script>
<script src='../../resources/testharnessreport.js'></script>
<p>This test ensures drag-n-drop does start when extending an existing selecting with shift + mouse drag, starting over an image.</p>
<span id='span' style='font-size: 30px; padding: 10px;'>Some text with an image <img id='img' src='resources/abe.png' width='50' height='50'> at end.</span>
<div id='log'></div>
<script>
test(function() {
assert_not_equals(window.testRunner, undefined, 'Requires testRunner.');
assert_not_equals(window.eventSender, undefined, 'Requires eventSender.');
var span = document.getElementById('span');
span.focus();
var img = document.getElementById('img');
var dragStartCount = 0;
document.addEventListener('dragstart', function (event) { dragStartCount++; });
var y = span.offsetTop + span.offsetHeight / 2;
eventSender.mouseMoveTo(span.offsetLeft + 5, y);
eventSender.mouseDown();
eventSender.leapForward(200);
eventSender.mouseUp();
eventSender.mouseMoveTo(span.offsetLeft + span.offsetWidth / 4, y);
eventSender.mouseDown(0, ['shiftKey']);
eventSender.leapForward(200);
eventSender.mouseUp();
eventSender.mouseMoveTo(img.offsetLeft + img.offsetWidth / 2 , y);
eventSender.mouseDown(0, ['shiftKey']);
eventSender.leapForward(200);
eventSender.mouseMoveTo(img.offsetLeft + img.offsetWidth / 2 , y + 120);
eventSender.leapForward(200);
eventSender.mouseUp();
assert_equals(dragStartCount, 1);
});
</script>
<!DOCTYPE html>
<script src='../../resources/testharness.js'></script>
<script src='../../resources/testharnessreport.js'></script>
<p>This test ensures drag-n-drop does start when extending an existing selecting with shift + mouse drag, starting over a link.</p>
<span id='span' style='font-size: 30px; padding: 10px;'>Some text with a <a href=#>link in the end of sentence</a>.</span>
<div id="log"></div>
<script>
test(function() {
assert_not_equals(window.testRunner, undefined, 'Requires testRunner.');
assert_not_equals(window.eventSender, undefined, 'Requires eventSender.');
var span = document.getElementById('span');
span.focus();
var dragStartCount = 0;
document.addEventListener('dragstart', function (event) { dragStartCount++; });
testRunner.dumpAsText();
var y = span.offsetTop + span.offsetHeight / 2;
eventSender.mouseMoveTo(span.offsetLeft + 5, y);
eventSender.mouseDown();
eventSender.leapForward(200);
eventSender.mouseUp();
eventSender.mouseMoveTo(span.offsetLeft + span.offsetWidth / 4, y);
eventSender.mouseDown(0, ['shiftKey']);
eventSender.leapForward(200);
eventSender.mouseUp();
eventSender.mouseMoveTo(span.offsetLeft + span.offsetWidth * .75 , y);
eventSender.mouseDown(0, ['shiftKey']);
eventSender.leapForward(200);
eventSender.mouseMoveTo(span.offsetLeft + span.offsetWidth * .75, y + 120);
eventSender.leapForward(200);
eventSender.mouseUp();
assert_equals(dragStartCount, 1);
});
</script>
......@@ -128,8 +128,8 @@ bool SelectionController::handleMousePressEventSingleClick(const MouseEventWithH
if (!(innerNode && innerNode->layoutObject() && m_mouseDownMayStartSelect))
return false;
// Extend the selection if the Shift key is down, unless the click is in a link.
bool extendSelection = event.event().shiftKey() && !event.isOverLink();
// Extend the selection if the Shift key is down, unless the click is in a link or image.
bool extendSelection = isExtendingSelection(event);
// Don't restart the selection when the mouse is pressed on an
// existing selection so we can allow for text dragging.
......@@ -670,4 +670,10 @@ bool isLinkSelection(const MouseEventWithHitTestResults& event)
return event.event().altKey() && event.isOverLink();
}
bool isExtendingSelection(const MouseEventWithHitTestResults& event)
{
bool isMouseDownOnLinkOrImage = event.isOverLink() || event.hitTestResult().image();
return event.event().shiftKey() && !isMouseDownOnLinkOrImage;
}
} // namespace blink
......@@ -90,6 +90,7 @@ private:
};
bool isLinkSelection(const MouseEventWithHitTestResults&);
bool isExtendingSelection(const MouseEventWithHitTestResults&);
} // namespace blink
......
......@@ -329,7 +329,7 @@ WebInputEventResult EventHandler::handleMousePressEvent(const MouseEventWithHitT
bool singleClick = event.event().clickCount() <= 1;
m_mouseDownMayStartDrag = singleClick && !isLinkSelection(event);
m_mouseDownMayStartDrag = singleClick && !isLinkSelection(event) && !isExtendingSelection(event);
selectionController().handleMousePressEvent(event);
......
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