Closed Bug 1553022 Opened 3 months ago Closed 3 months ago

Make nested-scrollIntoView-snaps.html in wpt pass

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: hiro, Assigned: hiro)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

There were two issues which make the test fail.

  1. We do descend down descendant elements inside a scroll container even if we meet other scroll containers there. The spec clearly says at the last sentence of Scroll Snap Model section that is that "Snap positions only affect the nearest ancestor scroll container on the element’s containing block chain"

  2. In this commit I did add code that makes a choice whether the given range is passed to ScrollToWithOrigin (or ScrollTo) or not to not constrain the final destination position modified by snapping. But it was not sufficient since ScrollToShowRect in nsPresShell also calls ScrollTo with an nsRange. What we should do instead is in ScrollToWithOrigin if we found a valid snap position in a call of GetSnapPointForDestination we ignore the given nsRange there.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=021985ca37393eda280b3804380fdf14a381eee2

From the last sentence of "3. Scroll Snap Model" [1] in the spec;

Snap positions only affect the nearest ancestor scroll container on the
element's containing block chain.

[1] https://drafts.csswg.org/css-scroll-snap-1/#overview

ScrollToShowRect calls nsIScrollableFrame::ScrollTo with an nsRange which
will be used to constrain the final scroll position so that if snapping needs
to happen we need to ignore the given range not to constrain the final
destination position in the range.

Depends on D31947

Attachment #9066301 - Attachment description: Bug 1553022 - Don't use the given range in ScrollToWithOrigin in the case where scroll snap will happen. → Bug 1553022 - Don't use the given range in ScrollToWithOrigin in the case where scroll snap will happen. r?botond
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9a556ee9f039
Stop descending down frame tree if we met another scrollable frame when we collect snap positions. r=botond
https://hg.mozilla.org/integration/autoland/rev/d136380e4945
Don't use the given range in ScrollToWithOrigin in the case where scroll snap will happen. r=botond

gcc isn't smart enough, I'd say. :/

Flags: needinfo?(hikezoe)
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f68ba99112d6
Stop descending down frame tree if we met another scrollable frame when we collect snap positions. r=botond
https://hg.mozilla.org/integration/autoland/rev/e882a5c576ab
Don't use the given range in ScrollToWithOrigin in the case where scroll snap will happen. r=botond
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Regressions: 1554022
Summary: Make scroll-snap-type-on-root-element.html in wpt pass → Make nested-scrollIntoView-snaps.html in wpt pass
Regressions: 1554023
You need to log in before you can comment on or make changes to this bug.