Bug 147836 - A focused node should not be assisted when handling touch events synchronously
Summary: A focused node should not be assisted when handling touch events synchronously
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 147946
Blocks:
  Show dependency treegraph
 
Reported: 2015-08-10 12:35 PDT by Wenson Hsieh
Modified: 2020-03-10 13:20 PDT (History)
7 users (show)

See Also:


Attachments
Try to tap, scroll, or zoom in the pink square, and the keyboard will deploy when it shouldn't (802 bytes, text/html)
2015-08-10 12:35 PDT, Wenson Hsieh
no flags Details
Patch (2.77 KB, patch)
2015-08-10 13:32 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (10.03 KB, patch)
2015-08-13 12:33 PDT, Wenson Hsieh
enrica: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2015-08-10 12:35:23 PDT
Created attachment 258637 [details]
Try to tap, scroll, or zoom in the pink square, and the keyboard will deploy when it shouldn't

Currently, when a node is focused programmatically (either through element.focus() or the autofocus attribute) and we interact with a region that has touch event handlers (ontouchstart, etc.) the focused node will be refocused and assisted.
Comment 1 Wenson Hsieh 2015-08-10 12:36:50 PDT
<rdar://problem/22204108>
Comment 2 Wenson Hsieh 2015-08-10 13:32:48 PDT
Created attachment 258646 [details]
Patch
Comment 3 Wenson Hsieh 2015-08-12 13:24:49 PDT
Comment on attachment 258646 [details]
Patch

This approach is insufficient. More specifically, it will not handle the case where the developer calls preventDefault() on a touch event but we still want to show the keyboard.
Comment 4 Wenson Hsieh 2015-08-13 12:33:10 PDT
Created attachment 258915 [details]
Patch
Comment 5 Enrica Casucci 2015-08-13 13:15:57 PDT
Comment on attachment 258915 [details]
Patch

It would be nice to add some manual tests to this patch. It is not possible to have the automatic tests but at least having the manual ones would be helpful. Make sure you test the most popular frameworks that use touch handlers to make sure none of them relies on the wrong behavior we had before.
Comment 6 Wenson Hsieh 2015-08-13 13:49:07 PDT
Got it. I'll try out Sencha touch and fastclick (the two libraries Ben mentioned yesterday) and make sure they still work.

Simon also looked at this patch and advised me to leave out the manual tests, since they are ignored most of the time. However, I think it will be nice to still include them, since I'll be working on an automated framework for testing forms on iOS soon, and (at some point) I'll probably go through the manual tests directory and convert as many as I can to automated tests (including this one). It'll be helpful to have them all in one place.

In any case, the test case still lives in the Radar, so we can always go back and consult it if necessary!
Comment 7 Wenson Hsieh 2015-08-13 14:49:11 PDT
Committed r188405: <http://trac.webkit.org/changeset/188405>