Commit 8c441107 authored by Fergal Daly's avatar Fergal Daly Committed by Commit Bot

BUG: Scroller was not observing revealed elements.

This bug was introduced https://crrev.com/c/1772850. Set.add does not
take multiple arguments, so most revealed elements were not added to
the observed set.

Bug: 1004102
Change-Id: If57b3125a54a52e1d0fd242c29b059b1cf69b2cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1804096
Commit-Queue: Fergal Daly <fergal@chromium.org>
Reviewed-by: default avatarDomenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696712}
parent e8366027
......@@ -189,9 +189,8 @@ export class VisibilityManager {
// This should include all of the elements to be revealed and
// also 1 element above and below those (if such elements
// exist).
const newObserved = new Set();
const newObserved = new Set(newRevealed);
if (newRevealed.size !== 0) {
newObserved.add(...newRevealed);
const p = newBounds.low.previousElementSibling;
if (p) {
newObserved.add(p);
......
<!DOCTYPE html>
<html class="reftest-wait">
<style>body { overflow:hidden; }</style>
<script type="module">
import * as refTests from './resources/ref-tests.mjs';
refTests.withDiv(refTests.testFullScrollToEndIn2Steps);
</script>
</html>
<!DOCTYPE html>
<html class="reftest-wait">
<meta charset="utf8">
<title>Creates several pages of divs and scrolls in 2 steps.</title>
<style>body { overflow:hidden; }</style>
<link rel="author" title="Fergal Daly" href="mailto:fergal@chromium.org">
<link rel="match" href="full-scroll-to-end-in-2-steps-ref.html">
<script type="module">
import 'std:elements/virtual-scroller';
import * as refTests from './resources/ref-tests.mjs';
refTests.withVirtualScroller(refTests.testFullScrollToEndIn2Steps);
</script>
</html>
......@@ -26,6 +26,24 @@ export function testFullScroll500px(target) {
});
};
/**
* Tests scrolling in 2 steps to the end of the page.
* This reproduces crbug.com/1004102.
*/
export function testFullScrollToEndIn2Steps(target) {
helpers.appendDivs(target, MORE_THAN_SCREENFUL, '10px');
// TODO(fergal): rewrite tests to use await.
// Give the scroller time to settle.
helpers.inNFrames(10, () => {
target.children[1].scrollIntoView(/* alignToTop= */ true);
// Give the scroller time to settle.
helpers.inNFrames(10, () => {
window.scrollBy(0, target.getBoundingClientRect().height);
helpers.stopWaiting();
});
});
};
export function testLargeChild(target) {
// This scrollTo and nextFrame are not necessary for the ref-test
// however it helps when trying to debug this test in a
......
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