-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
REGRESSION (UI-side compositing): Unable to scroll to top of page aft…
…er double tapping trackpad causes page to scroll down https://bugs.webkit.org/show_bug.cgi?id=266529 rdar://116419609 Reviewed by Tim Horton. The smart magnify gesture (two-finger double tap on the trackpad) results in some combination of scrolling and zooming. `RemoteLayerTreeDrawingAreaProxyMac::commitTransientZoom()` uses a CAAnimation to animate to the final state, but the completion handler of that animation put the transform directly onto the `layerForPageScale`. That's OK if the gesture results in a scale change; that transform will get replaced by one that comes from the web process. However, if a gesture just resulted in a scroll with no scale change, we never get a new transform from the web process, so we leave that transform on `layerForPageScale`. This transform has a translation baked in, so this causes a permanent scroll offset. So we have to stop shoving the transform onto this layer. This revealed an issue where there was a jump when the animation was removed. Fixing this just requires two things: 1. Delay removing the transient zoom animations until we know the web process has committed the new scale and scroll position (via a `callAfterNextPresentationUpdate()`). 2. Override any scroll position changes that come from the web process before this by adding a CAAnimation onto the scrolled contents layer which overrides the `position` property. If we don't do this, scroll position changes cause a flash of offset content. This requires tracking m_pageScrollingLayerID, but that information was already in the transaction for banner hookup. * LayoutTests/view-gestures/smart-magnify/double-tap-zoom-scroll-above-top-expected.html: Added. * LayoutTests/view-gestures/smart-magnify/double-tap-zoom-scroll-above-top.html: Added. * Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeDrawingAreaProxyMac.h: * Source/WebKit/UIProcess/RemoteLayerTree/mac/RemoteLayerTreeDrawingAreaProxyMac.mm: (WebKit::RemoteLayerTreeDrawingAreaProxyMac::didCommitLayerTree): (WebKit::fillFowardsAnimationWithKeyPath): (WebKit::transientZoomTransformOverrideAnimation): (WebKit::RemoteLayerTreeDrawingAreaProxyMac::applyTransientZoomToLayer): (WebKit::RemoteLayerTreeDrawingAreaProxyMac::removeTransientZoomFromLayer): (WebKit::RemoteLayerTreeDrawingAreaProxyMac::commitTransientZoom): (WebKit::RemoteLayerTreeDrawingAreaProxyMac::sendCommitTransientZoom): Canonical link: https://commits.webkit.org/272229@main
- Loading branch information
Showing
4 changed files
with
130 additions
and
20 deletions.
There are no files selected for viewing
27 changes: 27 additions & 0 deletions
27
LayoutTests/view-gestures/smart-magnify/double-tap-zoom-scroll-above-top-expected.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <style> | ||
| body { | ||
| height: 2000px; | ||
| margin: 0; | ||
| } | ||
| .target { | ||
| margin-top: 400px; | ||
| width: 100%; | ||
| height: 1200px; | ||
| background-color: silver; | ||
| } | ||
|
|
||
| .top-stripe { | ||
| width: 100%; | ||
| height: 20px; | ||
| background-color: green; | ||
| } | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <div class="top-stripe"></div> | ||
| <div class="target"></div> | ||
| </body> | ||
| </html> |
48 changes: 48 additions & 0 deletions
48
LayoutTests/view-gestures/smart-magnify/double-tap-zoom-scroll-above-top.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <style> | ||
| body { | ||
| height: 2000px; | ||
| margin: 0; | ||
| } | ||
| .target { | ||
| margin-top: 400px; | ||
| width: 100%; | ||
| height: 1200px; | ||
| background-color: silver; | ||
| } | ||
|
|
||
| .top-stripe { | ||
| width: 100%; | ||
| height: 20px; | ||
| background-color: green; | ||
| } | ||
| </style> | ||
| <script src="../../resources/ui-helper.js"></script> | ||
| <script> | ||
| if (window.testRunner) | ||
| testRunner.waitUntilDone(); | ||
|
|
||
| async function doSmartMagnify() | ||
| { | ||
| await UIHelper.setWebViewAllowsMagnification(true); | ||
| await UIHelper.smartMagnifyAt(400, 500); | ||
|
|
||
| window.scrollTo(0, 0); | ||
| await UIHelper.ensurePresentationUpdate(); | ||
|
|
||
| if (window.testRunner) | ||
| testRunner.notifyDone(); | ||
| } | ||
|
|
||
| window.addEventListener('load', () => { | ||
| doSmartMagnify(); | ||
| }, false); | ||
| </script> | ||
| </head> | ||
| <body> | ||
| <div class="top-stripe"></div> | ||
| <div class="target"></div> | ||
| </body> | ||
| </html> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters