Bug 227691

Summary: [GTK] Touchscreen navigation swipe doesn't work when the page scrolls horizontally
Product: WebKit Reporter: Alice Mikhaylenko <alicem>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, cgarcia, ews-watchlist, gustavo, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Alice Mikhaylenko 2021-07-05 14:44:06 PDT
This may sound like https://bugs.webkit.org/show_bug.cgi?id=226173 but it's ever so slightly different.

That bug happens with GTK4 only and the reason for it is different, so filing this one separately.
Comment 1 Alice Mikhaylenko 2021-07-06 04:18:56 PDT
Created attachment 432917 [details]
Patch

Probably doesn't apply without https://bugs.webkit.org/show_bug.cgi?id=226745
Comment 2 Alice Mikhaylenko 2021-07-06 04:43:40 PDT
Hm, there's a crash when tapping after a scroll.
Comment 3 Alice Mikhaylenko 2021-07-06 06:24:58 PDT
It's the fact that we put the event back if WebCore hasn't handled them and then it gets processed again even though it shouldn't be.

Makes me wonder how or why it works for scroll events.
Comment 4 Alice Mikhaylenko 2021-07-07 06:02:51 PDT
Created attachment 433023 [details]
Patch

Fixed. For now, just don't propagate touch events, there's no real point in doing that anyway.
Comment 5 Alice Mikhaylenko 2021-07-08 11:31:44 PDT
Created attachment 433148 [details]
Patch

Oh wow, so the style script can determine if a name is redundant or not?
Comment 6 EWS Watchlist 2021-07-08 11:32:42 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 7 Michael Catanzaro 2021-07-09 03:56:37 PDT
(In reply to Alexander Mikhaylenko from comment #5)
> Created attachment 433148 [details]
> Patch
> 
> Oh wow, so the style script can determine if a name is redundant or not?

Yeah that's a little weird, but the rule is to name the parameter only in the source file, and leave it unnamed in the header file to save a bit of space (if the name is going to be something obvious like e.g. "Point point").
Comment 8 Alice Mikhaylenko 2021-07-09 04:28:52 PDT
Yeah, I know the rule, but didn't think the script could catch that.
Comment 9 Michael Catanzaro 2021-07-09 04:43:53 PDT
Comment on attachment 433148 [details]
Patch

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

> Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp:444
> +    // Wheel events can have either scroll events or touch events attached to them.
> +    // We only want to propagate scroll events; touch events are controlled via their
> +    // event sequences and if we're scrolling with touch events, that sequence is
> +    // already claimed and there's no point in propagating it.

Thanks for adding a comment to explain the weird behavior.
Comment 10 EWS 2021-07-09 04:47:42 PDT
Committed r279779 (239546@main): <https://commits.webkit.org/239546@main>

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