Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translate the scroll coordinate to ScrollOrigin #17851

Merged
merged 1 commit into from Oct 10, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

Translate the scroll coordinate to ScrollOrigin

Currently, the value of ScrollLeft / ScrollTop / ScrollTo for a box in
Element is the offset to the origin of ScrollableArea(left-top corner).
This behavior isn't consistent with Document-scroll or the behavior of
other vendors either whose origin is ScrollOrigin. There're compatibility
problems when the box has leftward or upwards scroll overflow direction.
According to the Specification, the scroll x-coordinate of a leftward box
is nonpositive, and the scroll y-coordinate of an upwards box is also
nonpositive. With using the origin of ScrollableArea, the coordinate
is always nonnegative.

In order to fix it, this patch transforms the scroll coordinate of a
box in Element interface to use ScrollOrigin as its origin.

There are a few cases needed to recalculate the scroll coordinate to
meet this change. Since the origin of scroll coordinate transforms
from the ScrollableArea origin to ScrollOrigin(), current_coordinate
is equal to old_coordinate - ScrollOrigin. E.g.
current_scrollLeft = old_scrollLeft - ScrollOrigin().X().

This behavior is guarded by a feature flag.

See intent to ship blink-dev thread:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/7X2CKPGeEa0

Bug: 721759
Change-Id: I0ceed62e6845c6e5cd976e59b36f292d60bb669c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1700001
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Frédéric Wang <fwang@igalia.com>
Commit-Queue: cathie chen <cathiechen@igalia.com>
Cr-Commit-Position: refs/heads/master@{#704470}
  • Loading branch information
cathiechen authored and chromium-wpt-export-bot committed Oct 10, 2019
commit 022536f4d7b97dc04f39c2c894d162d2e58b9efe
@@ -108,11 +108,11 @@
const elements = createTestDOM(true, 'vertical-rl', 'ltr');
const animation = createAndPlayTestAnimation(elements, 'block');
// Move the scroller to the 75% point. Since it is vertical-rl, this is
// equivalent to the 25% point for the ScrollTimeline.
// Move the scroller to the left 25% point, since it is vertical-rl,
// i.e leftwards overflow direction, scrollLeft is -25% point.
const maxScroll =
elements.scroller.scrollWidth - elements.scroller.clientWidth;
elements.scroller.scrollLeft = 0.75 * maxScroll;
elements.scroller.scrollLeft = -0.25 * maxScroll;
waitForNotNullLocalTime(animation).then(t.step_func_done(() => {
assert_equals(
@@ -124,11 +124,11 @@
const elements = createTestDOM(true, 'horizontal-tb', 'rtl');
const animation = createAndPlayTestAnimation(elements, 'inline');
// Move the scroller to the 75% point. Since it is direction: rtl, this is
// equivalent to the 25% point for the ScrollTimeline.
// Move the scroller to the left 25% point, since it is direction: rtl,
// i.e leftwards overflow direction, scrollLeft is -25% point.
const maxScroll =
elements.scroller.scrollWidth - elements.scroller.clientWidth;
elements.scroller.scrollLeft = 0.75 * maxScroll;
elements.scroller.scrollLeft = -0.25 * maxScroll;
waitForNotNullLocalTime(animation).then(t.step_func_done(() => {
assert_equals(
@@ -155,11 +155,11 @@
const elements = createTestDOM(false, 'vertical-lr', 'rtl');
const animation = createAndPlayTestAnimation(elements, 'inline');
// Move the scroller to the 75% point. Since this is a vertical writing mode
// and direction: rtl, this is 25% of the ScrollTimeline currentTime.
// Move the scroller to the top 25% point, since it is a vertical-lr writing mode
// and direction: rtl, i.e upwards overflow direction, scrollTop is -25% point.
const maxScroll =
elements.scroller.scrollHeight - elements.scroller.clientHeight;
elements.scroller.scrollTop = 0.75 * maxScroll;
elements.scroller.scrollTop = -0.25 * maxScroll;
waitForNotNullLocalTime(animation).then(t.step_func_done(() => {
assert_equals(
@@ -166,8 +166,8 @@
}
nodes = document.querySelectorAll(".rtl > .row-reverse");
for (var i = 0; i < nodes.length; i++) {
nodes[i].scrollLeft = 0;
nodes[i].scrollTop = 0;
nodes[i].scrollLeft = -10000;
nodes[i].scrollTop = -10000;
}
nodes = document.querySelectorAll(".column-reverse");
for (var i = 0; i < nodes.length; i++) {
@@ -176,7 +176,7 @@
}
nodes = document.querySelectorAll(".flipped-blocks > .column-reverse");
for (var i = 0; i < nodes.length; i++) {
nodes[i].scrollLeft = 0;
nodes[i].scrollLeft = -10000;
nodes[i].scrollTop = 0;
}
</script>
@@ -166,8 +166,8 @@
}
nodes = document.querySelectorAll(".rtl > .row-reverse");
for (var i = 0; i < nodes.length; i++) {
nodes[i].scrollLeft = 0;
nodes[i].scrollTop = 0;
nodes[i].scrollLeft = -10000;
nodes[i].scrollTop = -10000;
}
nodes = document.querySelectorAll(".column-reverse");
for (var i = 0; i < nodes.length; i++) {
@@ -176,7 +176,7 @@
}
nodes = document.querySelectorAll(".flipped-blocks > .column-reverse");
for (var i = 0; i < nodes.length; i++) {
nodes[i].scrollLeft = 0;
nodes[i].scrollLeft = -10000;
nodes[i].scrollTop = 0;
}
</script>
@@ -35,7 +35,7 @@
document.getElementById('scroller1').scrollTop = 50;
document.getElementById('scroller1').scrollLeft = 20;
document.getElementById('scroller2').scrollTop = 50;
document.getElementById('scroller2').scrollLeft = 60;
document.getElementById('scroller2').scrollLeft = -25;
});
</script>

@@ -44,7 +44,7 @@
document.getElementById('scroller1').scrollTop = 50;
document.getElementById('scroller1').scrollLeft = 20;
document.getElementById('scroller2').scrollTop = 50;
document.getElementById('scroller2').scrollLeft = 60;
document.getElementById('scroller2').scrollLeft = -25;
});
</script>

@@ -92,20 +92,6 @@
inlineEnd: ((2*box_height) - scroller_height) + scrollbar_width,
};
// As browsers differ in the meaning of scrollLeft when
// in a right-to-left mode, we adjust the expectation
// to match the semantics of scrollLeft.
// In vertical-rl mode, the scroll x coordinate should be nonpositive per the the spec.
// But some browsers is nonnegative, so we adjust the expectation.
scroller.scrollLeft = -1000;
if(scroller.scrollLeft === 0) {
expectedX = {
blockStart: ((2*box_width) - scroller_width) + scrollbar_width,
blockCenter: ((3*box_width - scroller_width)/2) + (scrollbar_width/2),
blockEnd: box_width,
};
}
[
[{block: "start", inline: "start"}, expectedX.blockStart, expectedY.inlineStart],
[{block: "start", inline: "center"}, expectedX.blockStart, expectedY.inlineCenter],
@@ -51,7 +51,7 @@
// The offset in the inline/horizontal direction should be inverted. The
// block/vertical direction should be unaffected.
scroller.scrollTop = 50;
scroller.scrollLeft = 75;
scroller.scrollLeft = 75 - scrollerSize;
assert_equals(blockScrollTimeline.currentTime, 50, 'Scrolled block timeline');
assert_equals(
@@ -105,7 +105,7 @@
// horizontal/vertical cases, horizontal starts on the right-hand-side and
// vertical is normal.
scroller.scrollTop = 50;
scroller.scrollLeft = 75;
scroller.scrollLeft = 75 - scrollerSize;
assert_equals(
blockScrollTimeline.currentTime, scrollerSize - 75,
@@ -207,52 +207,52 @@
assert_equals(
calcScrollTimeline.currentTime, null, 'Unscrolled calc-based timeline');
// With direction rtl offsets are inverted, such that scrollLeft ==
// scrollerSize is the 'zero' point for currentTime. However the
// With direction rtl offsets are inverted, such that scrollLeft == 0
// is the 'zero' point for currentTime. However the
// startScrollOffset is an absolute distance along the offset, so doesn't
// need adjusting.
// Check the length-based ScrollTimeline.
scroller.scrollLeft = scrollerSize;
scroller.scrollLeft = 0;
assert_equals(
lengthScrollTimeline.currentTime, null,
'Length-based timeline before the startScrollOffset point');
scroller.scrollLeft = scrollerSize - 20;
scroller.scrollLeft = -20;
assert_equals(
lengthScrollTimeline.currentTime, 0,
'Length-based timeline at the startScrollOffset point');
scroller.scrollLeft = scrollerSize - 50;
scroller.scrollLeft = -50;
assert_equals(
lengthScrollTimeline.currentTime,
calculateCurrentTime(50, 20, scrollerSize, scrollerSize),
'Length-based timeline after the startScrollOffset point');
// Check the percentage-based ScrollTimeline.
scroller.scrollLeft = scrollerSize - (0.19 * scrollerSize);
scroller.scrollLeft = -(0.19 * scrollerSize);
assert_equals(
percentageScrollTimeline.currentTime, null,
'Percentage-based timeline before the startScrollOffset point');
scroller.scrollLeft = scrollerSize - (0.20 * scrollerSize);
scroller.scrollLeft = -(0.20 * scrollerSize);
assert_equals(
percentageScrollTimeline.currentTime, 0,
'Percentage-based timeline at the startScrollOffset point');
scroller.scrollLeft = scrollerSize - (0.4 * scrollerSize);
scroller.scrollLeft = -(0.4 * scrollerSize);
assert_equals(
percentageScrollTimeline.currentTime,
calculateCurrentTime(
0.4 * scrollerSize, 0.2 * scrollerSize, scrollerSize, scrollerSize),
'Percentage-based timeline after the startScrollOffset point');
// Check the calc-based ScrollTimeline.
scroller.scrollLeft = scrollerSize - (0.2 * scrollerSize - 10);
scroller.scrollLeft = -(0.2 * scrollerSize - 10);
assert_equals(
calcScrollTimeline.currentTime, null,
'Calc-based timeline before the startScrollOffset point');
scroller.scrollLeft = scrollerSize - (0.2 * scrollerSize - 5);
scroller.scrollLeft = -(0.2 * scrollerSize - 5);
assert_equals(
calcScrollTimeline.currentTime, 0,
'Calc-based timeline at the startScrollOffset point');
scroller.scrollLeft = scrollerSize - (0.2 * scrollerSize);
scroller.scrollLeft = -(0.2 * scrollerSize);
assert_equals(
calcScrollTimeline.currentTime,
calculateCurrentTime(
@@ -288,53 +288,53 @@
endScrollOffset: 'calc(80% + 5px)'
});
// With direction rtl offsets are inverted, such that scrollLeft ==
// scrollerSize is the 'zero' point for currentTime. However the
// With direction rtl offsets are inverted, such that scrollLeft == 0
// is the 'zero' point for currentTime. However the
// endScrollOffset is an absolute distance along the offset, so doesn't need
// adjusting.
// Check the length-based ScrollTimeline.
scroller.scrollLeft = 0;
scroller.scrollLeft = -scrollerSize;
assert_equals(
lengthScrollTimeline.currentTime, null,
'Length-based timeline after the endScrollOffset point');
scroller.scrollLeft = 20;
scroller.scrollLeft = 20 - scrollerSize;
assert_equals(
lengthScrollTimeline.currentTime, null,
'Length-based timeline at the endScrollOffset point');
scroller.scrollLeft = 50;
scroller.scrollLeft = 50 - scrollerSize;
assert_equals(
lengthScrollTimeline.currentTime,
calculateCurrentTime(
scrollerSize - 50, 0, scrollerSize - 20, scrollerSize),
'Length-based timeline before the endScrollOffset point');
// Check the percentage-based ScrollTimeline.
scroller.scrollLeft = 0.19 * scrollerSize;
scroller.scrollLeft = 0.19 * scrollerSize - scrollerSize;
assert_equals(
percentageScrollTimeline.currentTime, null,
'Percentage-based timeline after the endScrollOffset point');
scroller.scrollLeft = 0.20 * scrollerSize;
scroller.scrollLeft = 0.20 * scrollerSize - scrollerSize;
assert_equals(
percentageScrollTimeline.currentTime, null,
'Percentage-based timeline at the endScrollOffset point');
scroller.scrollLeft = 0.4 * scrollerSize;
scroller.scrollLeft = 0.4 * scrollerSize - scrollerSize;
assert_equals(
percentageScrollTimeline.currentTime,
calculateCurrentTime(
0.6 * scrollerSize, 0, 0.8 * scrollerSize, scrollerSize),
'Percentage-based timeline before the endScrollOffset point');
// Check the calc-based ScrollTimeline. 80% + 5px
scroller.scrollLeft = 0.2 * scrollerSize - 10;
scroller.scrollLeft = -0.8 * scrollerSize - 10;
assert_equals(
calcScrollTimeline.currentTime, null,
'Calc-based timeline after the endScrollOffset point');
scroller.scrollLeft = 0.2 * scrollerSize - 5;
scroller.scrollLeft = -0.8 * scrollerSize - 5;
assert_equals(
calcScrollTimeline.currentTime, null,
'Calc-based timeline at the endScrollOffset point');
scroller.scrollLeft = 0.2 * scrollerSize;
scroller.scrollLeft = -0.8 * scrollerSize;
assert_equals(
calcScrollTimeline.currentTime,
calculateCurrentTime(
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.