Bug 193182 - [iOS] fast/events/touch/ios/hover-when-style-change-is-async.html times out
Summary: [iOS] fast/events/touch/ios/hover-when-style-change-is-async.html times out
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-06 16:51 PST by Wenson Hsieh
Modified: 2019-01-23 12:10 PST (History)
10 users (show)

See Also:


Attachments
Patch (1.97 KB, patch)
2019-01-17 10:15 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (2.98 KB, patch)
2019-01-22 11:09 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Rebase on trunk (3.43 KB, patch)
2019-01-22 23:29 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2019-01-06 16:51:13 PST
This test times out nearly 100% of the time on our internal iOS bots. This test is not run in any non-internal configurations.
Comment 1 Wenson Hsieh 2019-01-17 09:40:48 PST
(In reply to Wenson Hsieh from comment #0)
> This test times out nearly 100% of the time on our internal iOS bots. This
> test is not run in any non-internal configurations.

I can't reproduce the timeouts when running the test on repeat, but I can reproduce this timeout 100% of the time by running fast/events/touch/ios/drag-to-autoscroll-in-single-line-editable.html and then fast/events/touch/ios/long-press-then-drag-to-select-text.html.

Additionally, doing `await liftUpAtPoint(dragX + 5, tapPointY);` after the last `continueTouchAndDragFromPointToPoint` fixes this timeout for me. It looks like the simulated touch down from a previous test is bleeding into this one. Perhaps we should make the test runner smart enough to send a touch up and/or force gesture recognizers to reset when resetting the test controller between tests...
Comment 2 Wenson Hsieh 2019-01-17 10:15:03 PST
Created attachment 359387 [details]
Patch
Comment 3 Wenson Hsieh 2019-01-17 14:24:23 PST
Some additional observations:

- There appear to be three layout tests that pass before my patch (which just calls -_cancelAllTouches when resetting state), and start failing after the patch...
  • long-press-on-editable-content-then-drag-left-to-change-selected-text.html
  • long-press-on-editable-content-then-drag-right-to-change-selected-text.html
  • long-press-on-editable-content-then-drag-up-to-change-selected-text.html

- If I just skip hover-when-style-change-is-async.html and also don't call -_cancelAllTouches, these 3 tests start failing.
- If I just lift up to end the touch in drag-to-autoscroll-in-single-line-editable.html by calling liftUpAtPoint, don't skip hover-when-style-change-is-async.html, and also don't call -_cancelAllTouches, these 3 tests also start failing.
- Run individually, all of these 3 tests pass.

...need to dig a bit deeper to understand why this happens.
Comment 4 Wenson Hsieh 2019-01-22 11:02:03 PST
(In reply to Wenson Hsieh from comment #3)
> Some additional observations:
> 
> - There appear to be three layout tests that pass before my patch (which
> just calls -_cancelAllTouches when resetting state), and start failing after
> the patch...
>   •
> long-press-on-editable-content-then-drag-left-to-change-selected-text.html
>   •
> long-press-on-editable-content-then-drag-right-to-change-selected-text.html
>   • long-press-on-editable-content-then-drag-up-to-change-selected-text.html
> 
> - If I just skip hover-when-style-change-is-async.html and also don't call
> -_cancelAllTouches, these 3 tests start failing.
> - If I just lift up to end the touch in
> drag-to-autoscroll-in-single-line-editable.html by calling liftUpAtPoint,
> don't skip hover-when-style-change-is-async.html, and also don't call
> -_cancelAllTouches, these 3 tests also start failing.
> - Run individually, all of these 3 tests pass.
> 
> ...need to dig a bit deeper to understand why this happens.

Looks like this actually exposed a real bug!

If -_beginSuppressingSelectionAssistantForReason: is called due to the focused element being empty, then we'll fail to unset this bit when blurring the focused element (e.g. upon mainframe navigation), which causes subsequent selection gestures in an editable element (without bringing focus to the editable element) to fail. Fixing this bug fixes all three of these cases.

It's a good thing these tests exist :)
Comment 5 Radar WebKit Bug Importer 2019-01-22 11:04:45 PST
<rdar://problem/47452154>
Comment 6 Wenson Hsieh 2019-01-22 11:09:45 PST
Created attachment 359753 [details]
Patch
Comment 7 Wenson Hsieh 2019-01-22 23:29:27 PST
Created attachment 359859 [details]
Rebase on trunk
Comment 8 Wenson Hsieh 2019-01-23 11:43:38 PST
Comment on attachment 359859 [details]
Rebase on trunk

Thanks for the review!
Comment 9 WebKit Commit Bot 2019-01-23 12:10:00 PST
Comment on attachment 359859 [details]
Rebase on trunk

Clearing flags on attachment: 359859

Committed r240352: <https://trac.webkit.org/changeset/240352>
Comment 10 WebKit Commit Bot 2019-01-23 12:10:02 PST
All reviewed patches have been landed.  Closing bug.