Skip to content

Commit

Permalink
Update scroll-animation tests to be independent of scrollbar width
Browse files Browse the repository at this point in the history
This patch addresses sources of test failures observed when implementing
a polyfill implementation of scroll timelines. Different platforms can
use different scrollbar widths, which leads to some changes in
expectations. By hiding the scrollbars, we have consistency across
platforms and avoid fractional scroll offsets.

Previous expectations may be the source of Mac failures.
Deferring possible re-enabling of Mac tests to a followup CL.

Bug: 1246372
Change-Id: I6b7ad072af495b23e4acf0654a210847de8cb78b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3149090
Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/main@{#919758}
  • Loading branch information
Kevin Ellis authored and chromium-wpt-export-bot committed Sep 9, 2021
1 parent eb90781 commit ef3d38e
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 24 deletions.
32 changes: 24 additions & 8 deletions scroll-animations/current-time.html
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,30 @@
// current time.
await waitForNextFrame();

assert_percents_equal(blockScrollTimeline.currentTime, 12.048,
'Scrolled block timeline');
assert_percents_equal(inlineScrollTimeline.currentTime, 18.072,
'Scrolled inline timeline');
assert_percents_equal(horizontalScrollTimeline.currentTime, 18.072,
'Scrolled horizontal timeline');
assert_percents_equal(verticalScrollTimeline.currentTime, 12.048,
'Scrolled vertical timeline');
const inlineScrollRange = scroller.scrollWidth - scroller.clientWidth;
const expectedInlineCurrentTime =
100 * scroller.scrollLeft / inlineScrollRange;

const blockScrollRange = scroller.scrollHeight - scroller.clientHeight;
const expectedBlockCurrentTime =
100 * scroller.scrollTop / blockScrollRange;

assert_percents_approx_equal(blockScrollTimeline.currentTime,
expectedBlockCurrentTime,
blockScrollRange,
'Scrolled block timeline');
assert_percents_approx_equal(inlineScrollTimeline.currentTime,
expectedInlineCurrentTime,
inlineScrollRange,
'Scrolled inline timeline');
assert_percents_approx_equal(horizontalScrollTimeline.currentTime,
expectedInlineCurrentTime,
inlineScrollRange,
'Scrolled horizontal timeline');
assert_percents_approx_equal(verticalScrollTimeline.currentTime,
expectedBlockCurrentTime,
blockScrollRange,
'Scrolled vertical timeline');
}, 'currentTime calculates the correct time based on scroll progress');


Expand Down
13 changes: 9 additions & 4 deletions scroll-animations/element-based-offset.html
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@
await waitForNextFrame();

const animation = createScrollLinkedAnimation(t, timeline);
const scrollRange = end.offsetTop - start.offsetTop;
const scrollRange = (config.orientation == 'vertical')
? end.offsetTop - start.offsetTop
: end.offsetLeft - start.offsetLeft;

// Verify initial start and current times in Idle state.
assert_equals(animation.currentTime, null,
Expand Down Expand Up @@ -119,19 +121,22 @@
}

await waitForNextFrame();
assert_percents_equal(
assert_percents_approx_equal(
animation.timeline.currentTime,
config.expectedCurrentTime,
scrollRange,
"The timeline current time corresponds to the scroll position of " +
"the scroller.");
assert_percents_equal(
assert_percents_approx_equal(
animation.currentTime,
config.expectedCurrentTime,
scrollRange,
"The animation current time corresponds to the scroll position of " +
"the scroller.");
assert_percents_equal(
assert_percents_approx_equal(
animation.effect.getComputedTiming().localTime,
config.expectedCurrentTime,
scrollRange,
"Effect local time corresponds to the scroll position of the " +
"scroller.");
}, description);
Expand Down
33 changes: 25 additions & 8 deletions scroll-animations/progress-based-current-time.tentative.html
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,31 @@
// current time.
await waitForNextFrame();

assert_percents_equal(blockScrollTimeline.currentTime, 12.048,
'Scrolled block timeline');
assert_percents_equal(inlineScrollTimeline.currentTime, 18.072,
'Scrolled inline timeline');
assert_percents_equal(horizontalScrollTimeline.currentTime, 18.072,
'Scrolled horizontal timeline');
assert_percents_equal(verticalScrollTimeline.currentTime, 12.048,
'Scrolled vertical timeline');

const inlineScrollRange = scroller.scrollWidth - scroller.clientWidth;
const expectedInlineCurrentTime =
100 * scroller.scrollLeft / inlineScrollRange;

const blockScrollRange = scroller.scrollHeight - scroller.clientHeight;
const expectedBlockCurrentTime =
100 * scroller.scrollTop / blockScrollRange;

assert_percents_approx_equal(blockScrollTimeline.currentTime,
expectedBlockCurrentTime,
blockScrollRange,
'Scrolled block timeline');
assert_percents_approx_equal(inlineScrollTimeline.currentTime,
expectedInlineCurrentTime,
inlineScrollRange,
'Scrolled inline timeline');
assert_percents_approx_equal(horizontalScrollTimeline.currentTime,
expectedInlineCurrentTime,
inlineScrollRange,
'Scrolled horizontal timeline');
assert_percents_approx_equal(verticalScrollTimeline.currentTime,
expectedBlockCurrentTime,
blockScrollRange,
'Scrolled vertical timeline');
}, 'currentTime calculates the correct time based on scroll progress');


Expand Down
9 changes: 5 additions & 4 deletions scroll-animations/testcommon.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ function setupScrollTimelineTest(
let scroller = document.createElement('div');
scroller.style.width = '100px';
scroller.style.height = '100px';
scroller.style.overflow = 'scroll';
// Hide the scrollbars, but maintain the ability to scroll. This setting
// ensures that variability in scrollbar sizing does not contribute to test
// failures or flakes.
scroller.style.overflow = 'hidden';
for (const [key, value] of scrollerOverrides) {
scroller.style[key] = value;
}
Expand Down Expand Up @@ -122,9 +125,7 @@ function assert_percents_approx_equal(actual, expected, maxScroll,

function assert_percents_equal(actual, expected, description) {
// Rough estimate of the default size of the scrollable area based on
// sizes in setupScrollTimelineTest. This size estimate ignores the height and
// width of the scrollbar, but nonetheless provides a conservative estimate
// that may be used to compute a realistic tolerance.
// sizes in setupScrollTimelineTest.
const defaultScrollRange = 400;
return assert_percents_approx_equal(actual, expected, defaultScrollRange,
description);
Expand Down

0 comments on commit ef3d38e

Please sign in to comment.