[Chromium] WebWidget::selectionBounds should return the bounds in document space
Created attachment 185583 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Does this fix a specific problem?
Comment on attachment 185583 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=185583&action=review > Source/WebKit/chromium/public/WebWidget.h:204 > + virtual bool documentSelectionBounds(WebRect& anchor, WebRect& focus) const { return false; } nit: WebWidget as an interface generally isn't supposed to know about DOM-related concepts. this method name seems to suggest that coordinates are returning in document space instead of viewport space? such a method probably belongs on WebView instead.
(In reply to comment #3) > Does this fix a specific problem? So here's the issue: when page scale is applied in the compositor, there are essentially 2 different window spaces, the scaled one and the unscaled one. WebKit deals with the unscaled and then when points or rects are passed between WebKit and the renderer, if they are passed in window space, they need to be scaled or unscaled. It is unclear (at least to me) whose responsibility this is (i.e. at what boundary should it be done). Currently, it looks like we do this in WebKit (look for applyPageScaleFactorInCompositor in WebFrameImpl.cpp or WebViewImpl.cpp). I don't like this because it becomes confusing when a public API is called within WebKit. For example, WebWidget::selectionBounds() returns two rects. If WebKit applies the pagescale, then when selectionBounds is used in WebKit (for example WebViewImpl::computeSCaleAndScrollForFocusedNode) you have to know that the rects are not actually in WebKit's viewport space, they are in this other scaled space. And so now, we have this layer that needs to know all about an extra space. My thought was that document space could be this common language that could be used across the boundary into and out of WebKit, so that neither side needed to know about the other's viewport space (and thus never had to distinguish between points in unscaled vs scaled viewport space). I guess I can just switch to what we've already done in other places and make selectionBounds instead scale these rects when necessary (and its users in WebKit can be updated to know that the rects are scaled).
+cc aelias for the scaling questions
Created attachment 185849 [details] Patch
(In reply to comment #7) > Created an attachment (id=185849) [details] > Patch This second patch is following our current approach of having WebKit scale/unscale points when they are returned from/passed to the public API. This works for me and fixes the immediate problem, but I think longer-term we may want to reconsider how we are handling this issue. What do you think?
LGTM. In the long run the correct solution is to push up all coordinates out of WebKit in absolute coordinates. But that will be a more involved multi-sided fix and this quick fix doesn't make it harder to do that later.
Comment on attachment 185849 [details] Patch Clearing flags on attachment: 185849 Committed r141657: <http://trac.webkit.org/changeset/141657>
All reviewed patches have been landed. Closing bug.