Skip to content

Commit

Permalink
Bug 1552089 - Don't tweak snapport position even in the case of RTL s…
Browse files Browse the repository at this point in the history
…croll 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
  • Loading branch information
Hiroyuki Ikezoe committed May 17, 2019
1 parent f2920c1 commit ba8e59a
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 12 deletions.
7 changes: 1 addition & 6 deletions layout/generic/nsGfxScrollFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@
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;
Expand All @@ -65,8 +67,10 @@
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;
Expand All @@ -81,7 +85,7 @@
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,
Expand All @@ -98,7 +102,7 @@
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");
Expand Down

0 comments on commit ba8e59a

Please sign in to comment.