[webkit-reviews] review denied: [Bug 34915] Chromium: Need to be able to get the bounds of selection rectangle(s) : [Attachment 49185] Using implicit conversion per Darin's request.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 22 17:31:39 PST 2010


David Levin <levin 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 49185: Using implicit conversion per Darin's request.
https://bugs.webkit.org/attachment.cgi?id=49185&action=review

------- Additional Comments from David Levin <levin at chromium.org>
Some trivial nits to fix and then I'd be willing to r+, cq+ (based on this plus
past reviews).

> diff --git a/WebKit/chromium/ChangeLog b/WebKit/chromium/ChangeLog
> @@ -1,3 +1,21 @@
> +2010-02-21  Dmitriy Belenko	<dbelenko at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
Typically one does: 

Bug title
Bug link

Description.

Like this:
> +	   Chromium: Need to be able to get the bounds of selection
rectangle(s) 
> +	   https://bugs.webkit.org/show_bug.cgi?id=34915
> +
> +	   This change will enable about 30 test cases to pass in Chromium.
> +	   All of these test cases are related to selection rect boundaries.
> +	   This change will enable the test cases to retrieve the selection
> +	   rect boundary rectangle for the current selection.
> +
> +
> +	   * public/WebFrame.h:
> +	   * src/WebFrameImpl.cpp:
> +	   (WebKit::WebFrameImpl::selectionBoundsRect):
> +	   * src/WebFrameImpl.h:
> +

> diff --git a/WebKit/chromium/public/WebFrame.h
b/WebKit/chromium/public/WebFrame.h
> +    // Returns the bounds rect for current selection. If selection is
performed
> +    // on transformed text, the rect will still bound the selection, but
will

no comma before the but (since the phrase after but isn't a complete sentence.


> +    // not be transformed itself. If no selection is present the rect will
be

Add a comma after present.

> +    // empty ((0,0), (0,0))

Add a period to the end of the sentence.


More information about the webkit-reviews mailing list