Since WebKit2 clients only know about the WKView coordinate space, this rectangle is only useful if it's in that space.
<rdar://problem/10275376>
Created attachment 110747 [details] Patch
Comment on attachment 110747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110747&action=review Is it possible to write a test for this? > Source/WebKit2/ChangeLog:7 > + InjectedBundleHitTestResult::imageRect() should return rect in WKView coordinates > + https://bugs.webkit.org/show_bug.cgi?id=69963 > + > + Use the confusingly-named ScrollView::contentsToWindow() function to convert the image rect > + from frame to WKView coordinates. Can you give a little justification as to why WKView coordinates are better than whatever coordinates we were previously using?
In the immortal words of Adam Roben, "this should have an API test."
(In reply to comment #4) > In the immortal words of Adam Roben, "this should have an API test." Actually, my words were: (In reply to comment #3) > Is it possible to write a test for this?
Comment on attachment 110747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110747&action=review >> Source/WebKit2/ChangeLog:7 >> + from frame to WKView coordinates. > > Can you give a little justification as to why WKView coordinates are better than whatever coordinates we were previously using? The justification is in the comment in the code, but I'll elaborate in the ChangeLog.
Comment on attachment 110747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110747&action=review >>> Source/WebKit2/ChangeLog:7 >>> + from frame to WKView coordinates. >> >> Can you give a little justification as to why WKView coordinates are better than whatever coordinates we were previously using? > > The justification is in the comment in the code, but I'll elaborate in the ChangeLog. "ToWindow" really is window coords, not view coords. The coords will be flipped in WK1.
Comment on attachment 110747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110747&action=review >>>> Source/WebKit2/ChangeLog:7 >>>> + from frame to WKView coordinates. >>> >>> Can you give a little justification as to why WKView coordinates are better than whatever coordinates we were previously using? >> >> The justification is in the comment in the code, but I'll elaborate in the ChangeLog. > > "ToWindow" really is window coords, not view coords. The coords will be flipped in WK1. This is injected bundle code in WebKit2, so I don’t see how WebKit1 is relevant.
Comment on attachment 110747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110747&action=review >>>>> Source/WebKit2/ChangeLog:7 >>>>> + from frame to WKView coordinates. >>>> >>>> Can you give a little justification as to why WKView coordinates are better than whatever coordinates we were previously using? >>> >>> The justification is in the comment in the code, but I'll elaborate in the ChangeLog. >> >> "ToWindow" really is window coords, not view coords. The coords will be flipped in WK1. > > This is injected bundle code in WebKit2, so I don’t see how WebKit1 is relevant. WebKit1 isn't relevant, but the first part of Simon's comment is correct. After talking to Simon more, I'm working on a patch to add member functions to Widget and ScrollView to convert between frame view and root view coordinate systems, since there's no way to do this currently. InjectedBundleHitTestResult::imageRect() can use one of these functions, and I also need to write a WK2 API test (as Sam and Adam pointed out).
I wrote bug 70136 to cover writing the test for this, since I need to land this patch soon.
Created attachment 111062 [details] Patch
Comment on attachment 111062 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111062&action=review > Source/WebCore/platform/Widget.cpp:106 > +IntRect Widget::convertFromRootView(const IntRect& rootRect) const > +{ > + if (const ScrollView* parentScrollView = parent()) { > + IntRect parentRect = parentScrollView->convertFromRootView(rootRect); > + return convertFromContainingView(parentRect); > + } > + return rootRect; > +} > + > +IntRect Widget::convertToRootView(const IntRect& localRect) const > +{ > + if (const ScrollView* parentScrollView = parent()) { > + IntRect parentRect = convertToContainingView(localRect); > + return parentScrollView->convertToRootView(parentRect); > + } > + return localRect; > +} > + > +IntPoint Widget::convertFromRootView(const IntPoint& rootPoint) const > +{ > + if (const ScrollView* parentScrollView = parent()) { > + IntPoint parentPoint = parentScrollView->convertFromRootView(rootPoint); > + return convertFromContainingView(parentPoint); > + } > + return rootPoint; > +} > + > +IntPoint Widget::convertToRootView(const IntPoint& localPoint) const > +{ > + if (const ScrollView* parentScrollView = parent()) { > + IntPoint parentPoint = convertToContainingView(localPoint); > + return parentScrollView->convertToRootView(parentPoint); > + } > + return localPoint; > +} How about calling these from the existing toWindow/fromWindow calls?
Comment on attachment 111062 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111062&action=review >> Source/WebCore/platform/Widget.cpp:106 >> +} > > How about calling these from the existing toWindow/fromWindow calls? Good idea, but I'd rather not perturb existing code in this patch. I will file a followup bug to track this.
Committed r97514: <http://trac.webkit.org/changeset/97514>
I wrote bug 70152 to track Simon's suggestion.