Skip to content

GitLab

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

Use FrameView rect for IntersectionObserver root 

This bug occurs because of Clank's "resize main frame after layout"
behavior. In a nutshell: we want users to be able to zoom out to that
the entire width of the content fits on screen. Because we can't zoom
out further than the layout viewport (i.e. the main FrameView), after
a layout occurs we resize the main FrameView to match the content width.
This leaves us in an interesting situation - the viewport scroller is
actually larger than the initial containing block (i.e. the LayoutView
size). This situation cannot occur on desktop and generally doesn't
occur on well-behaved mobile sites (sites that don't allow zoom out).

The bug was occuring because the intersection observer used the
LayoutView/ICB size for the root element. This is incorrect in the
situation described above. We should really use the ViewRect which is
the scroller bounds.

Bug: 836408
Change-Id: I96029cdb11a6a16353d5b8b628b0182c13926a8d
Reviewed-on: https://chromium-review.googlesource.com/1031118Reviewed-by: default avatarStefan Zager <szager@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554661}
parent a9fc2db2
Hide whitespace changes
Inline Side-by-side
Showing with 129 additions and 8 deletions
third_party/WebKit/LayoutTests/VirtualTestSuites
View file @ 3fba0d48
......@@ -151,6 +151,15 @@
"--force-overlay-fullscreen-video", "--enable-overscroll-notifications",
"--enable-viewport", "--disable-canvas-aa", "--disable-composited-antialiasing"]
},
{
"prefix": "android",
"base": "frame-size",
"args": ["--enable-features=OverlayScrollbar", "--enable-threaded-compositing",
"--enable-fixed-position-compositing", "--enable-prefer-compositing-to-lcd-text",
"--enable-composited-scrolling-for-frames", "--enable-gesture-tap-highlight", "--enable-pinch",
"--force-overlay-fullscreen-video", "--enable-overscroll-notifications",
"--enable-viewport", "--disable-canvas-aa", "--disable-composited-antialiasing"]
},
{
"prefix": "media-gpu-accelerated",
"base": "external/wpt/media-source",
......
third_party/WebKit/LayoutTests/virtual/android/frame-size/README.txt 0 → 100644
View file @ 3fba0d48
Tests related to Android Chrome's "resize after layout" of the main viewport
done to allow zooming out to fit content width.
third_party/WebKit/LayoutTests/virtual/android/frame-size/intersection-observer-root-uses-frame-size.html 0 → 100644
View file @ 3fba0d48
<!DOCTYPE html>
<script>
if (window.internals)
window.internals.settings.setViewportMetaEnabled(true);
</script>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<meta name="viewport" content="width=980">
<style>
html {
width: 100%;
height: 100%;
background-color: white;
}
body {
width: 100%;
height: 100%;
background-color: lightgrey;
margin: 0;
}
#widediv {
border: 5px solid black;
box-sizing: border-box;
width: 200%;
}
#target {
width: 300px;
height: 300px;
position: absolute;
top: 600%;
background-color: red;
margin-bottom: 600%;
}
p {
margin-top: 0;
}
</style>
<p>
This test verifies that intersection observer on the root is correct in the
presence of Android's quirky viewport mode when an element wider than the
initial contianing block is present. The light grey box represents the
initial contianing block. The wide black box ensures the page is zoomed out
further than the ICB.
</p>
<p>
To test manually: Scroll down until you see the box. This test passes if, as
soon as the box intersects the viewport, it turns green.
</p>
<div id="widediv"></div>
<div id="target"></div>
<script>
const box = document.getElementById("target");
const targetTop = box.getBoundingClientRect().top;
const targetBottom = box.getBoundingClientRect().bottom;
const viewportHeight = window.innerHeight;
const startIntersectingScrollTop = targetTop - viewportHeight;
const endIntersectingScrollTop = targetBottom;
function isVisible() {
return window.scrollY >= startIntersectingScrollTop &&
window.scrollY < targetBottom;
}
function scrollToAndWaitFrame(y) {
return () => {
return new Promise((resolve) => {
window.scrollTo(0, y);
requestAnimationFrame(() => {
resolve();
});
});
};
}
addEventListener("load", () => {
if (window.internals)
window.internals.setPageScaleFactor(0.5);
promise_test( t => {
return new Promise((resolve, reject) => {
if (window.testRunner)
assert_equals(visualViewport.scale, 0.5, "Page must start zoomed out");
const callback = function(entries) {
if (window.testRunner) {
t.step(() => {
assert_equals(entries.length, 1, "Observing only one element");
assert_equals(entries[0].isIntersecting, isVisible(),
"Report intersection only if element is on-screen");
});
}
box.style.backgroundColor =
entries[0].isIntersecting ? "green" : "red";
};
const observer = new IntersectionObserver(callback, {});
observer.observe(box);
resolve();
})
.then(scrollToAndWaitFrame(startIntersectingScrollTop - 1))
.then(scrollToAndWaitFrame(startIntersectingScrollTop + 1))
.then(scrollToAndWaitFrame(endIntersectingScrollTop - 1))
.then(scrollToAndWaitFrame(endIntersectingScrollTop + 1));
}, "IntersectionObserver uses correct root box");
});
</script>
third_party/blink/renderer/core/layout/intersection_geometry.cc
View file @ 3fba0d48
......@@ -117,17 +117,17 @@ void IntersectionGeometry::InitializeRootRect() {
!RuntimeEnabledFeatures::RootLayerScrollingEnabled()) {
root_rect_ = LayoutRect(root_->GetFrameView()->VisibleContentRect());
root_->MapToVisualRectInAncestorSpace(nullptr, root_rect_);
} else if (root_->IsLayoutView() && root_->GetDocument().IsInMainFrame() &&
root_->GetDocument()
.GetPage()
->GetSettings()
.GetForceZeroLayoutHeight()) {
// The ForceZeroLayoutHeight quirk setting is used in Android WebView for
} else if (root_->IsLayoutView() && root_->GetDocument().IsInMainFrame()) {
// The main frame is a bit special (even after RLS) as the scrolling
// viewport can differ in size from the LayoutView itself. There's two
// situations this occurs in:
// 1) The ForceZeroLayoutHeight quirk setting is used in Android WebView for
// compatibility and sets the initial-containing-block's (a.k.a.
// LayoutView) height to 0. Thus, we can't use its size for intersection
// testing. Use the FrameView geometry instead.
root_rect_ = LayoutRect(
LayoutPoint(), LayoutSize(root_->GetFrameView()->VisibleContentSize()));
// 2) An element wider than the ICB can cause us to resize the FrameView so
// we can zoom out to fit the entire element width.
root_rect_ = ToLayoutView(root_)->OverflowClipRect(LayoutPoint());
} else if (root_->IsBox() && root_->HasOverflowClip()) {
root_rect_ = LayoutRect(ToLayoutBox(root_)->ContentBoxRect());
} else {
......
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