Bug 215368 - iOS: Scrolling and touch events sporadically stop working after navigating
Summary: iOS: Scrolling and touch events sporadically stop working after navigating
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-11 03:53 PDT by Tim Horton
Modified: 2020-08-11 14:04 PDT (History)
7 users (show)

See Also:


Attachments
Patch (22.17 KB, patch)
2020-08-11 03:53 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (21.97 KB, patch)
2020-08-11 03:55 PDT, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (21.93 KB, patch)
2020-08-11 13:17 PDT, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2020-08-11 03:53:09 PDT
iOS: Scrolling and touch events sporadically stop working after navigating
Comment 1 Tim Horton 2020-08-11 03:53:57 PDT
Created attachment 406372 [details]
Patch
Comment 2 Tim Horton 2020-08-11 03:53:59 PDT
<rdar://problem/65801531>
Comment 3 Tim Horton 2020-08-11 03:55:55 PDT
Created attachment 406373 [details]
Patch
Comment 4 Wenson Hsieh 2020-08-11 07:47:43 PDT
Comment on attachment 406373 [details]
Patch

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

> LayoutTests/fast/events/touch/ios/touch-event-stall-after-navigating-with-pending-asynchronous-touch-start.html:19
> +                await new Promise((resolve) => { testRunner.runUIScriptImmediately(`
> +                    uiController.doubleTapAtPoint(${x}, ${y}, .01, function() {
> +                        uiController.uiScriptComplete();
> +                    });`, resolve) });

Nit - the indentation looks a bit off here.
Comment 5 Simon Fraser (smfr) 2020-08-11 10:25:31 PDT
Comment on attachment 406373 [details]
Patch

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

> Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp:179
> +void EventDispatcher::takeQueuedTouchEventsForPage(const WebPage& webPage, TouchEventQueue& destinationQueue)

Why doesn't this just return a TouchEventQueue?
Comment 6 Devin Rousso 2020-08-11 10:48:23 PDT
Comment on attachment 406373 [details]
Patch

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

> LayoutTests/fast/events/touch/ios/touch-event-stall-after-navigating-with-pending-asynchronous-touch-start.html:16
> +                await new Promise((resolve) => { testRunner.runUIScriptImmediately(`

NIT: why is this `Promise` needed?
Comment 7 Tim Horton 2020-08-11 13:16:11 PDT
Comment on attachment 406373 [details]
Patch

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

>> Source/WebKit/WebProcess/WebPage/EventDispatcher.cpp:179
>> +void EventDispatcher::takeQueuedTouchEventsForPage(const WebPage& webPage, TouchEventQueue& destinationQueue)
> 
> Why doesn't this just return a TouchEventQueue?

πŸ€·β€β™‚οΈ probably predates `move`

Not going to fix it now, but it's a good point.

>> LayoutTests/fast/events/touch/ios/touch-event-stall-after-navigating-with-pending-asynchronous-touch-start.html:16
>> +                await new Promise((resolve) => { testRunner.runUIScriptImmediately(`
> 
> NIT: why is this `Promise` needed?

You are right! It's not (and I don't totally understand why the test works anymore, but it does).

>> LayoutTests/fast/events/touch/ios/touch-event-stall-after-navigating-with-pending-asynchronous-touch-start.html:19
>> +                    });`, resolve) });
> 
> Nit - the indentation looks a bit off here.

Devin said the same thing, but it looks right to meβ€½ I am confused :) Maybe getting rid of the promise will help.
Comment 8 Tim Horton 2020-08-11 13:17:45 PDT
Created attachment 406410 [details]
Patch
Comment 9 EWS 2020-08-11 14:04:15 PDT
Committed r265515: <https://trac.webkit.org/changeset/265515>

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