[Webkit-unassigned] [Bug 87911] [chromium] There is no way to retrieve composition character rectangle in WebKit/chromium

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Kentaro Hara <haraken at chromium.org>  2012-06-04 04:07:28 PST ---
(From update of attachment 144974)
View in context: https://bugs.webkit.org/attachment.cgi?id=144974&action=review

Just nit picking comments

> Source/WebKit/chromium/ChangeLog:12
> +        Introduce new API to retrieve composition character boundary rectangles.
> +        This API is necessary for Japanese IME to show IME related window on
> +        the right position. In the case of Windows, this message is used to
> +        implement IMR_QUERYCHARPOSITION.
> +

I do not understand what this change is, but you might want to add a LayoutTest if possible.

> Source/WebKit/chromium/public/WebWidget.h:188
> +    virtual bool compositionCharacterBounds(size_t index, WebRect& rect) const { return false; }

WebKit does not write obvious argument names. This can be:

virtual bool compositionCharacterBounds(size_t index, WebRect&)

> Source/WebKit/chromium/src/WebViewImpl.cpp:2012
> +      return false;

4 spaces indentation please.

> Source/WebKit/chromium/src/WebViewImpl.cpp:2018
> +      return false;

4 spaces indentation please.

> Source/WebKit/chromium/src/WebViewImpl.cpp:2023
> +    tempRange->setStart(compositionRange->startContainer(ec),
> +                        compositionRange->startOffset(ec) + index,
> +                        ec);

You do not need wrap the code by 80 characters. You can write like this:

tempRange->setStart(compositionRange->startContainer(ec), compositionRange->startOffset(ec) + index, ec);

> Source/WebKit/chromium/src/WebViewImpl.cpp:2026
> +    tempRange->setEnd(compositionRange->startContainer(ec),
> +                      compositionRange->startOffset(ec) + index + 1,
> +                      ec);

Ditto.

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