Skip to content
Permalink
Browse files
[css-scroll-snap] only preserve a target if we have targets for both …
…axes

https://bugs.webkit.org/show_bug.cgi?id=245989
<rdar://100737724>

Reviewed by Simon Fraser.

The preserving of a particular snap target was intended to follow this line
of the spec: "If multiple boxes were snapped before and their snap positions
no longer coincide, then if one of them is focused or targeted, the scroll
container must re-snap to that one and otherwise which one to re-snap to is
UA-defined". Since it is specified that this is for the case of mulitple boxes
being snapped, change the logic so that we only preserve a target if we are
snapped to multiple boxes.

* LayoutTests/TestExpectations:
* Source/WebCore/platform/ScrollSnapAnimatorState.cpp:
(WebCore::ScrollSnapAnimatorState::setFocusedElementForAxis):
(WebCore::ScrollSnapAnimatorState::preserveCurrentTargetForAxis):
(WebCore::ScrollSnapAnimatorState::resnapAfterLayout):

Canonical link: https://commits.webkit.org/255493@main
  • Loading branch information
nmoucht committed Oct 13, 2022
1 parent 7053604 commit 501296a3e648f26c2612145395269563725fed2b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 9 deletions.
@@ -6066,8 +6066,6 @@ imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/cs
imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/css-module/cors-crossorigin-requests.html [ Pass Failure ]
imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/json-module/cors-crossorigin-requests.html [ Pass Failure ]

webkit.org/b/245164 css3/scroll-snap/scroll-snap-drag-scrollbar-thumb-with-relayouts.html [ Failure Pass ]

# Disable ShadowRealm (while running to ensure we do not crash)
webkit.org/b/245680 http/tests/misc/iframe-shadow-realm.html [ Skip ]
inspector/shadow-realm-console.html [ Pass Failure ]
@@ -103,6 +103,7 @@ void ScrollSnapAnimatorState::setFocusedElementForAxis(ScrollEventAxis axis)
});
if (found == snapOffsets.end())
return;

auto newIndex = std::distance(snapOffsets.begin(), found);
auto newID = snapOffsets[newIndex].snapTargetID;
setActiveSnapIndexForAxis(axis, newIndex);
@@ -117,8 +118,10 @@ bool ScrollSnapAnimatorState::preserveCurrentTargetForAxis(ScrollEventAxis axis)
auto found = std::find_if(snapOffsets.begin(), snapOffsets.end(), [snapID](SnapOffset<LayoutUnit> p) -> bool {
return p.snapTargetID == *snapID;
});
if (found == snapOffsets.end())
if (found == snapOffsets.end()) {
setActiveSnapIndexIDForAxis(axis, std::nullopt);
return false;
}

setActiveSnapIndexForAxis(axis, std::distance(snapOffsets.begin(), found));
return true;
@@ -148,12 +151,11 @@ bool ScrollSnapAnimatorState::resnapAfterLayout(ScrollOffset scrollOffset, const
activeHorizontalIndex = activeSnapIndexForAxis(ScrollEventAxis::Horizontal);
}

// If we have an active target, see if we need to preserve it
if (activeHorizontalID && activeHorizontalIndex) {
if (*activeHorizontalID != snapOffsetsHorizontal[*activeHorizontalIndex].snapTargetID)
snapPointChanged |= preserveCurrentTargetForAxis(ScrollEventAxis::Horizontal);
} else if (activeVerticalID && activeVerticalIndex && *activeVerticalID != snapOffsetsVertical[*activeVerticalIndex].snapTargetID)
snapPointChanged |= preserveCurrentTargetForAxis(ScrollEventAxis::Vertical);
// If we have active targets, see if we need to preserve one of them
if (activeHorizontalID && activeHorizontalIndex && activeVerticalID && activeVerticalIndex && *activeHorizontalID
!= snapOffsetsHorizontal[*activeHorizontalIndex].snapTargetID && *activeVerticalID
!= snapOffsetsVertical[*activeVerticalIndex].snapTargetID)
snapPointChanged |= preserveCurrentTargetForAxis(ScrollEventAxis::Horizontal);

// If we do not have current targets and are snapped to multiple targets, set them
if ((!activeHorizontalID && activeHorizontalIndex) && (!activeVerticalID && activeVerticalIndex)) {

0 comments on commit 501296a

Please sign in to comment.