Bug 93678

Created attachment 158318 [details] make it clonable

Created attachment 158683 [details] sync

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 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

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

Created attachment 159838 [details] Patch for landing

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 on attachment 159838 [details] Patch for landing Clearing flags on attachment: 159838 Committed r126256: <http://trac.webkit.org/changeset/126256>

All reviewed patches have been landed. Closing bug.