Bug 215368 – iOS: Scrolling and touch events sporadically stop working after navigating

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

iOS: Scrolling and touch events sporadically stop working after navigating

[Tim Horton]

Created attachment 406372 [details]
Patch

[Tim Horton]

<rdar://problem/65801531>

[Tim Horton]

Created attachment 406373 [details]
Patch

[Wenson Hsieh]

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.

[Simon Fraser (smfr)]

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?

[Devin Rousso]

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?

[Tim Horton]

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.

[Tim Horton]

Created attachment 406410 [details]
Patch

[EWS]

Committed r265515: <https://trac.webkit.org/changeset/265515>

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