Commit dd2d3501 authored by David Bokan's avatar David Bokan Committed by Commit Bot

Fix autoscrolling coordinate-space bug

This bug would only affect the non-root-layer-scrolling path and only
when the page is scrolled. This CL fixes the issue and cleans up the
surrounding code a bit.

I also took the opportunity to cleanup the drag-and-drop autoscrolling
tests:
 - Modernized using testharness.js and new test style guidelines
 - I've based them all on the drag-and-drop-autoscroll-frame.js
   script for commonality.
 - All the tests now occur at a non-0 scroll offset to make sure we
   catch these kinds of coordinate space bugs.

Change-Id: I3191796917f23b2e9b2cc3f561813176fa2dec9a
Reviewed-on: https://chromium-review.googlesource.com/972148
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarSandra Sun <sunyunjia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545076}
parent 45733135
For manual testing, drag and drop "Drop Me" to "Scrollable" area.
Check autoscroll by drag-and-drop
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS scrollable.scrollTop > 0
PASS autoscroll stopped
state=START
state=NE
PASS document.scrollingElement.scrollLeft > 0 is true
PASS !document.scrollingElement.scrollTop is true
state=SE
PASS document.scrollingElement.scrollLeft > 0 is true
PASS document.scrollingElement.scrollTop > 0 is true
state=SW
PASS document.scrollingElement.scrollLeft < lastScrollLeft is true
PASS document.scrollingElement.scrollTop > 0 is true
state=NW
PASS document.scrollingElement.scrollLeft <= lastScrollLeft is true
PASS document.scrollingElement.scrollTop < lastScrollTop is true
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script> <script>
window.addEventListener('message', function(event) { window.onload = () => {
document.body.outerHTML = event.data; fetch_tests_from_window(frames[0]);
testRunner.notifyDone(); }
});
</script> </script>
<frameset cols="50%,%50%"> <frameset cols="50%,50%">
<frame src="resources/drag-and-drop-autoscroll-frame.html"> <frame src="resources/drag-and-drop-autoscroll-frame.html">
<frame> <frame>
</frameset> </frameset>
Check autoscroll within an inner frame by drag-and-drop
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS Autoscroll should have scrolled the iframe downwards, and did.
PASS iframe.contentDocument.scrollingElement.scrollTop < middleTermScrollOffset is true
PASS successfullyParsed is true
TEST COMPLETE
For manual testing, drag and drop "Drop Me" downwards and then upwards.
<!DOCTYPE html> <!DOCTYPE html>
<head> <script src="../../resources/testharness.js"></script>
<style type="text/css"> <script src="../../resources/testharnessreport.js"></script>
#scrollable {
height: 200px;
overflow: auto;
border: solid 3px #cc0000;
font-size: 80px;
}
</style>
</head>
<body>
For manual testing, drag and drop "Drop Me" downwards and then upwards.
<script src="../../resources/js-test.js"></script>
<script> <script>
window.onload = () => {
var x, y, middleTermScrollOffset; fetch_tests_from_window(frames[0]);
var iframe, iframeDocument, draggable; }
function setUpTest()
{
if (!window.eventSender) {
debug('Please run within DumpRenderTree');
return;
}
iframe = document.getElementById('scrollable');
// iframes start in readyState complete, but with the about:blank doc, make sure our doc is loaded.
if (iframe.contentWindow.location.href == "about:blank" || iframe.contentDocument.readyState != "complete")
iframe.onload = testIt;
else
testIt();
}
function testIt()
{
eventSender.dragMode = false;
iframe = document.getElementById('scrollable');
iframeDocument = iframe.contentDocument;
draggable = iframeDocument.getElementById('draggable');
iframeDocument.addEventListener("scroll", recordScroll);
// Grab draggable.
x = iframe.offsetLeft + draggable.offsetLeft + 7;
y = iframe.offsetTop + draggable.offsetTop + 7;
eventSender.mouseMoveTo(x, y);
eventSender.mouseDown();
// Move mouse to the bottom autoscroll border belt.
y = iframe.offsetTop + iframe.offsetHeight - 10;
eventSender.mouseMoveTo(x, y);
}
function recordScroll(e)
{
autoscrollTestPart1();
iframeDocument.removeEventListener("scroll", recordScroll);
}
function recordScroll2(e)
{
autoscrollTestPart2();
iframeDocument.removeEventListener("scroll", recordScroll);
}
function autoscrollTestPart1()
{
if (iframe.contentDocument.scrollingElement.scrollTop == 0) {
testFailed("Autoscroll should have scrolled the iframe downwards, but did not");
finishTest();
return;
}
testPassed("Autoscroll should have scrolled the iframe downwards, and did.");
middleTermScrollOffset = iframe.contentDocument.scrollingElement.scrollTop;
iframeDocument.addEventListener("scroll", recordScroll2);
// Move mouse to the upper autoscroll border belt.
y = iframe.offsetTop + 10;
eventSender.mouseMoveTo(x, y);
}
function autoscrollTestPart2()
{
shouldBeTrue("iframe.contentDocument.scrollingElement.scrollTop < middleTermScrollOffset")
finishTest();
}
function finishTest()
{
eventSender.mouseUp();
document.body.removeChild(iframe);
finishJSTest();
}
description('Check autoscroll within an inner frame by drag-and-drop');
window.jsTestIsAsync = true;
window.onload = setUpTest;
</script> </script>
<iframe id="scrollable" src="data:text/html, <style>
<p id='draggable' draggable='true' style='cursor: hand;'> iframe {
<b>Drag me!</b> position: relative;
</p> top: 100px;
Try to drag and drop the text above in the input element at the bottom of this iframe. It should scroll. Then, try the way back. width: 300px;
<br><br>more<br>more<br>more<br>more<br>more<br>more<br>more<br>more<br>more<br>more<br>more<br>more<br><input> height: 300px;
"></iframe> }
</body> </style>
For manual testing, drag and drop "Drop Me" downwards and then upwards.
<iframe src="resources/drag-and-drop-autoscroll-frame.html"></iframe>
state=START
state=NE
PASS document.scrollingElement.scrollLeft > 0 is true
PASS !document.scrollingElement.scrollTop is true
state=SE
PASS document.scrollingElement.scrollLeft > 0 is true
PASS document.scrollingElement.scrollTop > 0 is true
state=SW
PASS document.scrollingElement.scrollLeft < lastScrollLeft is true
PASS document.scrollingElement.scrollTop > 0 is true
state=NW
PASS document.scrollingElement.scrollLeft <= lastScrollLeft is true
PASS document.scrollingElement.scrollTop < lastScrollTop is true
PASS successfullyParsed is true
TEST COMPLETE
<html> <!DOCTYPE html>
<head> <script src="../../resources/testharness.js"></script>
<style type="text/css"> <script src="../../resources/testharnessreport.js"></script>
#draggable {
padding: 5pt;
border: 3px solid #00cc00;
background: #00cccc;
width: 80px;
cursor: hand;
}
</style>
<script src="resources/drag-and-drop-autoscroll-frame.js"></script> <script src="resources/drag-and-drop-autoscroll-frame.js"></script>
</head> <style>
<body> #draggable {
padding: 5pt;
border: 3px solid #00cc00;
background: #00cccc;
width: 80px;
cursor: pointer;
}
#container {
position: relative;
left: 2000px;
top: 2000px;
}
#sample {
width: 2000px;
height: 2000px;
}
body,html {
margin: 0;
}
</style>
<div id="container"> <div id="container">
<p id="description"></p> Manual steps:
Manual steps: <ol>
<ol> <li>Drag "Drop Me" to edge of window</li>
<li>Drag "Drop Me" to edge of window</li> <li>You should see scrolling</li>
<li>You should see scrolling</li> </ol>
</ol> <div id="draggable" draggable="true">Drop Me</div>
<div id="draggable" draggable="true">Drop Me</div> <div id="scrollbars" style="overflow: scroll"><div>scrollbars</div></div>
<div id="scrollbars" style="overflow: scroll"><div>scrollbars</div></div> <table id="sample" border="1">
<table id="sample" border="1" style="width:2000; height:2000;"> <tr><td width="50%">North West</td><td width="50%">North East</td></tr>
<tr><td width="50%">North West</td><td width="50%">North East</td></tr> <tr><td width="50%">South West</td><td width="50%">South East</td></tr>
<tr><td width="50%">South West</td><td width="50%">South East</td></tr> </table>
</table>
</div> </div>
<div id="console"></div>
<script src="../../resources/js-test.js"></script>
<script>
if (window.testRunner)
window.jsTestIsAsync = true;
description('Check autoscroll of main frame by drag-and-drop');
</script>
</body>
</html>
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
border: 3px solid #00cc00; border: 3px solid #00cc00;
background: #00cccc; background: #00cccc;
width: 80px; width: 80px;
cursor: hand; cursor: pointer;
} }
#scrollable { #scrollable {
height: 200px; height: 200px;
......
<html> <!DOCTYPE html>
<head> <script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="resources/drag-and-drop-autoscroll-frame.js"></script>
<style type="text/css"> <style type="text/css">
#draggable { #draggable {
padding: 5pt; padding: 5pt;
border: 3px solid #00cc00; border: 3px solid #00cc00;
background: #00cccc; background: #00cccc;
width: 80px; width: 80px;
cursor: hand; cursor: pointer;
} }
#scrollable {
#scrollable { height: 400px;
height: 200px; width: 400px;
overflow: auto; overflow: auto;
border: solid 3px #cc0000; font-size: 80px;
font-size: 80px;
}
</style>
<script>
function $(id) { return document.getElementById(id); }
function finishTest() {
eventSender.mouseUp();
$('container').innerHTML = '';
window.testRunner.notifyDone();
}
function testIt() {
var draggable = $('draggable');
var scrollable = $('scrollable');
if (!window.eventSender)
return;
eventSender.dragMode = false;
// Grab draggable
eventSender.mouseMoveTo(draggable.offsetLeft + 5, draggable.offsetTop + 5);
eventSender.mouseDown();
// Move mouse to autoscroll border belt.
eventSender.mouseMoveTo(scrollable.offsetLeft + 5, scrollable.offsetTop + scrollable.offsetHeight - 10);
var retryCount = 0;
var lastScrollTop = 0;
function checkScrolled()
{
if (scrollable.scrollTop > 0) {
testPassed('scrollable.scrollTop > 0');
lastScrollTop = scrollable.scrollTop;
// Cancel drag and drop by ESC key.
eventSender.keyDown('Escape');
retryCount = 0;
window.setTimeout(checkStopped, 50);
return;
}
++retryCount;
if (retryCount > 10) {
testFailed('No autoscroll');
finishTest();
return;
}
// Autoscroll is occurred evey 0.05 sec.
window.setTimeout(checkScrolled, 50);
} }
#container {
function checkStopped() position: relative;
{ left: 2000px;
if (lastScrollTop == scrollable.scrollTop) { top: 2000px;
testPassed('autoscroll stopped');
finishTest();
return;
}
++retryCount;
if (retryCount > 10) {
testFailed('still autoscroll');
finishTest();
return;
}
lastScrollTop = scrollable.scrollTop;
window.setTimeout(checkStopped, 50);
} }
#sample {
checkScrolled(); width: 2000px;
} height: 2000px;
function setUpTest()
{
var scrollable = $('scrollable');
for (var i = 0; i < 100; ++i) {
var line = document.createElement('div');
line.innerHTML = "line " + i;
scrollable.appendChild(line);
} }
body {
if (!window.eventSender) { margin: 0;
console.log('Please run within DumpRenderTree');
return;
} }
</style>
window.jsTestIsAsync = true;
window.setTimeout(testIt, 0);
}
</script>
</head>
<body>
For manual testing, drag and drop "Drop Me" to "Scrollable" area. For manual testing, drag and drop "Drop Me" to "Scrollable" area.
<div id="container">
<div id="draggable" draggable="true">Drop Me</div> <div id="draggable" draggable="true">Drop Me</div>
<div id="scrollbars" style="overflow: scroll"><div>scrollbars</div></div>
Scrollable Scrollable
<div id="scrollable"> <div id="scrollable">
<div id="container">
<table id="sample" border="1">
<tr><td width="50%">North West</td><td width="50%">North East</td></tr>
<tr><td width="50%">South West</td><td width="50%">South East</td></tr>
</table>
</div>
</div> </div>
</div>
<div id="console"></div>
<script src="../../resources/js-test.js"></script>
<script>
description('Check autoscroll by drag-and-drop');
setUpTest();
</script>
</body>
</html>
<html> <!DOCTYPE html>
<head> <script src="../../../resources/testharness.js"></script>
<script src="drag-and-drop-autoscroll-frame.js"></script>
<style type="text/css"> <style type="text/css">
#draggable { #draggable {
padding: 5pt; padding: 5pt;
border: 3px solid #00cc00; border: 3px solid #00cc00;
background: #00cccc; background: #00cccc;
width: 80px; width: 80px;
cursor: hand; cursor: pointer;
} }
#container {
position: relative;
left: 2000px;
top: 2000px;
}
#sample {
width: 2000px;
height: 2000px;
}
body,html {
margin: 0;
}
</style> </style>
<script src="drag-and-drop-autoscroll-frame.js"></script>
</head>
<body>
<div id="container"> <div id="container">
<p id="description"></p> Manual steps:
Manual steps: <ol>
<ol> <li>Drag "Drop Me" to edge of window</li>
<li>Drag "Drop Me" to edge of window</li> <li>You should see scrolling</li>
<li>You should see scrolling</li> </ol>
</ol> <div id="draggable" draggable="true">Drop Me</div>
<div id="draggable" draggable="true">Drop Me</div> <div id="scrollbars" style="overflow: scroll"><div>scrollbars</div></div>
<div id="scrollbars" style="overflow: scroll"><div>scrollbars</div></div> <table id="sample" border="1">
<table id="sample" border="1" style="width:2000; height:2000;"> <tr><td width="50%">North West</td><td width="50%">North East</td></tr>
<tr><td width="50%">North West</td><td width="50%">North East</td></tr> <tr><td width="50%">South West</td><td width="50%">South East</td></tr>
<tr><td width="50%">South West</td><td width="50%">South East</td></tr> </table>
</table>
</div> </div>
<div id="console"></div>
<script src="../../../resources/js-test.js"></script>
<script>
if (window.testRunner)
window.jsTestIsAsync = true;
description('Check autoscroll of frame by drag-and-drop');
</script>
</body>
</html>
function $(id) { return document.getElementById(id); } function $(id) { return document.getElementById(id); }
// Convert client coordinates in this frame into client coordinates of the root
// frame, usable with event sender.
function toRootWindow(rect) {
var w = window;
var curRect = {
left: rect.left,
top: rect.top,
right: rect.right,
bottom: rect.bottom
};
while(w.parent != w) {
var frameRect = w.frameElement.getBoundingClientRect();
curRect.left += frameRect.left;
curRect.right += frameRect.left;
curRect.top += frameRect.top;
curRect.bottom += frameRect.top;
w = window.parent;
}
return curRect
}
var lastScrollLeft; var lastScrollLeft;
var lastScrollTop; var lastScrollTop;
window.onload = function() { window.onload = function() {
const test = async_test("DragAndDrop Autoscroll");
test.add_cleanup(() => {
eventSender.mouseUp();
});
var draggable = $('draggable'); var draggable = $('draggable');
var sample = $('sample');
var scrollBarWidth = $('scrollbars').offsetWidth - $('scrollbars').firstChild.offsetWidth; var scrollBarWidth = $('scrollbars').offsetWidth - $('scrollbars').firstChild.offsetWidth;
var scrollBarHeight = $('scrollbars').offsetHeight - $('scrollbars').firstChild.offsetHeight; var scrollBarHeight = $('scrollbars').offsetHeight - $('scrollbars').firstChild.offsetHeight;
var eastX = window.innerWidth - scrollBarWidth - 10;
var northY = 10; var scroller = $('scrollable');
var southY= window.innerHeight - scrollBarHeight - 10; var scrollerRect;
var westX = 10; if (scroller) {
scrollerRect = scroller.getBoundingClientRect();
} else {
scroller = document.scrollingElement;
scrollerRect = {
left: 0,
top: 0,
right: window.innerWidth,
bottom: window.innerHeight,
};
}
scrollerRect = toRootWindow(scrollerRect);
var eastX = scrollerRect.right - scrollBarWidth - 10;
var northY = scrollerRect.top + 10;
var southY= scrollerRect.bottom - scrollBarHeight - 10;
var westX = scrollerRect.left + 10;
function moveTo(newState, x, y) function moveTo(newState, x, y)
{ {
state = newState; state = newState;
lastScrollLeft = document.scrollingElement.scrollLeft; lastScrollLeft = scroller.scrollLeft;
lastScrollTop = document.scrollingElement.scrollTop; lastScrollTop = scroller.scrollTop;
eventSender.mouseMoveTo(x, y); eventSender.mouseMoveTo(x, y);
} }
var state = 'START'; var state = 'START';
function process(event) function process(event)
{ {
debug('state=' + state);
switch (state) { switch (state) {
case 'NE': case 'NE':
shouldBeTrue('document.scrollingElement.scrollLeft > 0'); test.step(() => {
shouldBeTrue('!document.scrollingElement.scrollTop'); assert_greater_than(
moveTo('SE', westX, southY); scroller.scrollLeft,
lastScrollLeft,
"NE should scroll right");
assert_less_than(
scroller.scrollTop,
lastScrollTop,
"NE should scroll up");
});
moveTo('SE', eastX, southY);
break; break;
case 'SE': case 'SE':
shouldBeTrue('document.scrollingElement.scrollLeft > 0'); test.step(() => {
shouldBeTrue('document.scrollingElement.scrollTop > 0'); assert_greater_than(
scroller.scrollLeft,
lastScrollLeft,
"SE should scroll right");
assert_greater_than(
scroller.scrollTop,
lastScrollTop,
"SE should scroll down");
});
moveTo('SW', westX, southY); moveTo('SW', westX, southY);
break; break;
case 'SW': case 'SW':
shouldBeTrue('document.scrollingElement.scrollLeft < lastScrollLeft'); test.step(() => {
shouldBeTrue('document.scrollingElement.scrollTop > 0'); assert_less_than(
scroller.scrollLeft,
lastScrollLeft,
"SW should scroll left");
assert_greater_than(
scroller.scrollTop,
lastScrollTop,
"SW should scroll down");
});
moveTo('NW', westX, northY); moveTo('NW', westX, northY);
break; break;
case 'NW': case 'NW':
shouldBeTrue('document.scrollingElement.scrollLeft <= lastScrollLeft'); test.step(() => {
shouldBeTrue('document.scrollingElement.scrollTop < lastScrollTop'); assert_less_than(
eventSender.mouseUp(); scroller.scrollLeft,
$('container').outerHTML = ''; lastScrollLeft,
if (window.parent !== window) "NW should scroll left");
window.jsTestIsAsync = false; assert_less_than(
finishJSTest(); scroller.scrollTop,
if (!window.jsTestIsAsync) lastScrollTop,
window.parent.postMessage($('console').innerHTML, '*'); "NW should scroll up");
});
test.done();
state = 'DONE'; state = 'DONE';
break; break;
case 'DONE': case 'DONE':
...@@ -58,20 +126,27 @@ window.onload = function() { ...@@ -58,20 +126,27 @@ window.onload = function() {
moveTo('NE', eastX, northY); moveTo('NE', eastX, northY);
break; break;
default: default:
testFailed('Bad state ' + state); console.error('Bad state ' + state);
break; break;
} }
}; };
if (!window.eventSender) if (!window.eventSender) {
$("container").scrollIntoView();
return; return;
}
if (scroller === document.scrollingElement)
window.addEventListener('scroll', process);
else
scroller.addEventListener('scroll', process);
$("container").scrollIntoView();
eventSender.dragMode = false; eventSender.dragMode = false;
// Grab draggable // Grab draggable
eventSender.mouseMoveTo(draggable.offsetLeft + 5, draggable.offsetTop + 5); const draggable_rect = toRootWindow(draggable.getBoundingClientRect());
eventSender.mouseMoveTo(draggable_rect.left + 5, draggable_rect.top + 5);
eventSender.mouseDown(); eventSender.mouseDown();
window.onscroll = process;
process();
}; };
...@@ -1084,8 +1084,9 @@ bool LayoutBox::CanAutoscroll() const { ...@@ -1084,8 +1084,9 @@ bool LayoutBox::CanAutoscroll() const {
return CanBeScrolledAndHasScrollableArea(); return CanBeScrolledAndHasScrollableArea();
} }
// If specified point is in border belt, returned offset denotes direction of // If specified point is outside the border-belt-excluded box (the border box
// scrolling. // inset by the autoscroll activation threshold), returned offset denotes
// direction of scrolling.
IntSize LayoutBox::CalculateAutoscrollDirection( IntSize LayoutBox::CalculateAutoscrollDirection(
const IntPoint& point_in_root_frame) const { const IntPoint& point_in_root_frame) const {
if (!GetFrame()) if (!GetFrame())
...@@ -1095,34 +1096,36 @@ IntSize LayoutBox::CalculateAutoscrollDirection( ...@@ -1095,34 +1096,36 @@ IntSize LayoutBox::CalculateAutoscrollDirection(
if (!frame_view) if (!frame_view)
return IntSize(); return IntSize();
LayoutRect box(AbsoluteBoundingBoxRect()); LayoutRect absolute_scrolling_box;
// TODO(bokan): This is wrong. Subtracting the scroll offset would get you to
// frame coordinates (pre-RLS) but *adding* the scroll offset to an absolute if (!RuntimeEnabledFeatures::RootLayerScrollingEnabled() && IsLayoutView()) {
// location never makes sense (and we assume below it's in content absolute_scrolling_box =
// coordinates). LayoutRect(frame_view->VisibleContentRect(kExcludeScrollbars));
box.Move(View()->GetFrameView()->ScrollOffsetInt()); } else {
absolute_scrolling_box = LayoutRect(AbsoluteBoundingBoxRect());
// Exclude scrollbars so the border belt (activation area) starts from the
// scrollbar-content edge rather than the window edge. // Exclude scrollbars so the border belt (activation area) starts from the
ExcludeScrollbars(box, kExcludeOverlayScrollbarSizeForHitTesting); // scrollbar-content edge rather than the window edge.
ExcludeScrollbars(absolute_scrolling_box,
IntRect window_box = kExcludeOverlayScrollbarSizeForHitTesting);
View()->GetFrameView()->ContentsToRootFrame(PixelSnappedIntRect(box)); }
IntPoint window_autoscroll_point = point_in_root_frame;
IntRect belt_box = View()->GetFrameView()->AbsoluteToRootFrame(
if (window_autoscroll_point.X() < window_box.X() + kAutoscrollBeltSize) PixelSnappedIntRect(absolute_scrolling_box));
window_autoscroll_point.Move(-kAutoscrollBeltSize, 0); belt_box.Inflate(-kAutoscrollBeltSize);
else if (window_autoscroll_point.X() > IntPoint point = point_in_root_frame;
window_box.MaxX() - kAutoscrollBeltSize)
window_autoscroll_point.Move(kAutoscrollBeltSize, 0); if (point.X() < belt_box.X())
point.Move(-kAutoscrollBeltSize, 0);
if (window_autoscroll_point.Y() < window_box.Y() + kAutoscrollBeltSize) else if (point.X() > belt_box.MaxX())
window_autoscroll_point.Move(0, -kAutoscrollBeltSize); point.Move(kAutoscrollBeltSize, 0);
else if (window_autoscroll_point.Y() >
window_box.MaxY() - kAutoscrollBeltSize) if (point.Y() < belt_box.Y())
window_autoscroll_point.Move(0, kAutoscrollBeltSize); point.Move(0, -kAutoscrollBeltSize);
else if (point.Y() > belt_box.MaxY())
return window_autoscroll_point - point_in_root_frame; point.Move(0, kAutoscrollBeltSize);
return point - point_in_root_frame;
} }
LayoutBox* LayoutBox::FindAutoscrollable(LayoutObject* layout_object) { LayoutBox* LayoutBox::FindAutoscrollable(LayoutObject* layout_object) {
......
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