Commit f9b0cf64 authored by rbyers@chromium.org's avatar rbyers@chromium.org

Prevent ctrl+wheel from scrolling in more places

Chromium now sends ctrl+wheel events to webkit first before using them for zooming. Rather than prevent scrolling for these events in EventHandler::handleWheel, push that check a couple levels deeper into ScrollableArea::handleWheelEvent.  In addition to EventHandler, this code is also called by WebPluginScrollbarImpl and so will fix some additional cases.

This is mostly already tested by the fast/event/wheelevent-ctrl.html test I added in http://crrev.com/183403002.  I've extended that test to cover document and div scrolling cases and ensured it fails reliably if either the ScrollableArea or EventHandler::defaultWheelEventHandler check is removed. 

BUG=352474, 111059

Review URL: https://codereview.chromium.org/212913002

git-svn-id: svn://svn.chromium.org/blink/trunk@170238 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 25ff2577
...@@ -7,19 +7,35 @@ Scroll mouse wheel over here ...@@ -7,19 +7,35 @@ Scroll mouse wheel over here
END END
And scroll the document here
Tests that wheel events with the ctrl modifier are handled properly Tests that wheel events with the ctrl modifier are handled properly
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Test mousewheel events over scrollable div
With ctrl modifier set
PASS wheelEventCount is 1 PASS wheelEventCount is 1
PASS deltaY is 80 PASS deltaY is 80
PASS ctrlKey is true PASS ctrlKey is true
PASS testDiv.scrollTop is 0 PASS testDiv.scrollTop is 0
PASS wheelEventCount is 2 Without ctrl
PASS wheelEventCount is 1
PASS deltaY is 80 PASS deltaY is 80
PASS ctrlKey is false PASS ctrlKey is false
PASS testDiv.scrollTop is deltaY PASS testDiv.scrollTop is deltaY
Test mousewheel events over the document
With ctrl modifier set
PASS wheelEventCount is 1
PASS deltaY is 80
PASS ctrlKey is true
PASS document.body.scrollTop is 0
Now without ctrl
PASS wheelEventCount is 1
PASS deltaY is 80
PASS ctrlKey is false
PASS window.scrollY is deltaY
PASS successfullyParsed is true PASS successfullyParsed is true
TEST COMPLETE TEST COMPLETE
......
...@@ -12,13 +12,16 @@ var testDiv; ...@@ -12,13 +12,16 @@ var testDiv;
function runTest() { function runTest() {
testDiv = document.getElementById('target'); testDiv = document.getElementById('target');
testDiv.addEventListener('wheel', wheelHandler); document.addEventListener('wheel', wheelHandler);
if (!window.eventSender) { if (!window.eventSender) {
debug("FAIL: This test requires window.eventSender."); debug("FAIL: This test requires window.eventSender.");
return; return;
} }
// Verify that wheel with ctrl modifier is fired property but doesn't scroll. debug('Test mousewheel events over scrollable div');
debug('With ctrl modifier set');
wheelEventCount = 0;
eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5); eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5);
eventSender.mouseScrollBy(0, scrollAmount, false, true, "ctrlKey"); eventSender.mouseScrollBy(0, scrollAmount, false, true, "ctrlKey");
shouldBe("wheelEventCount", "1"); shouldBe("wheelEventCount", "1");
...@@ -26,13 +29,36 @@ function runTest() { ...@@ -26,13 +29,36 @@ function runTest() {
shouldBeTrue("ctrlKey"); shouldBeTrue("ctrlKey");
shouldBe("testDiv.scrollTop", "0"); shouldBe("testDiv.scrollTop", "0");
// Verify that without the ctrl modifier we scroll as normal. debug('Without ctrl');
wheelEventCount = 0;
eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5); eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5);
eventSender.mouseScrollBy(0, scrollAmount, false, true); eventSender.mouseScrollBy(0, scrollAmount, false, true);
shouldBe("wheelEventCount", "2"); shouldBe("wheelEventCount", "1");
shouldEvaluateTo("deltaY", expectedDeltaY); shouldEvaluateTo("deltaY", expectedDeltaY);
shouldBeFalse("ctrlKey"); shouldBeFalse("ctrlKey");
shouldBe("testDiv.scrollTop", "deltaY"); shouldBe("testDiv.scrollTop", "deltaY");
debug('');
debug('Test mousewheel events over the document');
testDiv = document.getElementById('target2');
debug('With ctrl modifier set');
wheelEventCount = 0;
eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5);
eventSender.mouseScrollBy(0, scrollAmount, false, true, "ctrlKey");
shouldBe("wheelEventCount", "1");
shouldEvaluateTo("deltaY", expectedDeltaY);
shouldBeTrue("ctrlKey");
shouldBe("document.body.scrollTop", "0");
debug('Now without ctrl');
wheelEventCount = 0;
eventSender.mouseMoveTo(testDiv.offsetLeft + 5, testDiv.offsetTop + 5);
eventSender.mouseScrollBy(0, scrollAmount, false, true);
shouldBe("wheelEventCount", "1");
shouldEvaluateTo("deltaY", expectedDeltaY);
shouldBeFalse("ctrlKey");
shouldBe("window.scrollY", "deltaY");
} }
var wheelEventCount = 0; var wheelEventCount = 0;
...@@ -52,6 +78,10 @@ function wheelHandler(e) { ...@@ -52,6 +78,10 @@ function wheelHandler(e) {
Scroll mouse wheel over here<br/><br/><br/><br/> Scroll mouse wheel over here<br/><br/><br/><br/>
END END
</div> </div>
<div id="target2" style="border:solid 1px blue;">
And scroll the document here
</div>
<div style="height: 2000px;"></div>
</span> </span>
<div id="console"></div> <div id="console"></div>
<script> <script>
......
...@@ -2201,13 +2201,6 @@ bool EventHandler::handleWheelEvent(const PlatformWheelEvent& e) ...@@ -2201,13 +2201,6 @@ bool EventHandler::handleWheelEvent(const PlatformWheelEvent& e)
RETURN_WHEEL_EVENT_HANDLED(); RETURN_WHEEL_EVENT_HANDLED();
} }
// Ctrl + scrollwheel is reserved for triggering zoom in/out actions in Chromium.
// When Ctrl is pressed and the event was not canceled by JavaScript code,
// return false to notify the caller that the scrollwheel event was not canceled.
if (e.ctrlKey())
return false;
// We do another check on the frame view because the event handler can run JS which results in the frame getting destroyed. // We do another check on the frame view because the event handler can run JS which results in the frame getting destroyed.
view = m_frame->view(); view = m_frame->view();
if (!view || !view->wheelEvent(event)) if (!view || !view->wheelEvent(event))
......
...@@ -213,6 +213,10 @@ bool ScrollableArea::scrollBehaviorFromString(const String& behaviorString, Scro ...@@ -213,6 +213,10 @@ bool ScrollableArea::scrollBehaviorFromString(const String& behaviorString, Scro
bool ScrollableArea::handleWheelEvent(const PlatformWheelEvent& wheelEvent) bool ScrollableArea::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
{ {
// ctrl+wheel events are used to trigger zooming, not scrolling.
if (wheelEvent.modifiers() & PlatformEvent::CtrlKey)
return false;
return scrollAnimator()->handleWheelEvent(wheelEvent); return scrollAnimator()->handleWheelEvent(wheelEvent);
} }
......
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