Bug 223133 - [GTK] Kinetic iframe async scrolling does not work
Summary: [GTK] Kinetic iframe async scrolling does not work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alejandro G. Castro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-03-12 12:41 PST by Alejandro G. Castro
Modified: 2021-03-18 12:47 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.79 KB, patch)
2021-03-12 12:51 PST, Alejandro G. Castro
no flags Details | Formatted Diff | Diff
Patch for landing (1.95 KB, patch)
2021-03-18 12:21 PDT, Alejandro G. Castro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alejandro G. Castro 2021-03-12 12:41:11 PST
The end of momentum scroll event in gtk has a delta of zero, it is defined in the API, we have to add this situation to the detection of the ability to scroll an element or the event reaches the wrong element.

https://developer.gnome.org/gdk3/stable/gdk3-Events.html#gdk-event-is-scroll-stop-event
Comment 1 Alejandro G. Castro 2021-03-12 12:51:20 PST
Created attachment 423075 [details]
Patch
Comment 2 Carlos Garcia Campos 2021-03-15 02:29:30 PDT
Comment on attachment 423075 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423075&action=review

> Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp:173
> +#if PLATFORM(GTK)
> +    if (wheelEvent.isEndOfNonMomentumScroll())
> +        return true;
> +#endif

Maybe this check fits better in ScrollingTreeScrollingNode::canHandleWheelEvent() before calling eventCanScrollContents(), and including a comment explaining why.
Comment 3 Alejandro G. Castro 2021-03-15 04:13:04 PDT
(In reply to Carlos Garcia Campos from comment #2)
> Comment on attachment 423075 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=423075&action=review
> 
> > Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp:173
> > +#if PLATFORM(GTK)
> > +    if (wheelEvent.isEndOfNonMomentumScroll())
> > +        return true;
> > +#endif
> 
> Maybe this check fits better in
> ScrollingTreeScrollingNode::canHandleWheelEvent() before calling
> eventCanScrollContents(), and including a comment explaining why.

Thanks for the review.

The reason I like it in eventCanScrollContents is because that method is the one checking the delta in the event, which is what gtk is not adding in the end event, the delta. It is true we are just using the method here so for the moment we could do it in both places.

I'll add comment with some explanation of the if.
Comment 4 Alejandro G. Castro 2021-03-18 12:21:17 PDT
Created attachment 423632 [details]
Patch for landing
Comment 5 EWS 2021-03-18 12:47:29 PDT
Committed r274666: <https://commits.webkit.org/r274666>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423632 [details].