| Summary: | [iOS] fast/events/touch/ios/hover-when-style-change-is-async.html times out | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||
| Component: | HTML Editing | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ap, bdakin, commit-queue, jbedard, lforschler, megan_gardner, thorton, webkit-bug-importer, wenson_hsieh, zalan | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Wenson Hsieh
2019-01-06 16:51:13 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... Created attachment 359387 [details]
Patch
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. (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 :) Created attachment 359753 [details]
Patch
Created attachment 359859 [details]
Rebase on trunk
Comment on attachment 359859 [details]
Rebase on trunk
Thanks for the review!
Comment on attachment 359859 [details] Rebase on trunk Clearing flags on attachment: 359859 Committed r240352: <https://trac.webkit.org/changeset/240352> All reviewed patches have been landed. Closing bug. |