[css-scroll-snap] make resnap follow scroll snap target if necessary #3979

nmoucht · 2022-09-03T01:31:58Z

`5ed2b1d`

[css-scroll-snap] make resnap follow scroll snap target if necessary
https://bugs.webkit.org/show_bug.cgi?id=244745
<rdar://99557242>

Reviewed by Martin Robinson.

CSS scroll snap spec (https://www.w3.org/TR/css-scroll-snap-1/#re-snap): "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." To acheive this,
we add an id to scroll offset which represents the associated element to that scroll offset. We also add a bool which
represents wether the associated element is focused. The id of the currently snapped element is added
to ScrollSnapAnimatorState and for each relayout, check if it is necessary to preserve the currently
snapped element.

* LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/resnap-to-focused-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt:
* Source/WebCore/page/FrameView.cpp:
(WebCore::FrameView::updateSnapOffsets):
* Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp:
(WebCore::updateSnapOffsetsForScrollableArea):
(WebCore::convertOffsetInfo):
* Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.h:
(WebCore::operator<<):
* Source/WebCore/platform/ScrollSnapAnimatorState.cpp:
(WebCore::ScrollSnapAnimatorState::setFocusedElementForAxis):
(WebCore::ScrollSnapAnimatorState::preserveCurrentTargetForAxis):
(WebCore::ScrollSnapAnimatorState::resnapAfterLayout):
* Source/WebCore/platform/ScrollSnapAnimatorState.h:
(WebCore::ScrollSnapAnimatorState::activeSnapIDForAxis const):
(WebCore::ScrollSnapAnimatorState::setActiveSnapIndexIDForAxis):
* Source/WebCore/platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::resnapAfterLayout):
* Source/WebCore/rendering/RenderLayerScrollableArea.cpp:
(WebCore::RenderLayerScrollableArea::updateSnapOffsets):
* Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
(ArgumentCoder<SnapOffset<float>>::encode):
(ArgumentCoder<SnapOffset<float>>::decode):

Canonical link: https://commits.webkit.org/254773@main

f07c745

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	❌ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	~~🛠 mac-debug~~	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	✅ 🧪 api-gtk
	~~🛠 tv~~	~~🧪 mac-wk1~~
	~~🛠 tv-sim~~	✅ 🧪 mac-wk2
✅ 🛠 🧪 merge	~~🛠 watch~~	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-03T01:35:49Z

EWS run on previous version of this PR (hash 4655b02)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	❌ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	❌ 🛠 🧪 win
❌ 🧪 bindings	❌ 🛠 ios-sim	⏳ 🛠 mac-debug	❌ 🛠 gtk	⏳ 🛠 wincairo
❌ 🧪 webkitperl	❌ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	❌ 🧪 gtk-wk2
	❌ 🧪 api-ios	⏳ 🧪 api-mac	❌ 🧪 api-gtk
	❌ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	✅ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	❌ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	❌ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-03T01:41:39Z

EWS run on previous version of this PR (hash 1d6586c)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	~~🛠 🧪 win~~
✅ 🧪 bindings	✅ 🛠 ios-sim	⏳ 🛠 mac-debug	✅ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	⏳ 🛠 mac-AS-debug	~~🧪 gtk-wk2~~
	~~🧪 api-ios~~	⏳ 🧪 api-mac	~~🧪 api-gtk~~
	✅ 🛠 tv	⏳ 🧪 mac-wk1
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2
	✅ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-03T01:47:13Z

EWS run on previous version of this PR (hash fa60119)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	~~🛠 🧪 win~~
✅ 🧪 bindings	✅ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
	⏳ 🛠 tv	⏳ 🧪 mac-wk1
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2
	✅ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-03T01:50:22Z

EWS run on previous version of this PR (hash 96fc315)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	❌ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	❌ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	✅ 🧪 api-gtk
	✅ 🛠 tv	❌ 🧪 mac-wk1
	✅ 🛠 tv-sim	❌ 🧪 mac-wk2
	✅ 🛠 watch	❌ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

smfr

Tests are failing, so it seems this needs more work.

