Bug 227691 - [GTK] Touchscreen navigation swipe doesn't work when the page scrolls horizontally
Summary: [GTK] Touchscreen navigation swipe doesn't work when the page scrolls horizon...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-07-05 14:44 PDT by Alexander Mikhaylenko
Modified: 2021-07-09 04:47 PDT (History)
6 users (show)

See Also:


Attachments
Patch (13.44 KB, patch)
2021-07-06 04:18 PDT, Alexander Mikhaylenko
no flags Details | Formatted Diff | Diff
Patch (14.47 KB, patch)
2021-07-07 06:02 PDT, Alexander Mikhaylenko
no flags Details | Formatted Diff | Diff
Patch (14.46 KB, patch)
2021-07-08 11:31 PDT, Alexander Mikhaylenko
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander 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 Alexander 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 Alexander Mikhaylenko 2021-07-06 04:43:40 PDT
Hm, there's a crash when tapping after a scroll.
Comment 3 Alexander 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 Alexander 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 Alexander 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 Alexander 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].