Commit 8c76d127 authored by David Bokan's avatar David Bokan Committed by Commit Bot

Correct OverflowClipRect() for root scroller

When a layout object is the effective root scroller, the
OverflowClipRect uses the ViewRect rather than computing the padding box
as usual. This is because the padding box won't expand when the URL bar
is hidden.

Unfortunately in the root scroller case we forget to set the passed in
location. This patch adds that so the rect is in the correct coordinate
space. Additionally, this all works only if the box set as the effective
root scroller has no border since the OverflowClipRect uses the padding
box. I've updated the root scroller to ensure the padding box matches
the viewport rect.

Ideally we'd just return View()->OverflowClipRect - which does remember
to set the location - in LayoutBox but this could lead to an infinite
recursion as that calls LayoutBox::OverflowClipRect if the ViewRect is
empty. Thus, we call ViewRect directly.

Bug: 816161
Change-Id: I2f622ff0c25e58373955e1d0e22d54e254e134ff
Reviewed-on: https://chromium-review.googlesource.com/1026517Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553678}
parent c5501e8e
<!DOCTYPE html>
<style>
::-webkit-scrollbar {
width: 0px;
height: 0px;
}
body, html {
width: 100%;
height: 100%;
}
body {
margin: 0px;
}
#container {
width: 100%;
height: 100%;
overflow: auto;
}
#sticky {
width: 100%;
height: 50px;
background-color: lightgreen;
position: sticky;
top: 50px;
}
#space {
width: 100%;
height: 400%;
}
#before {
height: 300px;
}
</style>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<div id="container">
<div id="before"></div>
<div id="sticky">
</div>
<div id="space"></div>
</div>
<script>
var container = document.querySelector('#container');
var sticky = document.querySelector('#sticky');
test(function() {
assert_false(typeof document.rootScroller === 'undefined');
document.rootScroller = container;
assert_equals(sticky.getBoundingClientRect().top, 300,
"rect top should be regular in-flow location when unscrolled");
container.scrollTo(0, 100);
assert_equals(sticky.getBoundingClientRect().top, 200,
"rect top should account for scroll before becoming fixed");
container.scrollTo(0, 500);
assert_equals(sticky.getBoundingClientRect().top, 50,
"rect top should be sticky location");
}, 'getBoundingClientRect correct on position: sticky in rootScroller.');
</script>
<!DOCTYPE html>
<meta name="viewport" content="width=device-width, user-scalable=no" />
<style>
::-webkit-scrollbar {
width: 0px;
height: 0px;
}
html, body {
height: 100%;
width: 100%;
......
......@@ -29,6 +29,10 @@
});
</script>
<style>
::-webkit-scrollbar {
width: 0px;
height: 0px;
}
html, body {
height: 100%;
width: 100%;
......@@ -44,6 +48,10 @@
<iframe id="iframe" srcdoc="
<style>
::-webkit-scrollbar {
width: 0px;
height: 0px;
}
html,body {
height: 100%;
width: 100%;
......
......@@ -1949,16 +1949,24 @@ PaintInvalidationReason LayoutBox::InvalidatePaint(
LayoutRect LayoutBox::OverflowClipRect(
const LayoutPoint& location,
OverlayScrollbarClipBehavior overlay_scrollbar_clip_behavior) const {
if (RootScrollerUtil::IsEffective(*this))
return View()->ViewRect();
// FIXME: When overflow-clip (CSS3) is implemented, we'll obtain the property
// here.
LayoutRect clip_rect = BorderBoxRect();
clip_rect.SetLocation(location + clip_rect.Location() +
LayoutSize(BorderLeft(), BorderTop()));
clip_rect.SetSize(clip_rect.Size() -
LayoutSize(BorderWidth(), BorderHeight()));
LayoutRect clip_rect;
if (RootScrollerUtil::IsEffective(*this)) {
// If this box is the effective root scroller, use the viewport clipping
// rect since it will account for the URL bar correctly which the border
// box does not. We can do this because the effective root scroller is
// restricted such that it exactly fills the viewport. See
// RootScrollerController::IsValidRootScroller()
clip_rect = LayoutRect(location, View()->ViewRect().Size());
} else {
// FIXME: When overflow-clip (CSS3) is implemented, we'll obtain the
// property here.
clip_rect = BorderBoxRect();
clip_rect.SetLocation(location + clip_rect.Location() +
LayoutSize(BorderLeft(), BorderTop()));
clip_rect.SetSize(clip_rect.Size() -
LayoutSize(BorderWidth(), BorderHeight()));
}
if (HasOverflowClip())
ExcludeScrollbars(clip_rect, overlay_scrollbar_clip_behavior);
......
......@@ -38,14 +38,13 @@ bool FillsViewport(const Element& element) {
// TODO(bokan): Broken for OOPIF. crbug.com/642378.
Document& top_document = element.GetDocument().TopDocument();
Vector<FloatQuad> quads;
layout_object->AbsoluteQuads(quads);
DCHECK_EQ(quads.size(), 1u);
FloatQuad quad = layout_object->LocalToAbsoluteQuad(
FloatRect(ToLayoutBox(layout_object)->PaddingBoxRect()));
if (!quads[0].IsRectilinear())
if (!quad.IsRectilinear())
return false;
LayoutRect bounding_box(quads[0].BoundingBox());
LayoutRect bounding_box(quad.BoundingBox());
return bounding_box.Location() == LayoutPoint::Zero() &&
bounding_box.Size() == top_document.GetLayoutView()->GetLayoutSize();
......
......@@ -1269,6 +1269,43 @@ class RootScrollerSimTest : public testing::WithParamInterface<bool>,
INSTANTIATE_TEST_CASE_P(All, RootScrollerSimTest, testing::Bool());
// Test that the element is considered to be viewport filling only if its
// padding box fills the viewport. That means it must have no border.
TEST_P(RootScrollerSimTest, UsePaddingBoxForViewportFillingCondition) {
WebView().Resize(WebSize(800, 600));
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
margin: 0;
}
#container {
position: absolute;
width: 100%;
height: 100%;
box-sizing: border-box;
overflow: scroll;
}
</style>
<div id="container"></div>
)HTML");
Compositor().BeginFrame();
Element* container = GetDocument().getElementById("container");
GetDocument().setRootScroller(container);
ASSERT_EQ(container,
GetDocument().GetRootScrollerController().EffectiveRootScroller());
// Setting a border should cause the element to no longer be valid as its
// padding box doesn't fill the viewport exactly.
container->setAttribute(HTMLNames::styleAttr, "border: 1px solid black");
Compositor().BeginFrame();
EXPECT_EQ(&GetDocument(),
GetDocument().GetRootScrollerController().EffectiveRootScroller());
}
// Tests that the root scroller doesn't affect visualViewport pageLeft and
// pageTop.
TEST_P(RootScrollerSimTest, RootScrollerDoesntAffectVisualViewport) {
......
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