Bug 128924 - Shifted document touch handling in iframes on iOS
Summary: Shifted document touch handling in iframes on iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Event Handling (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad iOS 7.0
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-02-17 11:54 PST by Arty Gus
Modified: 2014-05-19 13:54 PDT (History)
5 users (show)

See Also:


Attachments
illustrates the problem (75.25 KB, image/png)
2014-02-17 11:54 PST, Arty Gus
no flags Details
iframe with touchevent on document (1.14 KB, text/html)
2014-02-17 11:56 PST, Arty Gus
no flags Details
uses iframe.html to show the problem (218 bytes, text/html)
2014-02-17 11:57 PST, Arty Gus
no flags Details
uses demo.html and iframe.html to show the workaround (354 bytes, text/html)
2014-02-17 11:59 PST, Arty Gus
no flags Details
Test case usable online (334 bytes, text/html)
2014-05-16 01:57 PDT, Benjamin Poulain
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arty Gus 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
Comment 1 Arty Gus 2014-02-17 11:56:23 PST
Created attachment 224407 [details]
iframe with touchevent on document
Comment 2 Arty Gus 2014-02-17 11:57:21 PST
Created attachment 224408 [details]
uses iframe.html to show the problem
Comment 3 Arty Gus 2014-02-17 11:59:32 PST
Created attachment 224409 [details]
uses demo.html and iframe.html to show the workaround
Comment 4 Piotrek Koszuliński (Reinmar) 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.
Comment 5 Radar WebKit Bug Importer 2014-05-12 09:28:12 PDT
<rdar://problem/16884784>
Comment 6 Benjamin Poulain 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.
Comment 7 Piotrek Koszuliński (Reinmar) 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.
Comment 8 Benjamin Poulain 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.
Comment 9 Piotrek Koszuliński (Reinmar) 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?
Comment 10 Benjamin Poulain 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.
Comment 11 Benjamin Poulain 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. :(
Comment 12 Arty Gus 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
Comment 13 Benjamin Poulain 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.
Comment 14 Benjamin Poulain 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.
Comment 15 Benjamin Poulain 2014-05-19 13:54:01 PDT
Closing the bug. This is a platform bug and will be handled in <rdar://problem/16884784>.