Bug 205458 - REGRESSION: [ iOS ] imported/w3c/web-platform-tests/dom/events/Event-dispatch-on-disabled-elements.html is failing
Summary: REGRESSION: [ iOS ] imported/w3c/web-platform-tests/dom/events/Event-dispatch...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-19 09:34 PST by Truitt Savell
Modified: 2020-01-27 12:36 PST (History)
8 users (show)

See Also:


Attachments
Patch (4.69 KB, patch)
2020-01-06 06:46 PST, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Truitt Savell 2019-12-19 09:34:23 PST
imported/w3c/web-platform-tests/dom/events/Event-dispatch-on-disabled-elements.html 

This test is failing constantly on iOS and has been very flakey. This was last modified in https://trac.webkit.org/changeset/253728/webkit
This is currently slowing down EWS

History:
https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fdom%2Fevents%2FEvent-dispatch-on-disabled-elements.html
Comment 1 Radar WebKit Bug Importer 2019-12-19 09:34:50 PST
<rdar://problem/58081704>
Comment 2 Antoine Quint 2019-12-20 08:45:08 PST
Right… so: it was failing constantly but I think this was addressed in r253746. This was based on results from EWS that somehow don't reproduce now, odd. I'll look into this shortly.
Comment 3 Antoine Quint 2020-01-01 08:47:50 PST
The failing test, "Real clicks on disabled elements must not dispatch events.", runs the same steps for five types of elements: "button", "fieldset", "input", "select" and "textarea", in that order.

If we run the steps for each element type individually, it passes. If we run "button", "fieldset", "input" and then either "select" or "textarea", it passes. However, if we try to run both of those, it fails.

Logging shows that we do not enter -[WKContentViewInteraction _singleTapRecognized:] after the "click" event to "select" has been delivered. My guess is that even though the "select" element has been removed, UIKit still delivers the immediate next tap to it and the WKContentView doesn't see it and fails to deliver the next tap to the Web content.
Comment 4 Antoine Quint 2020-01-01 08:57:53 PST
Adding this before continuing to the next element type fixes the issue:

    await new Promise(resolve => setTimeout(resolve, 1000));

We need to figure out why continuing the loop synchronously after the call to remove() fails.
Comment 5 Daniel Bates 2020-01-01 09:28:01 PST
Only considering what you wrote (I haven't looked at the test): my guess is that UIKit does **not** delivering the tap because the kit is animating a view and this is the default kit behavior. It's fixable.
Comment 6 Antoine Quint 2020-01-01 09:51:02 PST
So [_inputPeripheral beginEditing] is called in -[WKContentViewInteraction _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:] for the <select> element after we attempt to click on the <textarea> in the next loop iteration.
Comment 7 Wenson Hsieh 2020-01-01 19:58:23 PST
(In reply to Daniel Bates from comment #5)
> Only considering what you wrote (I haven't looked at the test): my guess is
> that UIKit does **not** delivering the tap because the kit is animating a
> view and this is the default kit behavior. It's fixable.

Dan's hunch is correct. WKScrollView is eating the subsequent touches after bringing up the input view, because it has active animations (i.e. the implementation of -[UIView hitTest:withEvent:]).

This happens because we're trying to zoom/scroll to reveal the focused element. To test this, I commented out the implementation of -[WKContentView _zoomToRevealFocusedElement], and saw that the flakiness did not reproduce.
Comment 8 Wenson Hsieh 2020-01-01 20:35:54 PST
A couple of potential ways to fix this:

1. Wait for animations on WKScrollView to finish before proceeding with a single tap in UIScriptControllerIOS::singleTapAtPointWithModifiers, like so:

+    NSDate *timeoutDate = [NSDate dateWithTimeIntervalSinceNow:someReasonableTimeoutDate];
+    while (webView().scrollView.layer.animationKeys.count) {
+        [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:timeoutDate];
+        if ([timeoutDate compare:NSDate.date] == NSOrderedAscending)
+            break;
+    }

or,

2. Force animations to finish before proceeding with a single tap in UIScriptControllerIOS::singleTapAtPointWithModifiers, like so:

+    [webView().scrollView _removeAllAnimations:NO];
Comment 9 Antoine Quint 2020-01-06 06:46:12 PST
Created attachment 386841 [details]
Patch
Comment 10 WebKit Commit Bot 2020-01-06 14:04:42 PST
Comment on attachment 386841 [details]
Patch

Clearing flags on attachment: 386841

Committed r254082: <https://trac.webkit.org/changeset/254082>
Comment 11 WebKit Commit Bot 2020-01-06 14:04:44 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 David Kilzer (:ddkilzer) 2020-01-27 12:36:27 PST
This fixed iPhone Simulator test failures, but not iPad Simulator test failures, which are tracked by:

Bug 206759: REGRESSION: [ iOS ] imported/w3c/web-platform-tests/dom/events/Event-dispatch-on-disabled-elements.html is flaky failing
<https://bugs.webkit.org/show_bug.cgi?id=206759>