[webkit-reviews] review granted: [Bug 38100] [chromium] Implement TextInputController::firstRectForCharacterRange : [Attachment 68607] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 23 15:45:42 PDT 2010


Tony Chang <tony at chromium.org> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 38100: [chromium] Implement TextInputController::firstRectForCharacterRange
https://bugs.webkit.org/show_bug.cgi?id=38100

Attachment 68607: Patch
https://bugs.webkit.org/attachment.cgi?id=68607&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=68607&action=review

> WebKit/chromium/src/WebBindings.cpp:319
> +    for (size_t i = data.size(); i > 0; i--)
> +	   result->Set(v8::Number::New(i-1), v8::Number::New(data[i-1]));

Nit: Spaces around the minus sign (e.g., New(i - 1)).  Why not just have the
loop go from 0 to < size() so you don't have to do the minus 1?

> LayoutTests/platform/chromium/test_expectations.txt:968
> +BUGWK38100 WIN LINUX MAC : editing/inserting/caret-position.html = TEXT PASS


Nit: Just remove the platform names here.  No platforms means it applies to all
platforms.


More information about the webkit-reviews mailing list