Scrolling with spacebar on a page with fixed header breaks reading flow
https://bugs.webkit.org/show_bug.cgi?id=135506
Patch by Benjamin Poulain <bpoulain@apple.com> on 2014-08-28
Reviewed by Simon Fraser.
Source/WebCore:
When scrolling by page, find the height of any bar that is obscuring the top or bottom of the page,
and substract that height from the step to scroll.
Tests: scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar.html
scrollbars/scrolling-backward-by-page-on-keyboard-spacebar.html
scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar.html
scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar.html
scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar.html
scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar.html
scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar.html
scrollbars/scrolling-by-page-on-keyboard-spacebar.html
* WebCore.exp.in:
* page/FrameView.cpp:
(WebCore::FrameView::adjustScrollStepForFixedContent):
* page/FrameView.h:
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::adjustScrollStepForFixedContent):
(WebCore::ScrollableArea::scroll):
* platform/ScrollableArea.h:
LayoutTests:
There was pretty much no test coverage for scrolling by page, add some tests.
* fast/events/scrollbar-double-click-expected.txt:
* platform/mac-wk1/TestExpectations:
* scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
* scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar.html: Added.
* scrollbars/scrolling-backward-by-page-on-keyboard-spacebar-expected.txt: Added.
* scrollbars/scrolling-backward-by-page-on-keyboard-spacebar.html: Added.
* scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
* scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar.html: Added.
* scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
* scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar.html: Added.
* scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar-expected.txt: Added.
* scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar.html: Added.
* scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
* scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar.html: Added.
* scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
* scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar.html: Added.
* scrollbars/scrolling-by-page-on-keyboard-spacebar-expected.txt: Added.
* scrollbars/scrolling-by-page-on-keyboard-spacebar.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@173074 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 9acc2b6..a16893f 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,31 @@
+2014-08-28 Benjamin Poulain <bpoulain@apple.com>
+
+ Scrolling with spacebar on a page with fixed header breaks reading flow
+ https://bugs.webkit.org/show_bug.cgi?id=135506
+
+ Reviewed by Simon Fraser.
+
+ There was pretty much no test coverage for scrolling by page, add some tests.
+
+ * fast/events/scrollbar-double-click-expected.txt:
+ * platform/mac-wk1/TestExpectations:
+ * scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
+ * scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar.html: Added.
+ * scrollbars/scrolling-backward-by-page-on-keyboard-spacebar-expected.txt: Added.
+ * scrollbars/scrolling-backward-by-page-on-keyboard-spacebar.html: Added.
+ * scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
+ * scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar.html: Added.
+ * scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
+ * scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar.html: Added.
+ * scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar-expected.txt: Added.
+ * scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar.html: Added.
+ * scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
+ * scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar.html: Added.
+ * scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar-expected.txt: Added.
+ * scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar.html: Added.
+ * scrollbars/scrolling-by-page-on-keyboard-spacebar-expected.txt: Added.
+ * scrollbars/scrolling-by-page-on-keyboard-spacebar.html: Added.
+
2014-08-27 Filip Pizlo <fpizlo@apple.com>
FTL should be able to do polymorphic call inlining
diff --git a/LayoutTests/fast/events/scrollbar-double-click-expected.txt b/LayoutTests/fast/events/scrollbar-double-click-expected.txt
index 51e0acad..7a1ac5a 100644
--- a/LayoutTests/fast/events/scrollbar-double-click-expected.txt
+++ b/LayoutTests/fast/events/scrollbar-double-click-expected.txt
@@ -1 +1 @@
-Scroll offset is 700
+Scroll offset is 720
diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
index 44888b9..87811cd 100644
--- a/LayoutTests/platform/mac-wk1/TestExpectations
+++ b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -54,5 +54,15 @@
[ MountainLion Mavericks ] platform/mac/fast/scrolling/scroll-select-bottom-test.html [ Skip ]
[ MountainLion Mavericks ] platform/mac-wk1/fast/backgrounds/top-content-inset-fixed-attachment.html [ Skip ]
+# WK1 uses the native scrollview for scrolling by page.
+scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar.html
+scrollbars/scrolling-backward-by-page-on-keyboard-spacebar.html
+scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar.html
+scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar.html
+scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar.html
+scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar.html
+scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar.html
+scrollbars/scrolling-by-page-on-keyboard-spacebar.html
+
### END OF (2) Failures without bug reports
########################################
diff --git a/LayoutTests/scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar-expected.txt b/LayoutTests/scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar-expected.txt
new file mode 100644
index 0000000..ea76f9d
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar-expected.txt
@@ -0,0 +1,10 @@
+Test that scrolling backward by page excludes the area taken by fixed element covering the full width.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Scrolled to 520
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar.html b/LayoutTests/scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar.html
new file mode 100644
index 0000000..aca7302
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar.html
@@ -0,0 +1,76 @@
+<html>
+ <head>
+ <style>
+ body {
+ height: 10000px;
+ }
+ #bottom-bar {
+ position: fixed;
+ height: 120px;
+ width: 100%;
+ background-color: green;
+ bottom: 0;
+ left: 0;
+ }
+ /* This fixed element does not cover the entire width and should be ignored. */
+ #ignored-right {
+ position: fixed;
+ width: 20px;
+ height: 5000px;
+ background-color: red;
+ right: 0;
+ top: 0;
+ }
+ /* This fixed element does not cover an edge and should be ignored. */
+ #ignored-center {
+ position: fixed;
+ width: 100%;
+ height: 150px;
+ background-color: red;
+ left: 0;
+ top: 50%;
+ }
+ </style>
+ <script src="../resources/js-test-pre.js"></script>
+ </head>
+ <body>
+ <div id="bottom-bar"></div>
+ <div id="ignored-right"></div>
+ <div id="ignored-center"></div>
+ <script>
+ description("Test that scrolling backward by page excludes the area taken by fixed element covering the full width.");
+
+ jsTestIsAsync = true;
+
+ var failTimeoutId;
+ function test() {
+ if (window.eventSender) {
+ // Force the first layout to avoid the suppressed scrollbar cases.
+ scratch = document.documentElement.offsetWidth;
+ scrollBy(0, 1000);
+
+ // Avoid special cases for being "onload".
+ setTimeout(function() {
+ eventSender.keyDown(' ', ['shiftKey']);
+ } , 0);
+
+ failTimeoutId = setTimeout(function() {
+ testFailed("The scrollview failed to scroll in response to the event.");
+ debug("window.scrollY = " + window.scrollY + " excepted value around " + (1000 - (window.innerHeight - 120)));
+ finishJSTest();
+ }, 1000);
+ }
+ }
+
+ window.addEventListener("scroll", function() {
+ if (window.scrollY == 1000 - (window.innerHeight - 120)) {
+ testPassed("Scrolled to " + window.scrollY);
+ clearTimeout(failTimeoutId);
+ finishJSTest();
+ }
+ })
+ window.addEventListener("load", test);
+ </script>
+ <script src="../resources/js-test-post.js"></script>
+ </body>
+</html>
diff --git a/LayoutTests/scrollbars/scrolling-backward-by-page-on-keyboard-spacebar-expected.txt b/LayoutTests/scrollbars/scrolling-backward-by-page-on-keyboard-spacebar-expected.txt
new file mode 100644
index 0000000..9bc6bea
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-backward-by-page-on-keyboard-spacebar-expected.txt
@@ -0,0 +1,10 @@
+Test scrolling backward with page granularity by using the space bar.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Scrolled to 440
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/scrollbars/scrolling-backward-by-page-on-keyboard-spacebar.html b/LayoutTests/scrollbars/scrolling-backward-by-page-on-keyboard-spacebar.html
new file mode 100644
index 0000000..ac43438
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-backward-by-page-on-keyboard-spacebar.html
@@ -0,0 +1,47 @@
+<html>
+ <head>
+ <style>
+ body {
+ height: 10000px;
+ }
+ </style>
+ <script src="../resources/js-test-pre.js"></script>
+ </head>
+ <body>
+ <script>
+ description("Test scrolling backward with page granularity by using the space bar.");
+
+ jsTestIsAsync = true;
+
+ var failTimeoutId;
+ function test() {
+ if (window.eventSender) {
+ // Force the first layout to avoid the suppressed scrollbar cases.
+ scratch = document.documentElement.offsetWidth;
+ scrollBy(0, 1000);
+
+ // Avoid special cases for being "onload".
+ setTimeout(function() {
+ eventSender.keyDown(' ', ['shiftKey']);
+ } , 0);
+
+ failTimeoutId = setTimeout(function() {
+ testFailed("The scrollview failed to scroll in response to the event.");
+ debug("window.scrollY = " + window.scrollY + " excepted value around " + (1000 - (window.innerHeight - 40)));
+ finishJSTest();
+ }, 1000);
+ }
+ }
+
+ window.addEventListener("scroll", function() {
+ if (window.scrollY == 1000 - (window.innerHeight - 40)) {
+ testPassed("Scrolled to " + window.scrollY);
+ clearTimeout(failTimeoutId);
+ finishJSTest();
+ }
+ })
+ window.addEventListener("load", test);
+ </script>
+ <script src="../resources/js-test-post.js"></script>
+ </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar-expected.txt b/LayoutTests/scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar-expected.txt
new file mode 100644
index 0000000..6300f25
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar-expected.txt
@@ -0,0 +1,10 @@
+Test scrolling with page granularity by using the space bar excludes the height of fixed element covering the full page width. In this case, the cumulative size of both bars is larger than the threshold, and only 120px are removed from the 240px fixed height.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Scrolled to 480
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar.html b/LayoutTests/scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar.html
new file mode 100644
index 0000000..f06af6b
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar.html
@@ -0,0 +1,84 @@
+<html>
+ <head>
+ <style>
+ body {
+ height: 10000px;
+ }
+ #top-bar {
+ position: fixed;
+ width: 100%;
+ height: 120px;
+ background-color: green;
+ top: 0;
+ left: 0;
+ }
+ #bottom-bar {
+ position: fixed;
+ width: 100%;
+ height: 120px;
+ background-color: green;
+ bottom: 0;
+ left: 0;
+ }
+ /* This fixed element does not cover the entire width and should be ignored. */
+ #ignored-left {
+ position: fixed;
+ width: 20px;
+ height: 5000px;
+ background-color: red;
+ left: 0;
+ top: 0;
+ }
+ /* This fixed element does not cover an edge and should be ignored. */
+ #ignored-center {
+ position: fixed;
+ width: 100%;
+ height: 150px;
+ background-color: red;
+ left: 0;
+ top: 50%;
+ }
+ </style>
+ <script src="../resources/js-test-pre.js"></script>
+ </head>
+ <body>
+ <div id="top-bar"></div>
+ <div id="bottom-bar"></div>
+ <div id="ignored-left"></div>
+ <div id="ignored-center"></div>
+ <script>
+ description("Test scrolling with page granularity by using the space bar excludes the height of fixed element covering the full page width. In this case, the cumulative size of both bars is larger than the threshold, and only 120px are removed from the 240px fixed height.");
+
+ jsTestIsAsync = true;
+
+ var failTimeoutId;
+ function test() {
+ if (window.eventSender) {
+ // Force the first layout to avoid the suppressed scrollbar cases.
+ scratch = document.documentElement.offsetWidth;
+
+ // Avoid special cases for being "onload".
+ setTimeout(function() {
+ eventSender.keyDown(' ');
+ } , 0);
+
+ failTimeoutId = setTimeout(function() {
+ testFailed("The scrollview failed to scroll in response to the event.");
+ debug("window.scrollY = " + window.scrollY + " excepted value around " + (window.innerHeight - 120));
+ finishJSTest();
+ }, 1000);
+ }
+ }
+
+ window.addEventListener("scroll", function() {
+ if (window.scrollY == window.innerHeight - 120) {
+ testPassed("Scrolled to " + window.scrollY);
+ clearTimeout(failTimeoutId);
+ finishJSTest();
+ }
+ })
+ window.addEventListener("load", test);
+ </script>
+ <script src="../resources/js-test-post.js"></script>
+ </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar-expected.txt b/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar-expected.txt
new file mode 100644
index 0000000..794ddd2
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar-expected.txt
@@ -0,0 +1,10 @@
+Test scrolling with page granularity by using the space bar excludes the height of fixed element covering the full page width.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Scrolled to 480
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar.html b/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar.html
new file mode 100644
index 0000000..12b911a
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar.html
@@ -0,0 +1,75 @@
+<html>
+ <head>
+ <style>
+ body {
+ height: 10000px;
+ }
+ #top-bar {
+ position: fixed;
+ width: 100%;
+ height: 120px;
+ background-color: green;
+ top: 0;
+ left: 0;
+ }
+ /* This fixed element does not cover the entire width and should be ignored. */
+ #ignored-left {
+ position: fixed;
+ width: 20px;
+ height: 5000px;
+ background-color: red;
+ left: 0;
+ top: 0;
+ }
+ /* This fixed element does not cover an edge and should be ignored. */
+ #ignored-center {
+ position: fixed;
+ width: 100%;
+ height: 150px;
+ background-color: red;
+ left: 0;
+ top: 50%;
+ }
+ </style>
+ <script src="../resources/js-test-pre.js"></script>
+ </head>
+ <body>
+ <div id="top-bar"></div>
+ <div id="ignored-left"></div>
+ <div id="ignored-center"></div>
+ <script>
+ description("Test scrolling with page granularity by using the space bar excludes the height of fixed element covering the full page width.");
+
+ jsTestIsAsync = true;
+
+ var failTimeoutId;
+ function test() {
+ if (window.eventSender) {
+ // Force the first layout to avoid the suppressed scrollbar cases.
+ scratch = document.documentElement.offsetWidth;
+
+ // Avoid special cases for being "onload".
+ setTimeout(function() {
+ eventSender.keyDown(' ');
+ } , 0);
+
+ failTimeoutId = setTimeout(function() {
+ testFailed("The scrollview failed to scroll in response to the event.");
+ debug("window.scrollY = " + window.scrollY + " excepted value around " + (window.innerHeight - 120));
+ finishJSTest();
+ }, 1000);
+ }
+ }
+
+ window.addEventListener("scroll", function() {
+ if (window.scrollY == window.innerHeight - 120) {
+ testPassed("Scrolled to " + window.scrollY);
+ clearTimeout(failTimeoutId);
+ finishJSTest();
+ }
+ })
+ window.addEventListener("load", test);
+ </script>
+ <script src="../resources/js-test-post.js"></script>
+ </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar-expected.txt b/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar-expected.txt
new file mode 100644
index 0000000..da2862c
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar-expected.txt
@@ -0,0 +1,10 @@
+Test scrolling with page granularity by using the space bar excludes the height of fixed element covering the full page width. In this case, the top fixed element starts above the page, but the overlap should still be accounted for.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Scrolled to 510
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar.html b/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar.html
new file mode 100644
index 0000000..6aa1796
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar.html
@@ -0,0 +1,76 @@
+<html>
+ <head>
+ <style>
+ body {
+ height: 10000px;
+ }
+ #top-bar {
+ position: fixed;
+ width: 100%;
+ height: 200px;
+ background-color: green;
+ top: -150px;
+ left: 0;
+ }
+ /* This fixed element does not cover the entire width and should be ignored. */
+ #ignored-left {
+ position: fixed;
+ width: 20px;
+ height: 5000px;
+ background-color: red;
+ left: 0;
+ top: 0;
+ }
+ /* This fixed element does not cover an edge and should be ignored. */
+ #ignored-center {
+ position: fixed;
+ width: 100%;
+ height: 150px;
+ background-color: red;
+ left: 0;
+ top: 50%;
+ }
+ </style>
+ <script src="../resources/js-test-pre.js"></script>
+ </head>
+ <body>
+ <div id="top-bar"></div>
+ <div id="ignored-left"></div>
+ <div id="ignored-center"></div>
+ <script>
+ description("Test scrolling with page granularity by using the space bar excludes the height of fixed element covering the full page width. In this case, the top fixed element starts above the page, but the overlap should still be accounted for.");
+
+ jsTestIsAsync = true;
+
+ var maxOverlapBetweenPages = 40;
+ var failTimeoutId;
+ function test() {
+ if (window.eventSender) {
+ // Force the first layout to avoid the suppressed scrollbar cases.
+ scratch = document.documentElement.offsetWidth;
+
+ // Avoid special cases for being "onload".
+ setTimeout(function() {
+ eventSender.keyDown(' ');
+ } , 0);
+
+ failTimeoutId = setTimeout(function() {
+ testFailed("The scrollview failed to scroll in response to the event.");
+ debug("window.scrollY = " + window.scrollY + " excepted value around " + (window.innerHeight - 50 - maxOverlapBetweenPages));
+ finishJSTest();
+ }, 1000);
+ }
+ }
+
+ window.addEventListener("scroll", function() {
+ if (window.scrollY == window.innerHeight - 50 - maxOverlapBetweenPages) {
+ testPassed("Scrolled to " + window.scrollY);
+ clearTimeout(failTimeoutId);
+ finishJSTest();
+ }
+ })
+ window.addEventListener("load", test);
+ </script>
+ <script src="../resources/js-test-post.js"></script>
+ </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar-expected.txt b/LayoutTests/scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar-expected.txt
new file mode 100644
index 0000000..325948ed
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar-expected.txt
@@ -0,0 +1,10 @@
+Test scrolling with page granularity by using the space bar. The fixed elements should be ignored because they are hidden.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Scrolled to 560
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar.html b/LayoutTests/scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar.html
new file mode 100644
index 0000000..17423d0
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar.html
@@ -0,0 +1,66 @@
+<html>
+ <head>
+ <style>
+ body {
+ height: 10000px;
+ }
+ #top-bar {
+ position: fixed;
+ width: 100%;
+ height: 200px;
+ background-color: green;
+ top: 0;
+ left: 0;
+ visibility: hidden;
+ }
+ #bottom-bar {
+ position: fixed;
+ width: 100%;
+ height: 200px;
+ background-color: green;
+ bottom: 0;
+ left: 0;
+ visibility: hidden;
+ }
+ </style>
+ <script src="../resources/js-test-pre.js"></script>
+ </head>
+ <body>
+ <div id="top-bar"></div>
+ <div id="bottom-bar"></div>
+ <script>
+ description("Test scrolling with page granularity by using the space bar. The fixed elements should be ignored because they are hidden.");
+
+ jsTestIsAsync = true;
+
+ var failTimeoutId;
+ function test() {
+ if (window.eventSender) {
+ // Force the first layout to avoid the suppressed scrollbar cases.
+ scratch = document.documentElement.offsetWidth;
+
+ // Avoid special cases for being "onload".
+ setTimeout(function() {
+ eventSender.keyDown(' ');
+ } , 0);
+
+ failTimeoutId = setTimeout(function() {
+ testFailed("The scrollview failed to scroll in response to the event.");
+ debug("window.scrollY = " + window.scrollY + " excepted value around " + (window.innerHeight - 40));
+ finishJSTest();
+ }, 1000);
+ }
+ }
+
+ window.addEventListener("scroll", function() {
+ if (window.scrollY == window.innerHeight - 40) {
+ testPassed("Scrolled to " + window.scrollY);
+ clearTimeout(failTimeoutId);
+ finishJSTest();
+ }
+ })
+ window.addEventListener("load", test);
+ </script>
+ <script src="../resources/js-test-post.js"></script>
+ </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar-expected.txt b/LayoutTests/scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar-expected.txt
new file mode 100644
index 0000000..db41c28
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar-expected.txt
@@ -0,0 +1,10 @@
+Test scrolling with page granularity by using the space bar. The fixed elements should be ignored because they are transparent.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Scrolled to 560
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar.html b/LayoutTests/scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar.html
new file mode 100644
index 0000000..8371639
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar.html
@@ -0,0 +1,66 @@
+<html>
+ <head>
+ <style>
+ body {
+ height: 10000px;
+ }
+ #top-bar {
+ position: fixed;
+ width: 100%;
+ height: 200px;
+ background-color: green;
+ top: 0;
+ left: 0;
+ opacity: 0;
+ }
+ #bottom-bar {
+ position: fixed;
+ width: 100%;
+ height: 200px;
+ background-color: green;
+ bottom: 0;
+ left: 0;
+ opacity: 0;
+ }
+ </style>
+ <script src="../resources/js-test-pre.js"></script>
+ </head>
+ <body>
+ <div id="top-bar"></div>
+ <div id="bottom-bar"></div>
+ <script>
+ description("Test scrolling with page granularity by using the space bar. The fixed elements should be ignored because they are transparent.");
+
+ jsTestIsAsync = true;
+
+ var failTimeoutId;
+ function test() {
+ if (window.eventSender) {
+ // Force the first layout to avoid the suppressed scrollbar cases.
+ scratch = document.documentElement.offsetWidth;
+
+ // Avoid special cases for being "onload".
+ setTimeout(function() {
+ eventSender.keyDown(' ');
+ } , 0);
+
+ failTimeoutId = setTimeout(function() {
+ testFailed("The scrollview failed to scroll in response to the event.");
+ debug("window.scrollY = " + window.scrollY + " excepted value around " + (window.innerHeight - 40));
+ finishJSTest();
+ }, 1000);
+ }
+ }
+
+ window.addEventListener("scroll", function() {
+ if (window.scrollY == window.innerHeight - 40) {
+ testPassed("Scrolled to " + window.scrollY);
+ clearTimeout(failTimeoutId);
+ finishJSTest();
+ }
+ })
+ window.addEventListener("load", test);
+ </script>
+ <script src="../resources/js-test-post.js"></script>
+ </body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/scrollbars/scrolling-by-page-on-keyboard-spacebar-expected.txt b/LayoutTests/scrollbars/scrolling-by-page-on-keyboard-spacebar-expected.txt
new file mode 100644
index 0000000..0706b6f
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-on-keyboard-spacebar-expected.txt
@@ -0,0 +1,10 @@
+Test scrolling with page granularity by using the space bar.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Scrolled to 560
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/scrollbars/scrolling-by-page-on-keyboard-spacebar.html b/LayoutTests/scrollbars/scrolling-by-page-on-keyboard-spacebar.html
new file mode 100644
index 0000000..940d947
--- /dev/null
+++ b/LayoutTests/scrollbars/scrolling-by-page-on-keyboard-spacebar.html
@@ -0,0 +1,46 @@
+<html>
+ <head>
+ <style>
+ body {
+ height: 10000px;
+ }
+ </style>
+ <script src="../resources/js-test-pre.js"></script>
+ </head>
+ <body>
+ <script>
+ description("Test scrolling with page granularity by using the space bar.");
+
+ jsTestIsAsync = true;
+
+ var failTimeoutId;
+ function test() {
+ if (window.eventSender) {
+ // Force the first layout to avoid the suppressed scrollbar cases.
+ scratch = document.documentElement.offsetWidth;
+
+ // Avoid special cases for being "onload".
+ setTimeout(function() {
+ eventSender.keyDown(' ');
+ } , 0);
+
+ failTimeoutId = setTimeout(function() {
+ testFailed("The scrollview failed to scroll in response to the event.");
+ debug("window.scrollY = " + window.scrollY + " excepted value around " + (window.innerHeight - 40));
+ finishJSTest();
+ }, 1000);
+ }
+ }
+
+ window.addEventListener("scroll", function() {
+ if (window.scrollY == window.innerHeight - 40) {
+ testPassed("Scrolled to " + window.scrollY);
+ clearTimeout(failTimeoutId);
+ finishJSTest();
+ }
+ })
+ window.addEventListener("load", test);
+ </script>
+ <script src="../resources/js-test-post.js"></script>
+ </body>
+</html>
\ No newline at end of file
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 61abb44..ef28b2d 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2014-08-28 Benjamin Poulain <bpoulain@apple.com>
+
+ Scrolling with spacebar on a page with fixed header breaks reading flow
+ https://bugs.webkit.org/show_bug.cgi?id=135506
+
+ Reviewed by Simon Fraser.
+
+ When scrolling by page, find the height of any bar that is obscuring the top or bottom of the page,
+ and substract that height from the step to scroll.
+
+ Tests: scrollbars/scrolling-backward-by-page-accounting-bottom-fixed-elements-on-keyboard-spacebar.html
+ scrollbars/scrolling-backward-by-page-on-keyboard-spacebar.html
+ scrollbars/scrolling-by-page-accounting-oversized-fixed-elements-on-keyboard-spacebar.html
+ scrollbars/scrolling-by-page-accounting-top-fixed-elements-on-keyboard-spacebar.html
+ scrollbars/scrolling-by-page-accounting-top-fixed-elements-with-negative-top-on-keyboard-spacebar.html
+ scrollbars/scrolling-by-page-ignoring-hidden-fixed-elements-on-keyboard-spacebar.html
+ scrollbars/scrolling-by-page-ignoring-transparent-fixed-elements-on-keyboard-spacebar.html
+ scrollbars/scrolling-by-page-on-keyboard-spacebar.html
+
+ * WebCore.exp.in:
+ * page/FrameView.cpp:
+ (WebCore::FrameView::adjustScrollStepForFixedContent):
+ * page/FrameView.h:
+ * platform/ScrollableArea.cpp:
+ (WebCore::ScrollableArea::adjustScrollStepForFixedContent):
+ (WebCore::ScrollableArea::scroll):
+ * platform/ScrollableArea.h:
+
2014-08-28 Zalan Bujtas <zalan@apple.com>
Subpixel layout: Remove unused pixel snapping functions.
diff --git a/Source/WebCore/WebCore.exp.in b/Source/WebCore/WebCore.exp.in
index 3cc64eb..42a465d 100644
--- a/Source/WebCore/WebCore.exp.in
+++ b/Source/WebCore/WebCore.exp.in
@@ -430,6 +430,7 @@
__ZN7WebCore14ScrollableArea27notifyScrollPositionChangedERKNS_8IntPointE
__ZN7WebCore14ScrollableArea28setScrollOffsetFromInternalsERKNS_8IntPointE
__ZN7WebCore14ScrollableArea30scrollToOffsetWithoutAnimationERKNS_10FloatPointE
+__ZN7WebCore14ScrollableArea31adjustScrollStepForFixedContentEfNS_20ScrollbarOrientationENS_17ScrollGranularityE
__ZN7WebCore14ScrollableArea34constrainScrollPositionForOverhangERKNS_10LayoutRectERKNS_10LayoutSizeERKNS_11LayoutPointES9_ii
__ZN7WebCore14ScrollableArea6scrollENS_15ScrollDirectionENS_17ScrollGranularityEf
__ZN7WebCore14ScrollableAreaC2Ev
diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
index 339233d..4ae1348 100644
--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -3219,6 +3219,45 @@
frame().loader().client().didChangeScrollOffset();
}
+float FrameView::adjustScrollStepForFixedContent(float step, ScrollbarOrientation orientation, ScrollGranularity granularity)
+{
+ if (granularity != ScrollByPage || orientation == HorizontalScrollbar)
+ return step;
+
+ TrackedRendererListHashSet* positionedObjects = nullptr;
+ if (RenderView* root = m_frame->contentRenderer()) {
+ if (!root->hasPositionedObjects())
+ return step;
+ positionedObjects = root->positionedObjects();
+ }
+
+ FloatRect unobscuredContentRect = this->unobscuredContentRect();
+ float topObscuredArea = 0;
+ float bottomObscuredArea = 0;
+ for (const auto& positionedObject : *positionedObjects) {
+ const RenderStyle& style = positionedObject->style();
+ if (style.position() != FixedPosition || style.visibility() == HIDDEN || !style.opacity())
+ continue;
+
+ FloatQuad contentQuad = positionedObject->absoluteContentQuad();
+ if (!contentQuad.isRectilinear())
+ continue;
+
+ FloatRect contentBoundingBox = contentQuad.boundingBox();
+ FloatRect fixedRectInView = intersection(unobscuredContentRect, contentBoundingBox);
+
+ if (fixedRectInView.width() < unobscuredContentRect.width())
+ continue;
+
+ if (fixedRectInView.y() == unobscuredContentRect.y())
+ topObscuredArea = std::max(topObscuredArea, fixedRectInView.height());
+ else if (fixedRectInView.maxY() == unobscuredContentRect.maxY())
+ bottomObscuredArea = std::max(bottomObscuredArea, fixedRectInView.height());
+ }
+
+ return Scrollbar::pageStep(unobscuredContentRect.height(), unobscuredContentRect.height() - topObscuredArea - bottomObscuredArea);
+}
+
void FrameView::invalidateScrollbarRect(Scrollbar* scrollbar, const IntRect& rect)
{
// Add in our offset within the FrameView.
diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
index ac9198d..4488085 100644
--- a/Source/WebCore/page/FrameView.h
+++ b/Source/WebCore/page/FrameView.h
@@ -509,6 +509,8 @@
virtual void updateSnapOffsets() override;
#endif
+ virtual float adjustScrollStepForFixedContent(float step, ScrollbarOrientation, ScrollGranularity) override;
+
protected:
virtual bool scrollContentsFastPath(const IntSize& scrollDelta, const IntRect& rectToScroll, const IntRect& clipRect) override;
virtual void scrollContentsSlowPath(const IntRect& updateRect) override;
diff --git a/Source/WebCore/platform/ScrollableArea.cpp b/Source/WebCore/platform/ScrollableArea.cpp
index de27df1..4421069 100644
--- a/Source/WebCore/platform/ScrollableArea.cpp
+++ b/Source/WebCore/platform/ScrollableArea.cpp
@@ -87,6 +87,11 @@
}
}
+float ScrollableArea::adjustScrollStepForFixedContent(float step, ScrollbarOrientation, ScrollGranularity)
+{
+ return step;
+}
+
bool ScrollableArea::scroll(ScrollDirection direction, ScrollGranularity granularity, float multiplier)
{
ScrollbarOrientation orientation;
@@ -122,6 +127,7 @@
if (direction == ScrollUp || direction == ScrollLeft)
multiplier = -multiplier;
+ step = adjustScrollStepForFixedContent(step, orientation, granularity);
return scrollAnimator()->scroll(orientation, granularity, step, multiplier);
}
diff --git a/Source/WebCore/platform/ScrollableArea.h b/Source/WebCore/platform/ScrollableArea.h
index 907b012..938d622 100644
--- a/Source/WebCore/platform/ScrollableArea.h
+++ b/Source/WebCore/platform/ScrollableArea.h
@@ -261,6 +261,7 @@
void setScrollOrigin(const IntPoint&);
void resetScrollOriginChanged() { m_scrollOriginChanged = false; }
+ virtual float adjustScrollStepForFixedContent(float step, ScrollbarOrientation, ScrollGranularity);
virtual void invalidateScrollbarRect(Scrollbar*, const IntRect&) = 0;
virtual void invalidateScrollCornerRect(const IntRect&) = 0;
diff --git a/Source/WebCore/platform/Scrollbar.h b/Source/WebCore/platform/Scrollbar.h
index 3b76736..44e23f7 100644
--- a/Source/WebCore/platform/Scrollbar.h
+++ b/Source/WebCore/platform/Scrollbar.h
@@ -97,9 +97,10 @@
void offsetDidChange();
static int pixelsPerLineStep() { return 40; }
- static float minFractionToStepWhenPaging() { return 0.875f; }
+ static float minFractionToStepWhenPaging() { return 0.8; }
WEBCORE_EXPORT static int maxOverlapBetweenPages();
- static int pageStep(int widthOrHeight) { return std::max(std::max<int>(lroundf(widthOrHeight * Scrollbar::minFractionToStepWhenPaging()), lroundf(widthOrHeight - Scrollbar::maxOverlapBetweenPages())), 1); }
+ static int pageStep(int viewWidthOrHeight, int contentWidthOrHeight) { return std::max(std::max<int>(lroundf(viewWidthOrHeight * Scrollbar::minFractionToStepWhenPaging()), lroundf(contentWidthOrHeight - Scrollbar::maxOverlapBetweenPages())), 1); }
+ static int pageStep(int viewWidthOrHeight) { return pageStep(viewWidthOrHeight, viewWidthOrHeight); }
static float pageStepDelta(int widthOrHeight) { return std::max(std::max(static_cast<float>(widthOrHeight) * Scrollbar::minFractionToStepWhenPaging(), static_cast<float>(widthOrHeight) - Scrollbar::maxOverlapBetweenPages()), 1.0f); }
void disconnectFromScrollableArea() { m_scrollableArea = 0; }
diff --git a/Source/WebCore/platform/mock/ScrollbarThemeMock.h b/Source/WebCore/platform/mock/ScrollbarThemeMock.h
index 6333841..a500444 100644
--- a/Source/WebCore/platform/mock/ScrollbarThemeMock.h
+++ b/Source/WebCore/platform/mock/ScrollbarThemeMock.h
@@ -45,6 +45,7 @@
virtual void paintTrackBackground(GraphicsContext*, ScrollbarThemeClient*, const IntRect&);
virtual void paintThumb(GraphicsContext*, ScrollbarThemeClient*, const IntRect&);
+ virtual int maxOverlapBetweenPages() { return 40; }
private:
virtual bool isMockTheme() const { return true; }
