Closed Bug 1766192 Opened 1 year ago Closed 1 year ago

Do `snap-scope` with the snapped target point rather than the scroll's destination


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




102 Branch
Tracking Status
firefox102 --- fixed


(Reporter: hiro, Assigned: hiro)


(Regressed 1 open bug, )



(6 files)

I did wrongly implement the snap-scope feature by using the snapport on the destination.

From the spec text;

a scroll position cannot be considered a valid snap position if snapping to it would leave the contributing snap area entirely outside the snapport

I believe snapping to it clearly means "snapping to the candidate snap point".

This incorrect implementation has made some wpt tests failure, scrollTo-scrollBY-snaps.html for example. And the incorrect implementation has been made some wpt pass in overflowing-snap-areas.html.

Severity: -- → S3
Assignee: nobody → hikezoe.birchill

The second best edge is used for snap-overflow feature [1]. The
snap-overflow is applied when the following conditions are met.

  1. A scroll snap target element is larger than the snapport (e.g. scrollport)
    in an axis
  2. The distance between the best edge and the second best edge (on the opposite
    side of the best edge) is later than the snapport size in the axis

There was a problem in our implementation. For example, in below diagram, D is
the original destination of the given scroll operation, 1 is the best edge, 2 is
the second best in our original implementation and 3 is of of the other snap
points. In this case if there's an element larger than the distance between 1
and 3 and if the snapport size is larger than the distance between 1 and 2 but
smaller than the distance between 1 and 3, we should apply snap-overflow.

2 1 D 3

There are a couple of test cases in wpt for this condition, for example there's
one of them in oveflowing-snap-area.html [2]. That test case have been passed
incorrectly due to our incorrect snap-scope implementation which will be fixed
a subsequent change in this commit series.


Depends on D144530

In the next change we will use the snap-area to implement snap-score

Depends on D144531

For references I am attaching a test case that Botond raised in a review comment.

See Also: → 1768393
Keywords: leave-open
Pushed by
Drop CalcSnapPoints::AddEdgeInterval. r=botond
Choose the second best edge on the opposite side of the best edge. r=botond
Introduce SnapTarget struct along with snap-area for each element. r=botond
Do `snap-scope` with the snapped point rather than the destination point. r=botond
Comment on attachment 9275658 [details]
A test case Botond raised in a review comment

><!DOCTYPE html>
>div {
>  position: absolute;
>  width: 100%;
>#scroller {
>  left: 10px;
>  height: 200px;
>  width: 100px;
>  overflow: scroll;
>  scroll-snap-type: y mandatory;
>.snap {
>  scroll-snap-align: start;
>  background-color: green;
>.target {
>  background-color: red;
>  border-radius: 100%;
>  height: 88px;
>  top: 536px;
><div id="scroller">
>  <div style="height: 2000px;"></div>
>  <div class="snap" style="top:    0px; height:   40px;">1</div>
>  <div class="snap" style="top:   40px; height:   40px;">2</div>
>  <div class="snap" style="top:   80px; height: 1000px;">3</div>
>  <div class="snap" style="top: 1080px; height:   40px;">4</div>
>  <div class="snap" style="top: 1120px; height:   40px;">5</div>
>  <div class="target"></div>
Attachment #9275658 - Attachment description: A test case Botond raied in a review comment → A test case Botond raised in a review comment
Regressions: 1768701
Keywords: leave-open
Pushed by
Add a wpt that scrolling to a position on a large element but the element doesn't cover the snapport. r=botond
Created web-platform-tests PR for changes under testing/web-platform/tests
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.