Bug 223133 – [GTK] Kinetic iframe async scrolling does not work

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alejandro G. Castro]

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

[Alejandro G. Castro]

Created attachment 423075 [details]
Patch

[Carlos Garcia Campos]

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.

[Alejandro G. Castro]

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

[Alejandro G. Castro]

Created attachment 423632 [details]
Patch for landing

[EWS]

Committed r274666: <https://commits.webkit.org/r274666>

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