Skip to content

GitLab

T
tangled
Commit c0ae9747 authored by David Bokan's avatar David Bokan Committed by Commit Bot
Browse files
Options

Make root scroller valid in both URL bar states 

Some pages resize their main scroller to the viewport height as the
URL bar is shown and hidden. In those cases, we should allow a valid
root scroller at both heights.  Otherwise, when the URL bar is hidden
and the scroller resizes, it'll be demoted and the user will have to
scroll all the way to the top to show the URL bar again. Currently,
the root scroller must exactly match the initial containing block
which does not resize in response to the URL bar.

An obvious question would be why allow either height, rather than
calculating the current viewport height and using that. In most cases
a scroller we want to promote will not resize its height and we still
want to keep those scrollers promoted.

Bug: 798719
Change-Id: I11684ee5115a02f1f65868399f58074df43e764f
Reviewed-on: https://chromium-review.googlesource.com/1036287
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555555}
parent 40c47ac8
Hide whitespace changes
Inline Side-by-side
Showing with 102 additions and 1 deletion
third_party/blink/renderer/core/frame/local_frame_view.cc
View file @ c0ae9747
......@@ -1413,6 +1413,9 @@ FloatSize LocalFrameView::ViewportSizeForViewportUnits() const {
// use the viewport with browser controls hidden for vh (to match Safari).
int viewport_width = frame_->GetPage()->GetVisualViewport().Size().Width();
if (frame_->IsMainFrame() && layout_size.Width() && viewport_width) {
// TODO(bokan/eirage): BrowserControl height may need to account for the
// zoom factor when use-zoom-for-dsf is enabled on Android. Confirm this
// works correctly when that's turned on. https://crbug.com/737777.
float page_scale_at_layout_width = viewport_width / layout_size.Width();
layout_size.Expand(
0, browser_controls.TotalHeight() / page_scale_at_layout_width);
......
third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc
View file @ c0ae9747
......@@ -7,6 +7,7 @@
#include "third_party/blink/renderer/core/css/style_change_reason.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/element.h"
#include "third_party/blink/renderer/core/frame/browser_controls.h"
#include "third_party/blink/renderer/core/frame/local_frame_view.h"
#include "third_party/blink/renderer/core/fullscreen/document_fullscreen.h"
#include "third_party/blink/renderer/core/html/html_frame_owner_element.h"
......@@ -48,7 +49,15 @@ bool FillsViewport(const Element& element, bool check_location) {
LayoutRect bounding_box(quad.BoundingBox());
if (bounding_box.Size() != top_document.GetLayoutView()->GetLayoutSize())
LayoutSize icb_size =
LayoutSize(top_document.GetLayoutView()->GetLayoutSize());
float zoom = top_document.GetFrame()->PageZoomFactor();
LayoutSize controls_hidden_size = LayoutSize(
top_document.View()->ViewportSizeForViewportUnits().ScaledBy(zoom));
if (bounding_box.Size() != icb_size &&
bounding_box.Size() != controls_hidden_size)
return false;
if (!check_location)
......
third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
View file @ c0ae9747
......@@ -1266,6 +1266,11 @@ class RootScrollerSimTest : public testing::WithParamInterface<bool>,
: ScopedRootLayerScrollingForTest(GetParam()),
implicit_root_scroller_for_test_(false) {}
void SetUp() override {
SimTest::SetUp();
WebView().GetPage()->GetSettings().SetViewportEnabled(true);
}
private:
ScopedImplicitRootScrollerForTest implicit_root_scroller_for_test_;
};
......@@ -1762,6 +1767,90 @@ TEST_F(ImplicitRootScrollerSimTest,
<< "Once loaded, the iframe should be promoted.";
}
// Test that a root scroller is considered to fill the viewport at both the URL
// bar shown and URL bar hidden height.
TEST_F(ImplicitRootScrollerSimTest,
RootScrollerFillsViewportAtBothURLBarStates) {
WebView().ResizeWithBrowserControls(IntSize(800, 600), 50, 0, true);
SimRequest main_request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
main_request.Complete(R"HTML(
<!DOCTYPE html>
<style>
::-webkit-scrollbar {
width: 0px;
height: 0px;
}
body, html {
width: 100%;
height: 100%;
margin: 0px;
}
#container {
width: 100%;
height: 100%;
overflow: auto;
border: 0;
}
</style>
<div id="container">
<div style="height: 2000px;"></div>
</div>
<script>
onresize = () => {
document.getElementById("container").style.height =
window.innerHeight + "px";
};
</script>
)HTML");
Element* container = GetDocument().getElementById("container");
GetDocument().setRootScroller(container);
Compositor().BeginFrame();
ASSERT_EQ(container,
GetDocument().GetRootScrollerController().EffectiveRootScroller());
// Simulate hiding the top controls. The root scroller should remain valid at
// the new height.
WebView().GetPage()->GetBrowserControls().SetShownRatio(0);
WebView().ResizeWithBrowserControls(IntSize(800, 650), 50, 0, false);
Compositor().BeginFrame();
EXPECT_EQ(container,
GetDocument().GetRootScrollerController().EffectiveRootScroller());
// Simulate showing the top controls. The root scroller should remain valid.
WebView().GetPage()->GetBrowserControls().SetShownRatio(1);
WebView().ResizeWithBrowserControls(IntSize(800, 600), 50, 0, true);
Compositor().BeginFrame();
EXPECT_EQ(container,
GetDocument().GetRootScrollerController().EffectiveRootScroller());
// Set the height explicitly to a new value in-between. The root scroller
// should be demoted.
container->style()->setProperty(&GetDocument(), "height", "601px", String(),
ASSERT_NO_EXCEPTION);
Compositor().BeginFrame();
EXPECT_EQ(GetDocument(),
GetDocument().GetRootScrollerController().EffectiveRootScroller());
// Reset back to valid and hide the top controls. Zoom to 2x. Ensure we're
// still considered valid.
container->style()->setProperty(&GetDocument(), "height", "", String(),
ASSERT_NO_EXCEPTION);
Compositor().BeginFrame();
EXPECT_EQ(container,
GetDocument().GetRootScrollerController().EffectiveRootScroller());
EXPECT_EQ(ToLayoutBox(container->GetLayoutObject())->Size().Height(), 600);
WebView().SetZoomLevel(WebView::ZoomFactorToZoomLevel(2.0));
WebView().GetPage()->GetBrowserControls().SetShownRatio(0);
WebView().ResizeWithBrowserControls(IntSize(800, 650), 50, 0, false);
Compositor().BeginFrame();
EXPECT_EQ(container->clientHeight(), 325);
EXPECT_EQ(container,
GetDocument().GetRootScrollerController().EffectiveRootScroller());
}
// Tests that we don't explode when a layout occurs and the effective
// rootScroller no longer has a ContentFrame(). We setup the frame tree such
// that the first iframe is the effective root scroller. The second iframe has
......
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