[webkit-reviews] review denied: [Bug 34915] Chromium: Need to be able to get the bounds of selection rectangle(s) : [Attachment 48700] Fixed the spurious tab chars in Changelog

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Dmitriy Belenko
<dbelenko at google.com>'s request for review:
Bug 34915: Chromium: Need to be able to get the bounds of selection
rectangle(s)
https://bugs.webkit.org/show_bug.cgi?id=34915

Attachment 48700: Fixed the spurious tab chars in Changelog
https://bugs.webkit.org/attachment.cgi?id=48700&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>

> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   This change will enable 30 layout pixel tests to pass. The change
> +	   simply exposes a helper function to return a bounding rectangle for
the
> +	   current (possibly transformed) selection rectangles. The displayed
> +	   rectangle is not transformed, but it does adjust itself to
completely
> +	   enclose the transformed selection. In WebKit Apple makes this test
work
> +	   by using dynamic dispatch in Objective C, but here I had to
introduce a
> +	   method.
> +
> +	   This change does not require new test cases.
> +
> +	   Chromium: Need to be able to get the bounds of selection
rectangle(s)
> +	   https://bugs.webkit.org/show_bug.cgi?id=34915

^^^ please move these two lines to the top, just below the Reviewed by line. 
That
way tools that just show a snippet of the ChangeLog will show those lines.


> +++ b/WebKit/chromium/public/WebFrame.h
> @@ -478,6 +478,12 @@ public:
>      // Returns a text representation of the render tree.  This method is
used
>      // to support layout tests.
>      virtual WebString renderTreeAsText() const = 0;
> +  
> +    // Returns bool true if there is a selection and false if there's none.
> +    // In case there is a selection, sets the rect to the "straight"
bounding 
> +    // rect of the (possibly transformed) selection rect.
> +    // This method is used to support layout tests.
> +    virtual bool selectionBoundsRect(WebRect&) const = 0;

This should just return WebRect.  If the WebRect is empty, then it can mean
that there was no selection.


> +bool WebFrameImpl::selectionBoundsRect(WebRect& boundsRect) const 
> +{
> +    if (hasSelection()) {
> +	   FloatRect r = frame()->selectionBounds(false);
> +	   boundsRect.x = r.x();
> +	   boundsRect.y = r.y();
> +	   boundsRect.width = r.width();
> +	   boundsRect.height = r.height();

Is the loss of precision from float to int okay?

-Darin