Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: e94fbc9c6f2ad75e234c67d3642c0756c19679c8
https://github.com/WebKit/WebKit/commit/e94fbc9c6f2ad75e234c67d3642c0756c19679c8
Author: Wenson Hsieh <wenson_hs...@apple.com>
Date: 2023-03-24 (Fri, 24 Mar 2023)
Changed paths:
A
LayoutTests/fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset-expected.txt
A
LayoutTests/fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset.html
M LayoutTests/platform/glib/TestExpectations
M Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.cpp
M Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp
M Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h
M
Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp
Log Message:
-----------
REGRESSION (262041@main): Keyboard scrolling animates from (0, 0) when smooth
keyboard scrolling is disabled and UI-side compositing is enabled
https://bugs.webkit.org/show_bug.cgi?id=254428
Reviewed by Tim Horton.
My previous patch in 262041@main had a serious flaw — when decoding
`ScrollingStateScrollingNode`,
we'll decode a `RequestedScrollData` and use
`ScrollingStateScrollingNode::setRequestedScrollData`
to set the decoded data on the scrolling node. However, since:
- `setRequestedScrollData()` contains the request merging logic I added in
262041@main, and...
- We decode the changed property flags before calling `setRequestedScrollData`.
...we end up treating all animated scrolling as starting from the origin. This
patch fixes that by
keeping the merging logic in the web process, by passing in a
`CanMergeScrollData::No` flag when
deserializing the scrolling state node.
Test: fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset.html
*
LayoutTests/fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset-expected.txt:
Added.
*
LayoutTests/fast/scrolling/programmatic-smooth-scroll-from-nonzero-offset.html:
Added.
Add a new layout test to exercise the bug, by scrolling (without animation) to
a nonzero offset and
then performing an animated scroll from that offset, verifying that we don't
attempt to start from
(0, 0) when performing the animated scroll.
* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/page/scrolling/ScrollingCoordinatorTypes.cpp:
(WebCore::RequestedScrollData::merge):
After fixing the above issue, I also discovered that with UI-side compositing
enabled, the original
tests in `imported/w3c/web-platform-tests/css/cssom-view` that I tried to fix
started failing again;
this is because of another subtle corner case that wasn't covered in when
merging `RequestedScrollData`,
wherein the following sequence of programmatic scroll requests:
1. ScrollRequestType::PositionUpdate (Non-Animated)
2. ScrollRequestType::CancelAnimatedScroll
3. ScrollRequestType::PositionUpdate (Animated)
...caused us to stomp over the `requestedDataBeforeAnimatedScroll` in step (2),
when merging the
animated scroll cancellation request into the position update. To address this
and keep the WPT
passing with UI-side compositing enabled, I've adjusted the merging logic to
preserve `scrollPosition`
after step (2), so that we can stash it in the
`requestedDataBeforeAnimatedScroll` in step (3).
* Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp:
(WebCore::ScrollingStateScrollingNode::setRequestedScrollData):
* Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h:
*
Source/WebKit/Shared/RemoteLayerTree/RemoteScrollingCoordinatorTransaction.cpp:
(ArgumentCoder<ScrollingStateScrollingNode>::decode):
Canonical link: https://commits.webkit.org/262105@main
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
