Auto merge of #16807 - mrobinson:scroll-clamping, r=emilio

Fix clamping of scroll position in window.scrollBy

For rightward and downward overflow the spec says:

Let x be max(0, min(x, viewport scrolling area width - viewport width)).
Let y be max(0, min(y, viewport scrolling area height - viewport height)).

Previously, those operations were reversed, which created negative
overflow even when the overflow direction was downward. This change
ensures that Servo matches spec behavior.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16807)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-05-11 13:23:39 -05:00 committed by GitHub
commit e196776c24
3 changed files with 38 additions and 2 deletions

View file

@ -1066,8 +1066,8 @@ impl Window {
let content_size = e.upcast::<Node>().bounding_content_box_or_zero(); let content_size = e.upcast::<Node>().bounding_content_box_or_zero();
let content_height = content_size.size.height.to_f64_px(); let content_height = content_size.size.height.to_f64_px();
let content_width = content_size.size.width.to_f64_px(); let content_width = content_size.size.width.to_f64_px();
(xfinite.max(0.0f64).min(content_width - width), (xfinite.min(content_width - width).max(0.0f64),
yfinite.max(0.0f64).min(content_height - height)) yfinite.min(content_height - height).max(0.0f64))
}, },
None => { None => {
(xfinite.max(0.0f64), yfinite.max(0.0f64)) (xfinite.max(0.0f64), yfinite.max(0.0f64))

View file

@ -19783,6 +19783,12 @@
{} {}
] ]
], ],
"mozilla/scrollBy.html": [
[
"/_mozilla/mozilla/scrollBy.html",
{}
]
],
"mozilla/scrollTo.html": [ "mozilla/scrollTo.html": [
[ [
"/_mozilla/mozilla/scrollTo.html", "/_mozilla/mozilla/scrollTo.html",
@ -31354,6 +31360,10 @@
"b247e3a0ba733c1a8b129ce2994b862d8ed3a423", "b247e3a0ba733c1a8b129ce2994b862d8ed3a423",
"testharness" "testharness"
], ],
"mozilla/scrollBy.html": [
"0243d584bc615a73ea1667fc35f5d73b5165d19f",
"testharness"
],
"mozilla/scrollTo.html": [ "mozilla/scrollTo.html": [
"b9917be5fed364dbc46264f641f54f275b5c054a", "b9917be5fed364dbc46264f641f54f275b5c054a",
"testharness" "testharness"

View file

@ -0,0 +1,26 @@
<!doctype html>
<meta charset="utf-8">
<title>Ensure that the window.scrollBy function affects scroll position as expected</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<a id="link" href="http://mozilla.org">This is some link text.</a>
<div style="width: 10000px; height: 10000px; background: pink; z-index: -1;"></div>
<script>
test(function() {
scrollBy(0, 5000);
assert_equals(scrollX, 0);
assert_equals(pageXOffset, 0);
assert_equals(scrollY, 5000);
assert_equals(pageYOffset, 5000);
scrollTo(0, 0);
var link = document.getElementById("link");
var linkRect = link.getBoundingClientRect();
assert_equals(link, document.elementFromPoint(linkRect.left, linkRect.top));
window.scrollBy(0, 1);
assert_equals(link, document.elementFromPoint(linkRect.left, linkRect.top - 1));
});
</script>
</body>