[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 #7 from Seigo Nonaka <nona at chromium.org>  2012-06-04 05:40:44 PST ---
(From update of attachment 144974)
View in context: https://bugs.webkit.org/attachment.cgi?id=144974&action=review

>> Source/WebKit/chromium/ChangeLog:12
>> +
> 
> I do not understand what this change is, but you might want to add a LayoutTest if possible.

I revised the description.
And I think LayoutTest is not for WebKit/chromium.
I think webkit_unit_tests is for it, but I don't know how to...
Anyway, I will add tests for above API, thanks.

>> 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&)

Done, Thanks.

>> Source/WebKit/chromium/src/WebViewImpl.cpp:2012
>> +      return false;
> 
> 4 spaces indentation please.

Done, Thanks.

>> Source/WebKit/chromium/src/WebViewImpl.cpp:2018
>> +      return false;
> 
> 4 spaces indentation please.

Done, Thanks.

>> Source/WebKit/chromium/src/WebViewImpl.cpp:2023
>> +                        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);

Done, Thanks.

>> Source/WebKit/chromium/src/WebViewImpl.cpp:2026
>> +                      ec);
> 
> Ditto.

Done, Thanks.

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