Bug 211521 – REGRESSION (r253267): issues on touchstart/touchend/touchmove (pointerdown/pointerup/pointermove) events

• Component: UI Events
• Status: RESOLVED FIXED

[juwagn]

Hello,

on latest iOS update 13.4.1 with Safari the bug with fast touch sequences came back once it was fixed on 2019-11-14, but now exactly same bug is back again!
In the similar ticket number 203786 the bug link contains already full description.
That breaks behavior of our software totally compared as example to Android browsers since touchstart/touchend gets detected in sequence, and by fast sequence when it gets detected as dblclick then touchstart and touchend stay unfired!
That can't be replaced by dblclick since in this sequence i need to move finger after non-fired touchstart event but since on new Safari it gets not reported the functionality gets broken.
There is in fact no need to create extra reproducer case since issue is reproducible always, touch the screen fast two times in sequence and only one event gets fired instead expected two.
Is there any chance to return back this functionality? Or at least register specific new event name that would be fired regardless of dblclick detection where i could recognize that finger was touched/untouched or moved while being fast touched two times in sequence?
I just did not expect that same issue would come back after being reported and fixed already!

https://bugs.webkit.org/show_bug.cgi?id=203786
--------
The touch sequence: finger touch > finger release > finger touch > finger move > finger release, where last 3 event types get not reported to event handling. 
On earlier iOS versions like iOS12 the sequence above fired following events:
1. touchstart
2. touchend 
3. touchstart (in fast sequence to previous touchend)
4. touchmove
5. touchend (when moving ended)

On new iOS 13 versions on tested iPad only first two
1. touchstart
2. touchend 
3. ---- not fired> touchstart  (in fast sequence to previous touchend) 
4. ---- not fired> touchmove
5. ---- not fired> touchend 
In such case not event dblclick gets fired.

Unfortunatelly unreported "dbl-touch and move" breaks functionality of our software. Pointer events have same issue like touch events, they are not propagated to DOM if detected by iOS as potential dblclick even if moved between.
-----

[Antoine Quint]

This could well be a dupe of bug 211179.

[Radar WebKit Bug Importer]

<rdar://problem/62942374>

[Antoine Quint]

Hi juwagn. There is no test attached for either this bug nor bug 203786. It would help a great deal if you could provide us with one. Thanks!

[juwagn]

Created attachment 398666 [details]
reproducer requested

How to reproduce

Safari on iOS 13.4.1 open bug_reproducer.html

1. finger touch (touchstart fired)
2. finger remove (touchend fired)
3. - execute next steps immediately like by double click -
4. finger touch (NO touchstart fired)
5. finger MOVE (NO touchmove fired)
6. finger remove (NO touchend fired)
In this scenario 4-6. steps are not anymore fired.

Same way Android Chrome fires 4.-6. steps correctly, and even more double touch increases on Android Chrome amount of fired touchstart/touchend events always by 2 event sequences(2start and 2end) as expected. But iOS 13.4.1 Safari fires always only one event sequence (1start and 1end).
This is serious bug in detection of "double touch move" gesture in our application.

[juwagn]

Forgot to add, use "white" area for touching and use always one finger only!

In my description above "move event" won't be fired anyway, I didn't insert the extended handling for doing this, but that doesn't matter anyway, compared to Android Chrome the "6. finger remove (NO touchend fired)" will be fired by Android Chrome with noticeable change while Safari fires nothing here.
Expected would be firing of "touchstart" "touchend" "touchmove" events same way as on all common browsers like Android Chrome, Android FireFox etc.

I use "passive": false on addEventListener, so i don't care about scrolling acceleration since no page scrolling used, so "passive": false is wished behavior and should stay .preventDefault() preventable event.

[Antoine Quint]

Thanks for the test case! I managed to track down the regression to r253267, the fix for bug 204664. We already fixed a regression related to this bug in bug 211179, but this seems to be another aspect of the original regression that wasn't dealt with yet.

[Wenson Hsieh]

I’ve begun to investigate this.

One interesting thing I noticed is that the bug only seems to reproduce if the viewport is not responsive; adding something like:

<meta name="viewport" content="width=device-width, initial-scale=1">

seems to make the problem go away (at least, until the user manually pinches to zoom in).

This seems like bad interaction between the relatively new deferring gesture recognizers (WKDeferringGestureRecognizer) and the blocking double tap gesture recognizer (used when fast-clicking is disabled).

[Wenson Hsieh]

Created attachment 398999 [details]
Patch

[Darin Adler]

Comment on attachment 398999 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:719
> +- (NSArray<WKDeferringGestureRecognizer *> *)_deferringGestureRecognizers

I think this needs to go inside:

    #if ENABLE(IOS_TOUCH_EVENTS)

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:728
> +    NSMutableArray *deferringGestureRecognizers = [NSMutableArray arrayWithCapacity:3];
> +    if (_deferringGestureRecognizerForImmediatelyResettableGestures)
> +        [deferringGestureRecognizers addObject:_deferringGestureRecognizerForImmediatelyResettableGestures.get()];
> +    if (_deferringGestureRecognizerForDelayedResettableGestures)
> +        [deferringGestureRecognizers addObject:_deferringGestureRecognizerForDelayedResettableGestures.get()];
> +    if (_deferringGestureRecognizerForSyntheticTapGestures)
> +        [deferringGestureRecognizers addObject:_deferringGestureRecognizerForSyntheticTapGestures.get()];
> +    return deferringGestureRecognizers;

There are more efficient ways to do this than using an NSMutableArray. The simplest I can think of is to use a C array. Something like this:

    WKDeferringGestureRecognizer *recognizers[3];
    NSUInteger count = 0;
    auto add = [&] (const RetainPtr<WKDeferringGestureRecognizer>& recognizer) {
        if (recognizer)
            recognizers[count++] = recognizer.get();
    };
    add(_deferringGestureRecognizerForImmediatelyResettableGestures);
    add(_deferringGestureRecognizerForDelayedResettableGestures);
    add(_deferringGestureRecognizerForSyntheticTapGestures);
    return [[NSArray arrayWithObjects:recognizers count:count];

That’s more lines of code, but it always seems better to me not to use a mutable array.

[Wenson Hsieh]

Comment on attachment 398999 [details]
Patch

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

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:719
>> +- (NSArray<WKDeferringGestureRecognizer *> *)_deferringGestureRecognizers
> 
> I think this needs to go inside:
> 
>     #if ENABLE(IOS_TOUCH_EVENTS)

Oops! Yes, that’s correct.

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:728
>> +    return deferringGestureRecognizers;
> 
> There are more efficient ways to do this than using an NSMutableArray. The simplest I can think of is to use a C array. Something like this:
> 
>     WKDeferringGestureRecognizer *recognizers[3];
>     NSUInteger count = 0;
>     auto add = [&] (const RetainPtr<WKDeferringGestureRecognizer>& recognizer) {
>         if (recognizer)
>             recognizers[count++] = recognizer.get();
>     };
>     add(_deferringGestureRecognizerForImmediatelyResettableGestures);
>     add(_deferringGestureRecognizerForDelayedResettableGestures);
>     add(_deferringGestureRecognizerForSyntheticTapGestures);
>     return [[NSArray arrayWithObjects:recognizers count:count];
> 
> That’s more lines of code, but it always seems better to me not to use a mutable array.

I’ll give this a shot — thanks!

[Wenson Hsieh]

Created attachment 399022 [details]
For EWS

[EWS]

Committed r261480: <https://trac.webkit.org/changeset/261480>

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