Bug 93678 - Make an event object clonable to support an event propagation across seamless iframes.
Summary: Make an event object clonable to support an event propagation across seamless...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks: 91290
  Show dependency treegraph
 
Reported: 2012-08-09 19:11 PDT by Hayato Ito
Modified: 2012-08-21 20:11 PDT (History)
7 users (show)

See Also:


Attachments
make it clonable (5.18 KB, patch)
2012-08-14 07:01 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff
sync (5.24 KB, patch)
2012-08-15 18:36 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff
Patch for landing (5.29 KB, patch)
2012-08-21 18:42 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2012-08-09 19:11:54 PDT
To support a event propagation of seamless iframes, we have to create new Event object based on the original Event object for each browsing context so that we can isolate each event object.
Comment 1 Hayato Ito 2012-08-14 07:01:03 PDT
Created attachment 158318 [details]
make it clonable
Comment 2 Hayato Ito 2012-08-15 18:36:19 PDT
Created attachment 158683 [details]
sync
Comment 3 Dimitri Glazkov (Google) 2012-08-21 12:57:40 PDT
Comment on attachment 158683 [details]
sync

View in context: https://bugs.webkit.org/attachment.cgi?id=158683&action=review

> Source/WebCore/dom/MouseEvent.cpp:161
> +PassRefPtr<Event> MouseEvent::cloneFor(HTMLIFrameElement* iframe) const

Can we just pass Frame* here? Not sure the fact that it's coming from HTMLIFrameElement is important here.
Comment 4 Ojan Vafai 2012-08-21 14:04:35 PDT
Comment on attachment 158683 [details]
sync

View in context: https://bugs.webkit.org/attachment.cgi?id=158683&action=review

I'm fine with just putting FIXMEs for now for the positioning comments and fixing this when you actually do the retargeting patch since you can actually test it that way. Up to you, either way is fine.

> Source/WebCore/dom/MouseEvent.cpp:151
> +inline static int adjustedClinetX(int innerClientX, HTMLIFrameElement* iframe, FrameView* frameView)

Typo here and below s/Clinet/Client.

> Source/WebCore/dom/MouseEvent.cpp:153
> +    return iframe->offsetLeft() - frameView->scrollX() + innerClientX;

This gives you the border x position. I think you need to subtract the borderLeft and paddingLeft here. Also, the offset* methods give you a number relative to your offsetParent. I believe client* on the Event give you viewport relative locations.

So, here's a testcase that covers all these things:
<div style="position:relative">
    <iframe seamless style="position:absolute; border: 10px solid; padding: 5px"></iframe>
</div>

offsetLeft/offsetTop on the iframe will be 0. Instead off offsetLeft/Top you should just use x() and y() on the iframe. I'm not sure off the top of my head whether x()/y() are the border positions or the content positions. I think it's the border position, so you'll still need to remove the border/padding.

> Source/WebCore/dom/MouseEvent.cpp:158
> +inline static int adjustedClinetY(int innerClientY, HTMLIFrameElement* iframe, FrameView* frameView)
> +{
> +    return iframe->offsetTop() - frameView->scrollY() + innerClientY;

ditto
Comment 5 Hayato Ito 2012-08-21 17:43:46 PDT
Thank you for the reviews.

(In reply to comment #3)
> (From update of attachment 158683 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=158683&action=review
> 
> > Source/WebCore/dom/MouseEvent.cpp:161
> > +PassRefPtr<Event> MouseEvent::cloneFor(HTMLIFrameElement* iframe) const
> 
> Can we just pass Frame* here? Not sure the fact that it's coming from HTMLIFrameElement is important here.

We need a HTMLIFrameElement to adjust a (x, y) coordinate for MouseEvent.

(In reply to comment #4)
> (From update of attachment 158683 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=158683&action=review
> 
> I'm fine with just putting FIXMEs for now for the positioning comments and fixing this when you actually do the retargeting patch since you can actually test it that way. Up to you, either way is fine.

Thank you. I'd like to putting FIXME for now. It's not easy task for me to calculate new x and y exactly.
Let me fix it when I can test it using actual LayoutTest in a later patch. 

> 
> > Source/WebCore/dom/MouseEvent.cpp:151
> > +inline static int adjustedClinetX(int innerClientX, HTMLIFrameElement* iframe, FrameView* frameView)
> 
> Typo here and below s/Clinet/Client.

Ops. I'll fix it.

> 
> > Source/WebCore/dom/MouseEvent.cpp:153
> > +    return iframe->offsetLeft() - frameView->scrollX() + innerClientX;
> 
> This gives you the border x position. I think you need to subtract the borderLeft and paddingLeft here. Also, the offset* methods give you a number relative to your offsetParent. I believe client* on the Event give you viewport relative locations.
> 
> So, here's a testcase that covers all these things:
> <div style="position:relative">
>     <iframe seamless style="position:absolute; border: 10px solid; padding: 5px"></iframe>
> </div>
> 
> offsetLeft/offsetTop on the iframe will be 0. Instead off offsetLeft/Top you should just use x() and y() on the iframe. I'm not sure off the top of my head whether x()/y() are the border positions or the content positions. I think it's the border position, so you'll still need to remove the border/padding.

Thank you. I'll fix it later. I need actual layout tests to make sure that we can adjust it correctly.
I might ask further in another bug entry I am going to file in regard to this.

> 
> > Source/WebCore/dom/MouseEvent.cpp:158
> > +inline static int adjustedClinetY(int innerClientY, HTMLIFrameElement* iframe, FrameView* frameView)
> > +{
> > +    return iframe->offsetTop() - frameView->scrollY() + innerClientY;
> 
> ditto
Comment 6 Hayato Ito 2012-08-21 18:42:16 PDT
Created attachment 159838 [details]
Patch for landing
Comment 7 Hayato Ito 2012-08-21 18:46:09 PDT
Let me use this bug to fix positioning.
https://bugs.webkit.org/show_bug.cgi?id=93696

(In reply to comment #6)
> Created an attachment (id=159838) [details]
> Patch for landing
Comment 8 WebKit Review Bot 2012-08-21 20:11:31 PDT
Comment on attachment 159838 [details]
Patch for landing

Clearing flags on attachment: 159838

Committed r126256: <http://trac.webkit.org/changeset/126256>
Comment 9 WebKit Review Bot 2012-08-21 20:11:34 PDT
All reviewed patches have been landed.  Closing bug.