Bug 185001

Summary: Click events on outer page are not being dispatched correctly after touch-zooming within an iframe
Product: WebKit Reporter: Walter Kalata <wkalata>
Component: DOMAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, cdumez, commit-queue, jer.noble, megan_gardner, prlaodude, sasha.barab, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh, zach
Priority: P2 Keywords: InRadar
Version: Safari 11   
Hardware: iPhone / iPad   
OS: iOS 11   
See Also: https://bugs.webkit.org/show_bug.cgi?id=182252
Attachments:
Description Flags
Iframe Zoom Click Repro
none
Simpler repro, removes unnecessary CSS; still matches the youtube video sent in an earlier comment
none
Workaround -- an empty "ontouchstart" attribute allows for the separate click event to be dispatched. The reported issue can not be reproduced using this attachment.
none
Needs a test
none
Patch
none
Fix open source build none

Description Walter Kalata 2018-04-25 14:46:30 PDT
Created attachment 338799 [details]
Iframe Zoom Click Repro

The attached file has an anchor labelled "Click Me" containing an onclick attribute of "alert('Clicked');", as well as an iframe sourced to a youtube video.

Repro:

1.  Focus the video in the iframe
2.  Zoom In
3.  Zoom Out
4.  Click "Click Me" on the parent page

Expectation: "alert('Clicked');" is executed and an alert box displays

Observed:  The click event only seems to dispatch (and the alert box display) a fraction of the time.


Anecdotally, although the link should be at position 0,0 during all of the steps above, document.elementFromPoint(0, 0).click() also fails to result in a dispatched click event to the anchor element during the zoom operation as well.
Comment 1 Simon Fraser (smfr) 2018-05-25 16:18:54 PDT
What do you mean by "focus the video"? Should I tap the play button to start it playing?
Comment 2 Simon Fraser (smfr) 2018-05-25 16:22:34 PDT
Does the bug occur if the "Click me" is not inside position:fixed?
Comment 3 Radar WebKit Bug Importer 2018-05-25 16:25:07 PDT
<rdar://problem/40569615>
Comment 4 Simon Fraser (smfr) 2018-05-25 16:25:36 PDT
Can you put a "click me" outside the position:fixed for testing, and move it away from the edge of the page so that it's easier to test? Does the size of the hit target make any difference?
Comment 5 Zach 2018-05-25 16:50:08 PDT
I have shown the repro in a video here: https://www.youtube.com/watch?v=mqf8vJiu-yY - in the video I have taken the link out of the position: fixed container as well and it persists.
Comment 6 Sasha 2018-06-20 21:57:23 PDT
has there been any progress on this issue? It would really mean a lot. Thanks much.
Comment 7 Zach 2018-06-25 13:17:54 PDT
A temporary workaround we've found is adding a touchstart event handler to the body. After that, the click events work properly and all is good again.
Comment 8 Walter Kalata 2018-06-25 13:24:27 PDT
Created attachment 343530 [details]
Simpler repro, removes unnecessary CSS; still matches the youtube video sent in an earlier comment
Comment 9 Walter Kalata 2018-06-25 13:26:09 PDT
Created attachment 343531 [details]
Workaround -- an empty "ontouchstart" attribute allows for the separate click event to be dispatched.  The reported issue can not be reproduced using this attachment.
Comment 10 Lucas Forschler 2019-02-06 09:19:06 PST
Mass move bugs into the DOM component.
Comment 11 prlaodude 2019-03-23 18:33:08 PDT
This is a major issue as my website consists of a feed of embedded youtube videos. UI is rendered useless on iOS. Adding ontouchstart="" to the body works in the beginning, but as the user keeps scrolling the issue presents itself again.
Comment 12 Simon Fraser (smfr) 2019-06-01 20:02:08 PDT
I can reproduce.
Comment 13 Simon Fraser (smfr) 2019-06-01 21:25:06 PDT
The issue is that if you happen to put a finger in the YouTube iframe when zooming, WebPage::updatePotentialTapSecurityOrigin() puts youtube.com's security origin in WebPage::m_potentialTapSecurityOrigin and it doesn't get cleared, causing later potential taps in the main frame to fail.
Comment 14 Andy Estes 2019-06-04 00:58:06 PDT
We assume that any time a touch event is a potential tap, our single tap gesture recognizer will identify the potential tap (even if it never commits). That's a false assumption. In this case, the first touch event in the pinch gesture is a potential tap, but the second touch event occurs before the single tap gesture recognizer identifies a potential tap.
Comment 15 Andy Estes 2019-06-04 01:04:32 PDT
That false assumption causes this bug because we set m_potentialTapSecurityOrigin when a touch event is a potential tap, but never clear it until a potential tap in the WebPage sense either commits or cancels.

I think we can fix this by clearing m_potentialTapSecurityOrigin each time we see a new touch start event.
Comment 16 Andy Estes 2019-06-04 01:05:07 PDT
Created attachment 371254 [details]
Needs a test
Comment 17 Wenson Hsieh 2019-07-03 22:49:36 PDT
Created attachment 373454 [details]
Patch
Comment 18 Wenson Hsieh 2019-07-03 23:06:51 PDT
Created attachment 373455 [details]
Fix open source build
Comment 19 Wenson Hsieh 2019-07-05 09:24:41 PDT
Comment on attachment 373455 [details]
Fix open source build

Thanks for the review!
Comment 20 WebKit Commit Bot 2019-07-05 09:58:12 PDT
Comment on attachment 373455 [details]
Fix open source build

Clearing flags on attachment: 373455

Committed r247161: <https://trac.webkit.org/changeset/247161>
Comment 21 WebKit Commit Bot 2019-07-05 09:58:13 PDT
All reviewed patches have been landed.  Closing bug.