smfr · 2022-09-08T04:56:09Z

Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.h


 template <typename T>
 struct SnapOffset {
    T offset;
    ScrollSnapStop stop;
    bool hasSnapAreaLargerThanViewport;
+    unsigned snapTargetId;


Rather than storing this extra ID, and the associated (and tricky to maintain) renderer -> ID map, can we just compute the snap offset for the focus thing and send it along as the "preferred snap offset" or "preferred snap index"?

Any way that doesn't involve maintaining a RenderBox -> ID map would be preferable.

The issue with storing a snap index/offset is that it will change across layout, so when updateSnapOffsetsForScrollableArea is called, we have no way to figure out what the corresponding index/offset is. Instead of having an extra map, would it be preferable to replace the HashSet<const RenderBox*> already in RenderView with a set of structs which hold the RenderBox and id?

Actually this is not a great solution as we would have to do the work ourselves to maintain the set, due to the ids. This could of course work, but since we are maintaining the map in the same way as the set (which was on RenderView already), could you explain why this isn't preferable?

webkit-early-warning-system · 2022-09-12T20:14:13Z

EWS run on previous version of this PR (hash a0b60cb)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
~~🧪 style~~	~~🛠 ios~~	~~🛠 mac~~	~~🛠 wpe~~	~~🛠 🧪 win~~
~~🧪 bindings~~	✅ 🛠 ios-sim	~~🛠 mac-debug~~	~~🛠 gtk~~	~~🛠 wincairo~~
~~🧪 webkitperl~~	~~🧪 ios-wk2~~	~~🛠 mac-AS-debug~~	~~🧪 gtk-wk2~~
	~~🧪 api-ios~~	~~🧪 api-mac~~	~~🧪 api-gtk~~
	~~🛠 tv~~	~~🧪 mac-wk1~~
	~~🛠 tv-sim~~	~~🧪 mac-wk2~~
	~~🛠 watch~~	~~🧪 mac-AS-debug-wk2~~
	~~🛠 watch-sim~~	~~🧪 mac-wk2-stress~~

webkit-early-warning-system · 2022-09-12T20:14:28Z

EWS run on previous version of this PR (hash 18f2e06)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk1
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
	✅ 🛠 watch	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

Source/WebCore/platform/ScrollSnapAnimatorState.h

webkit-early-warning-system · 2022-09-20T01:00:52Z

EWS run on previous version of this PR (hash 1b22644)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
	✅ 🛠 tv	⏳ 🧪 mac-wk1
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2
	✅ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-20T01:03:05Z

EWS run on previous version of this PR (hash 1b1e602)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	❌ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk1
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
	✅ 🛠 watch	❌ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

mrobinson

Thanks for working on this tricky issue! I have a few comments, most of which are pretty minor, apart from one more serious concern about associated these new ids with the RenderView and not the Element.

Source/WebCore/platform/ScrollSnapAnimatorState.cpp

mrobinson · 2022-09-21T08:21:59Z

Source/WebCore/platform/ScrollSnapAnimatorState.cpp

+    if (activeHorizontalID && activeHorizontalIndex) {
+        if (*activeHorizontalID != snapOffsetsHorizontal[*activeHorizontalIndex].snapTargetId)


If combined into one if statement, this would be a bit clearer, I think. For instance:

if (activeHorizontalID && activeHorizontalIndex && *activeHorizontalID != snapOffsetsHorizontal[*activeHorizontalIndex].snapTargetId)

I think we want to leave the logic like this for horizontal axis. This is due to an implementation detail where we need to choose an axis to prioritize the target for (we choose horizontal). This prevents us from trying to follow the target of the vertical axis when we are already following the target of the horizontal axis.

Source/WebCore/platform/ScrollSnapAnimatorState.cpp

mrobinson · 2022-09-21T08:34:06Z

Source/WebCore/platform/ScrollSnapAnimatorState.cpp

+    }
+
+    // If we are snapped to multiple targets, save them
+    if ((!activeHorizontalID && activeHorizontalIndex) && (!activeVerticalID && activeVerticalIndex) && snapOffsetsForAxis(ScrollEventAxis::Horizontal)[*activeHorizontalIndex].snapTargetId != snapOffsetsForAxis(ScrollEventAxis::Vertical)[*activeVerticalIndex].snapTargetId) {


This should probably join the if-else-if sequence above, just to make it clearer that this will not happen if we are preserving a previous snap point. I was a bit thrown off by this at first, which is why I mention it.

At least to me it makes more sense to have them separate, as the first set of checks are for preserving the target for a particular axis, while the second check is for setting the targets if we don't have any. These seem logically separate to me. I updated the comment to make this difference a bit clearer.

Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.h

mrobinson · 2022-09-21T08:58:32Z

Source/WebCore/rendering/RenderView.cpp

@@ -957,11 +957,13 @@ RenderLayer* RenderView::takeStyleChangeLayerTreeMutationRoot()
 void RenderView::registerBoxWithScrollSnapPositions(const RenderBox& box)
 {
    m_boxesWithScrollSnapPositions.add(&box);
+    m_boxesWithScrollSnapPositionsIDMap.add(&box, m_boxesWithScrollSnapPositionsCurrentID++);


I wonder about this approach. The specification says that resnapping after layout should be preserved according the element, but IIUC style change can destroy and recreate an element's RenderView. Is there a way to rework this so that these identifiers are preserved when the RenderView is recreated?

While I'm not too familiar with the lifetime of RenderView, at least while testing this change I didn't see the RenderView being destroyed due to any style changes. However, another solution is to use the identifier() function on each Element. This also allows us to not maintain a map anymore, which Simon had some concerns about previously. This also might prevent any lifetime issues.

Thanks. The new approach looks great to me.

webkit-early-warning-system · 2022-09-21T21:30:40Z

EWS run on previous version of this PR (hash 6f4fc64)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-21T21:36:30Z

EWS run on previous version of this PR (hash 28cb69e)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	❌ 🧪 api-gtk
	⏳ 🛠 tv	✅ 🧪 mac-wk1
	⏳ 🛠 tv-sim	✅ 🧪 mac-wk2
	⏳ 🛠 watch	✅ 🧪 mac-AS-debug-wk2
	⏳ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp

nt1m · 2022-09-22T15:07:18Z

@nmoucht Also, it seems like the changelog in your commit message is out of date.

mrobinson

This looks good to me with the changes that @nt1m suggests. Thanks!

Source/WebCore/platform/ScrollSnapAnimatorState.h

Source/WebCore/platform/ScrollSnapAnimatorState.cpp

webkit-early-warning-system · 2022-09-22T18:51:23Z

EWS run on previous version of this PR (hash d086925)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
~~🧪 style~~	~~🛠 ios~~	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
~~🧪 bindings~~	~~🛠 ios-sim~~	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
~~🧪 webkitperl~~	~~🧪 ios-wk2~~	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	~~🧪 api-ios~~	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
	⏳ 🛠 tv	⏳ 🧪 mac-wk1
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-22T19:14:07Z

EWS run on previous version of this PR (hash 4babab7)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
~~🧪 style~~	~~🛠 ios~~	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
~~🧪 bindings~~	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
~~🧪 webkitperl~~	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-09-22T19:23:47Z

EWS run on current version of this PR (hash f07c745)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	❌ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	~~🛠 mac-debug~~	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	✅ 🧪 api-gtk
	~~🛠 tv~~	~~🧪 mac-wk1~~
	~~🛠 tv-sim~~	✅ 🧪 mac-wk2
✅ 🛠 🧪 merge	~~🛠 watch~~	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

https://bugs.webkit.org/show_bug.cgi?id=244745 <rdar://99557242> Reviewed by Martin Robinson. CSS scroll snap spec (https://www.w3.org/TR/css-scroll-snap-1/#re-snap): "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." To acheive this, we add an id to scroll offset which represents the associated element to that scroll offset. We also add a bool which represents wether the associated element is focused. The id of the currently snapped element is added to ScrollSnapAnimatorState and for each relayout, check if it is necessary to preserve the currently snapped element. * LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/resnap-to-focused-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt: * Source/WebCore/page/FrameView.cpp: (WebCore::FrameView::updateSnapOffsets): * Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp: (WebCore::updateSnapOffsetsForScrollableArea): (WebCore::convertOffsetInfo): * Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.h: (WebCore::operator<<): * Source/WebCore/platform/ScrollSnapAnimatorState.cpp: (WebCore::ScrollSnapAnimatorState::setFocusedElementForAxis): (WebCore::ScrollSnapAnimatorState::preserveCurrentTargetForAxis): (WebCore::ScrollSnapAnimatorState::resnapAfterLayout): * Source/WebCore/platform/ScrollSnapAnimatorState.h: (WebCore::ScrollSnapAnimatorState::activeSnapIDForAxis const): (WebCore::ScrollSnapAnimatorState::setActiveSnapIndexIDForAxis): * Source/WebCore/platform/ScrollableArea.cpp: (WebCore::ScrollableArea::resnapAfterLayout): * Source/WebCore/rendering/RenderLayerScrollableArea.cpp: (WebCore::RenderLayerScrollableArea::updateSnapOffsets): * Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp: (ArgumentCoder<SnapOffset<float>>::encode): (ArgumentCoder<SnapOffset<float>>::decode): Canonical link: https://commits.webkit.org/254773@main

webkit-commit-queue · 2022-09-23T01:28:10Z

Committed 254773@main (5ed2b1d): https://commits.webkit.org/254773@main

Reviewed commits have been landed. Closing PR #3979 and removing active labels.

nmoucht requested a review from cdumez as a code owner September 3, 2022 01:31

nmoucht self-assigned this Sep 3, 2022

nmoucht added New Bugs Unclassified bugs are placed in this component until the correct component can be determined. WebKit Nightly Build labels Sep 3, 2022

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch from 4655b02 to 1d6586c Compare September 3, 2022 01:41

nmoucht requested review from smfr and whsieh September 3, 2022 01:42

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch from 1d6586c to fa60119 Compare September 3, 2022 01:47

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch from fa60119 to 96fc315 Compare September 3, 2022 01:50

nmoucht requested a review from nt1m September 3, 2022 02:32

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 3, 2022

nt1m requested a review from mrobinson September 3, 2022 14:13

smfr requested changes Sep 8, 2022

View reviewed changes

nmoucht removed the merging-blocked Applied to prevent a change from being merged label Sep 12, 2022

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch 2 times, most recently from a0b60cb to 18f2e06 Compare September 12, 2022 20:08

nmoucht requested a review from smfr September 12, 2022 20:31

nt1m reviewed Sep 19, 2022

View reviewed changes

Source/WebCore/platform/ScrollSnapAnimatorState.h Outdated Show resolved Hide resolved

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch from 18f2e06 to 1b22644 Compare September 20, 2022 01:00

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch from 1b22644 to 1b1e602 Compare September 20, 2022 01:02

nmoucht requested a review from nt1m September 20, 2022 01:03

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 20, 2022

mrobinson requested changes Sep 21, 2022

View reviewed changes

nmoucht removed the merging-blocked Applied to prevent a change from being merged label Sep 21, 2022

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch from 1b1e602 to 6f4fc64 Compare September 21, 2022 21:30

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch from 6f4fc64 to 28cb69e Compare September 21, 2022 21:35

nmoucht requested a review from mrobinson September 21, 2022 21:47

nt1m reviewed Sep 22, 2022

View reviewed changes

Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp Outdated Show resolved Hide resolved

mrobinson approved these changes Sep 22, 2022

View reviewed changes

nt1m reviewed Sep 22, 2022

View reviewed changes

Source/WebCore/platform/ScrollSnapAnimatorState.h Outdated Show resolved Hide resolved

Source/WebCore/platform/ScrollSnapAnimatorState.cpp Outdated Show resolved Hide resolved

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch 2 times, most recently from d086925 to 4babab7 Compare September 22, 2022 18:32

nmoucht force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch from 4babab7 to f07c745 Compare September 22, 2022 18:51

nmoucht added the merge-queue Applied to send a pull request to merge-queue label Sep 22, 2022

webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Sep 22, 2022

nmoucht added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Sep 23, 2022

webkit-early-warning-system force-pushed the eng/css-scroll-snap-make-resnap-follow-scroll-snap-target-if-necessary branch from f07c745 to 5ed2b1d Compare September 23, 2022 01:28

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 23, 2022

webkit-early-warning-system merged commit 5ed2b1d into WebKit:main Sep 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[css-scroll-snap] make resnap follow scroll snap target if necessary #3979

[css-scroll-snap] make resnap follow scroll snap target if necessary #3979

nmoucht commented Sep 3, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Sep 3, 2022 •

edited

webkit-early-warning-system commented Sep 3, 2022 •

edited

webkit-early-warning-system commented Sep 3, 2022 •

edited

webkit-early-warning-system commented Sep 3, 2022 •

edited

smfr left a comment

smfr Sep 8, 2022

nmoucht Sep 8, 2022

nmoucht Sep 12, 2022

webkit-early-warning-system commented Sep 12, 2022 •

edited

webkit-early-warning-system commented Sep 12, 2022 •

edited

webkit-early-warning-system commented Sep 20, 2022 •

edited

webkit-early-warning-system commented Sep 20, 2022 •

edited

mrobinson left a comment

mrobinson Sep 21, 2022

nmoucht Sep 21, 2022

mrobinson Sep 21, 2022

nmoucht Sep 21, 2022

mrobinson Sep 21, 2022

nmoucht Sep 21, 2022 •

edited

mrobinson Sep 22, 2022

webkit-early-warning-system commented Sep 21, 2022 •

edited

webkit-early-warning-system commented Sep 21, 2022 •

edited

nt1m commented Sep 22, 2022

mrobinson left a comment

webkit-early-warning-system commented Sep 22, 2022 •

edited

webkit-early-warning-system commented Sep 22, 2022 •

edited

webkit-early-warning-system commented Sep 22, 2022 •

edited

webkit-commit-queue commented Sep 23, 2022

		if (activeHorizontalID && activeHorizontalIndex) {
		if (activeHorizontalID != snapOffsetsHorizontal[activeHorizontalIndex].snapTargetId)

[css-scroll-snap] make resnap follow scroll snap target if necessary #3979

[css-scroll-snap] make resnap follow scroll snap target if necessary #3979

Conversation

nmoucht commented Sep 3, 2022 • edited by webkit-early-warning-system

webkit-early-warning-system commented Sep 3, 2022 • edited

webkit-early-warning-system commented Sep 3, 2022 • edited

webkit-early-warning-system commented Sep 3, 2022 • edited

webkit-early-warning-system commented Sep 3, 2022 • edited

smfr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Sep 12, 2022 • edited

webkit-early-warning-system commented Sep 12, 2022 • edited

webkit-early-warning-system commented Sep 20, 2022 • edited

webkit-early-warning-system commented Sep 20, 2022 • edited

mrobinson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmoucht Sep 21, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Sep 21, 2022 • edited

webkit-early-warning-system commented Sep 21, 2022 • edited

nt1m commented Sep 22, 2022

mrobinson left a comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Sep 22, 2022 • edited

webkit-early-warning-system commented Sep 22, 2022 • edited

webkit-early-warning-system commented Sep 22, 2022 • edited

webkit-commit-queue commented Sep 23, 2022

nmoucht commented Sep 3, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Sep 3, 2022 •

edited

webkit-early-warning-system commented Sep 3, 2022 •

edited

webkit-early-warning-system commented Sep 3, 2022 •

edited

webkit-early-warning-system commented Sep 3, 2022 •

edited

webkit-early-warning-system commented Sep 12, 2022 •

edited

webkit-early-warning-system commented Sep 12, 2022 •

edited

webkit-early-warning-system commented Sep 20, 2022 •

edited

webkit-early-warning-system commented Sep 20, 2022 •

edited

nmoucht Sep 21, 2022 •

edited

webkit-early-warning-system commented Sep 21, 2022 •

edited

webkit-early-warning-system commented Sep 21, 2022 •

edited

webkit-early-warning-system commented Sep 22, 2022 •

edited

webkit-early-warning-system commented Sep 22, 2022 •

edited

webkit-early-warning-system commented Sep 22, 2022 •

edited