[Webkit-unassigned] [Bug 34915] Chromium: Need to be able to get the bounds of selection rectangle(s)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=34915


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #49029|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #8 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-02-19 11:27:31 PST ---
(From update of attachment 49029)
> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> index 4e8a629..0d90fd0 100644
> --- a/WebKit/chromium/ChangeLog
> +++ b/WebKit/chromium/ChangeLog
> @@ -1,3 +1,20 @@
> +2010-02-18  Dmitriy Belenko  <dbelenko at google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        This change will enable about 30 test cases to pass in Chromium.
> +        All of these test cases are related to selection rect boundaries.
> +        The three issues that this will solve can be found here:
> +
> +        http://code.google.com/p/chromium/issues/detail?id=28646
> +        http://code.google.com/p/chromium/issues/detail?id=23481
> +        http://code.google.com/p/chromium/issues/detail?id=23482

I'm sorry to nit-pick, but you should really just reference the WebKit
bug number in the WebKit ChangeLog.  (The bug can have references to these
Chromium bug reports.)


> +++ b/WebKit/chromium/public/WebFrame.h
> @@ -492,6 +492,11 @@ public:
>                                           float pageWidthInPixels,
>                                           float pageHeightInPixels) const = 0;
>  
> +    // Returns the bounds rect for current selection. If selection is performed
> +    // on transformed text, the rect will still bound the selection, but will
> +    // not be transformed itself. If no selection is present the rect will be
> +    // empty ((0,0), (0,0))
> +    virtual WebRect selectionBoundsRect() const = 0;
>  protected:

nit: please preserve the new line above the "protected:" line.


> +++ b/WebKit/chromium/src/WebFrameImpl.cpp
...
> +WebRect WebFrameImpl::selectionBoundsRect() const
> +{
> +    if (hasSelection()) {
> +        FloatRect fr = frame()->selectionBounds(false);
> +        return WebRect(roundf(fr.x()), roundf(fr.y()),
> +                       roundf(fr.width()), roundf(fr.height()));

Are you sure we want to round like this?  IntRect(const FloatRect&) just rounds
down.


> +    }
> +
> +    return WebRect(0, 0, 0, 0);

nit:  Just do "return WebRect();" since the default constructor does the same
thing.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.