Bug 1552089 - Don't tweak snapport position even in the case of RTL scroll containers. r=botond

In RTL scroll containers, the right most x-axis scroll position is 0 and
leftward scroll positions are negative values.  And also
nsLayoutUtils::TransformFrameRectToAncestor, which is used to tell whether the
snap target element is inside the destination snapport or not [1], returns
negative x-axis positions for elements in RTL scroll containers if the element
is positioned at places where the elements are outside of the initial scroll
position (0, 0).  So we don't need to tweak snapport postion even in the case
of RTL scroll containers.

Instead, what we needed in the first place is that we choose a proper x-axis
scroll position that the targe element appears inside the snapport.

[1] https://searchfox.org/mozilla-central/rev/11cfa0462a6b5d8c5e2111b8cfddcf78098f0141/layout/generic/nsGfxScrollFrame.cpp#6604-6605,6616-6617

Depends on D31409

Differential Revision: https://phabricator.services.mozilla.com/D31410

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Hiroyuki Ikezoe 2019-05-17 20:36:57 +00:00
parent f2920c13d8
commit ba8e59a283
3 changed files with 11 additions and 12 deletions

View File

@ -6922,12 +6922,7 @@ layers::ScrollSnapInfo ScrollFrameHelper::ComputeScrollSnapInfo(
Maybe<nsRect> snapportOnDestination;
if (aDestination) {
if (IsPhysicalLTR()) {
snapport.MoveTo(aDestination.value());
} else {
snapport.MoveTo(
nsPoint(aDestination->x - snapport.Size().width, aDestination->y));
}
snapport.MoveTo(aDestination.value());
snapport.Deflate(scrollPadding);
snapportOnDestination.emplace(snapport);
} else {

View File

@ -1,7 +1,7 @@
[scroll-snap-type-on-root-element.html]
[The writing-mode on the body is used]
expected: FAIL
bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1552089
expected:
if (os == "android") and e10s: FAIL
[The scroll-snap-type on the root element is applied]
expected:

View File

@ -44,8 +44,10 @@ const scroller_height = scroller.clientHeight;
target.style.scrollSnapAlign = "end start";
if (writing_mode == "vertical-rl") {
target.style.left = (scroller_width - 700) + "px";
scroller.scrollTo(-500, 0);
} else {
scroller.scrollTo(0, 0);
}
scroller.scrollTo(0, 0);
assert_equals(scroller.scrollLeft, left, "aligns correctly on x");
assert_equals(scroller.scrollTop, top, "aligns correctly on y");
target.style.left = target_left;
@ -65,8 +67,10 @@ const scroller_height = scroller.clientHeight;
target.style.scrollSnapAlign = "start end";
if (writing_mode == "vertical-rl") {
target.style.left = (scroller_width - 700) + "px";
scroller.scrollTo(-500, 0);
} else {
scroller.scrollTo(0, 0);
}
scroller.scrollTo(0, 0);
assert_equals(scroller.scrollLeft, left, "aligns correctly on x");
assert_equals(scroller.scrollTop, top, "aligns correctly on y");
target.style.left = target_left;
@ -81,7 +85,7 @@ test(() => {
target.style.scrollSnapAlign = "end start";
target.style.left = (scroller_width - 700) + "px";
scroller.scrollTo(0, 0);
scroller.scrollTo(-500, 0);
assert_equals(scroller.scrollLeft, target.clientWidth - 700,
"aligns correctly on x");
assert_equals(scroller.scrollTop, 500 - scroller_height,
@ -98,7 +102,7 @@ test(() => {
target.style.scrollSnapAlign = "start end";
target.style.left = (scroller_width - 700) + "px";
scroller.scrollTo(0, 0);
scroller.scrollTo(-500, 0);
assert_equals(scroller.scrollLeft, scroller_width - 700,
"aligns correctly on x");
assert_equals(scroller.scrollTop, 300, "aligns correctly on y");