Closed Bug 1552089 Opened 5 months ago Closed 5 months ago

Programmatic scroll doesn't omit snap target elements which are outside of snapport in RTL scroll container

Categories

(Core :: Layout: Scrolling and Overflow, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: hiro, Assigned: hiro)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached file An example

Attachment file is a test case in snap-inline-block.html, and you will see a blue rectangle in the RTL scroll container on current nightly but the blue rectangle should NOT be appeared.

The test has an RTL scroll container whose width is 500px and a blue box positioned at left: -320px and its width is 200px. So if we call scrollTo(0, 0) there, the blue box is not inside the snapport thus we shouldn't snap to the blue box. That means that where we call scrollTo(0,0) for RTL scroll container is totally wrong, it should be a negative value that the target element appears inside the snapport to some extent. That also means that the code I added for RTL scroll contaier is also totally wrong. We don't need it at all. :/ That's because in the case of RTL scroll container, we, in the first place, call scrollTo with a negative x value if we want to scroll leftward.

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

Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/14743da36853
Don't tweak snapport position even in the case of RTL scroll containers. r=botond
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16909 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.