Bug 235851

Summary:

Allow history swipe in scroller with overscroll-behavior

Product:

WebKit

Reporter:

Nikos Mouchtaris <nmouchtaris>

Component:

New Bugs

Assignee:

Nikos Mouchtaris <nmouchtaris>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cmarcelo, ews-watchlist, fred.wang, jamesr, luiz, simon.fraser, tonikitoo, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=240183

Bug Depends on:

Bug Blocks:

235847

Attachments:

Description	Flags
wip	none
Patch	simon.fraser: review+
Patch	none

Nikos Mouchtaris

Reported 2022-01-28 19:08:02 PST

Test that scroller with overscroll-behavior-x doesn't allow swipe gestures to trigger navigation.

Attachments
wip (6.98 KB, patch) 2022-02-24 16:19 PST, Nikos Mouchtaris	no flags	Details Formatted Diff Diff
Patch (12.82 KB, patch) 2022-03-11 19:19 PST, Nikos Mouchtaris	simon.fraser: review+	Details Formatted Diff Diff
Patch (12.84 KB, patch) 2022-03-17 13:50 PDT, Nikos Mouchtaris	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2022-02-04 19:08:14 PST

<rdar://problem/88518462>

Nikos Mouchtaris

Comment 2 2022-02-24 16:19:24 PST

Created attachment 453149 [details] wip

Nikos Mouchtaris

Comment 3 2022-03-11 19:19:11 PST

Created attachment 454525 [details] Patch

Simon Fraser (smfr)

Comment 4 2022-03-16 17:03:01 PDT

Comment on attachment 454525 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454525&action=review > Source/WebCore/ChangeLog:3 > + Add test for swipe navigation with overscroll-behavior The title should be about the code change, not about adding the test. > Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp:439 > + ScrollPropagationInfo propagation; > + if (horizontalOverscrollBehaviorPreventsPropagation() || verticalOverscrollBehaviorPreventsPropagation()) { > + if (horizontalOverscrollBehaviorPreventsPropagation() && !delta.height() && delta.width()) { > + propagation.shouldBlockScrollPropagation = true; > + propagation.isHandled = false; > + return propagation; > + } > + > + if ((horizontalOverscrollBehaviorPreventsPropagation() && verticalOverscrollBehaviorPreventsPropagation()) > + || (horizontalOverscrollBehaviorPreventsPropagation() && !delta.height()) > + || (verticalOverscrollBehaviorPreventsPropagation() && !delta.width())) { > + propagation.shouldBlockScrollPropagation = true; > + propagation.isHandled = true; > + } > + } > + return propagation; This is a little hard to follow. Maybe an early return if neither access prevents propagation? > Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:44 > + bool shouldBlockScrollPropagation = false; > + bool isHandled = false; Normally we use C++ initializer style: bool shouldBlockScrollPropagation { false }; > LayoutTests/scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe-overscroll-behavior.html:56 > + eventSender.monitorWheelEvents(); > + eventSender.mouseMoveTo(x, y); > + eventSender.mouseScrollByWithWheelAndMomentumPhases(1, 0, "began", "none"); > + eventSender.mouseScrollByWithWheelAndMomentumPhases(10, 0, "changed", "none"); > + eventSender.mouseScrollByWithWheelAndMomentumPhases(10, 0, "changed", "none"); > + await UIHelper.animationFrame(); > + eventSender.mouseScrollByWithWheelAndMomentumPhases(10, 0, "changed", "none"); > + eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none"); > + return UIHelper.waitForScrollCompletion(); I prefer the await UIHelper.mouseWheelSequence(wheelEventSquence); technique; see if that works without the intermediate UIHelper.animationFrame().

Nikos Mouchtaris

Comment 5 2022-03-17 13:50:20 PDT

Created attachment 455022 [details] Patch

EWS

Comment 6 2022-03-18 15:00:28 PDT

Committed r291497 (248609@main): <https://commits.webkit.org/248609@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 455022 [details].

Note You need to log in before you can comment on or make changes to this bug.