Bug 223133

Summary: [GTK] Kinetic iframe async scrolling does not work
Product: WebKit Reporter: Alejandro G. Castro <alex>
Component: WebKitGTKAssignee: Alejandro G. Castro <alex>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, cmarcelo, ews-watchlist, fred.wang, jamesr, luiz, tonikitoo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

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].