Bug 128924

Summary: Shifted document touch handling in iframes on iOS
Product: WebKit Reporter: Arty Gus <hero>
Component: UI EventsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, dbates, pkoszulinski, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: iPhone / iPad   
OS: iOS 7.0   
Attachments:
Description Flags
illustrates the problem
none
iframe with touchevent on document
none
uses iframe.html to show the problem
none
uses demo.html and iframe.html to show the workaround
none
Test case usable online none

Arty Gus
Reported 2014-02-17 11:54:41 PST
Created attachment 224406 [details] illustrates the problem Touch events bound to iframe document node is not handled correctly, if topmost window iframe parents has no touch events bound. How-to reproduce: - Put the iframe into div with margin-left 150px - Bind touchend to document inside the iframe - Iframe area which is 150px from the right border will not trigger touch events Workarounds: - Bind touch event to any iframe parent node in the topmost window (even iframe element itself) - Listen to body touch instead of the document node
Attachments
illustrates the problem (75.25 KB, image/png)
2014-02-17 11:54 PST, Arty Gus
no flags
iframe with touchevent on document (1.14 KB, text/html)
2014-02-17 11:56 PST, Arty Gus
no flags
uses iframe.html to show the problem (218 bytes, text/html)
2014-02-17 11:57 PST, Arty Gus
no flags
uses demo.html and iframe.html to show the workaround (354 bytes, text/html)
2014-02-17 11:59 PST, Arty Gus
no flags
Test case usable online (334 bytes, text/html)
2014-05-16 01:57 PDT, Benjamin Poulain
no flags
Arty Gus
Comment 1 2014-02-17 11:56:23 PST
Created attachment 224407 [details] iframe with touchevent on document
Arty Gus
Comment 2 2014-02-17 11:57:21 PST
Created attachment 224408 [details] uses iframe.html to show the problem
Arty Gus
Comment 3 2014-02-17 11:59:32 PST
Created attachment 224409 [details] uses demo.html and iframe.html to show the workaround
Piotrek Koszuliński (Reinmar)
Comment 4 2014-05-12 04:30:42 PDT
This issue affects also CKEditor and has been reported in http://dev.ckeditor.com/ticket/10714. I can also reproduce it on iOS 5.1.
Radar WebKit Bug Importer
Comment 5 2014-05-12 09:28:12 PDT
Benjamin Poulain
Comment 6 2014-05-14 16:40:58 PDT
I am very confused by this bug report. If I understand correctly, touch events in the iframe are not received on every part of the document. If that is the case, I suspect it is a problem with touch region. Where I am confused is the patch http://dev.ckeditor.com/attachment/ticket/10714/10714.patch From the comments in the patch, it seems that blur events are sent several times. In any case, *NEVER* set a touch handler on the entire main frame, you are gonna destroy runtime performance.
Piotrek Koszuliński (Reinmar)
Comment 7 2014-05-14 23:47:15 PDT
I think that the patch for #10714 is confusing because it solves more than one issue. It was proposed by Arty Gus and since I don't have the possibility to test it properly on iPad and understand what's a bug in Webkit and what's in CKEditor I didn't want to split it into separate patches. I think that Arty Gus will be able to tell more about bugs he found. For example - the changes in floatpanel blocking blur while touching - are they related to any Webkit bug or was it a correct behaviour of the engine which was incorrectly handled by CKEditor? > In any case, *NEVER* set a touch handler on the entire main frame, you are gonna destroy runtime performance. As for the touch handler, thanks for a tip. However, CKEditor cannot prevent others from adding it so we wanted to handle a case when someone reasonably or not added it.
Benjamin Poulain
Comment 8 2014-05-15 14:03:26 PDT
(In reply to comment #7) > For example - the changes in floatpanel blocking blur while touching - are they related to any Webkit bug or was it a correct behaviour of the engine which was incorrectly handled by CKEditor? When there is a touch handler, touch events can make elements active. It might just be that. If you have a test case, I can have a look. > > In any case, *NEVER* set a touch handler on the entire main frame, you are gonna destroy runtime performance. > > As for the touch handler, thanks for a tip. However, CKEditor cannot prevent others from adding it so we wanted to handle a case when someone reasonably or not added it. Maybe I misunderstand what CKEditor is doing. Are you adding a touch event handler on the root document even if there was none on the page? If you do, that is really really bad.
Piotrek Koszuliński (Reinmar)
Comment 9 2014-05-15 14:20:41 PDT
> Are you adding a touch event handler on the root document even if there was none on the page? If you do, that is really really bad. Oups! I totally forgot that Arty Gus' patch has introduced touch listeners. I thought that you're mentioning the case described in CKEditor's ticket. My fault. So, yes. Indeed we add now a touchend listener, but on the document inside the iframe, not the main frame. Should we avoid this too?
Benjamin Poulain
Comment 10 2014-05-15 17:34:20 PDT
(In reply to comment #9) > > Are you adding a touch event handler on the root document even if there was none on the page? If you do, that is really really bad. > > Oups! I totally forgot that Arty Gus' patch has introduced touch listeners. I thought that you're mentioning the case described in CKEditor's ticket. My fault. > > So, yes. Indeed we add now a touchend listener, but on the document inside the iframe, not the main frame. Should we avoid this too? That's ok. It is not ideal to have the whole surface respond to touch event, but if it is limited to an iframe, that's not too bad. The problem with touch event handlers is they are forcing the engine to block scrolling/pinching until the touch events are handled. When the page is not specifically designed for it, the interactions with the page become very laggy.
Benjamin Poulain
Comment 11 2014-05-16 01:57:34 PDT
Created attachment 231563 [details] Test case usable online I can reproduce in the sim. It looks like the touch region is broken for iframes. :(
Arty Gus
Comment 12 2014-05-17 02:48:26 PDT
(In reply to comment #8) > When there is a touch handler, touch events can make elements active. It might just be that. > > If you have a test case, I can have a look. > I crafted the sample with blurs http://bin.engineeriam.net/ios-blur.html
Benjamin Poulain
Comment 13 2014-05-17 16:02:19 PDT
(In reply to comment #12) > (In reply to comment #8) > > > When there is a touch handler, touch events can make elements active. It might just be that. > > > > If you have a test case, I can have a look. > > I crafted the sample with blurs http://bin.engineeriam.net/ios-blur.html Thanks a lot! That looks indeed like a bad bug. Can you please file a separate bug report and attach your example to the bug? Please CC me on that new bug or give the link here. For the original touch region bug, I know exactly what is going on now: when computing the touch region of a document, the rect is never transformed to the root view's coordinates. As a result, the touch region of the iframe is not at the right places, it starts at (0, 0). The bug is iOS specific, that is not in WebKit's code. I'll try to fix that next week.
Benjamin Poulain
Comment 14 2014-05-17 16:04:14 PDT
I should add: attaching the touch handler to the <body> element instead of the document is a valid workaround for the bug. Any element other than the document is transformed correctly to the root view's coordinates system.
Benjamin Poulain
Comment 15 2014-05-19 13:54:01 PDT
Closing the bug. This is a platform bug and will be handled in <rdar://problem/16884784>.
Note You need to log in before you can comment on or make changes to this bug.