[webkit-reviews] review denied: [Bug 61023] [Chromium] IME candidate window appears wrong position in an iframe : [Attachment 93871] Patch V0

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Kenichi Ishibashi
<bashi at chromium.org>'s request for review:
Bug 61023: [Chromium] IME candidate window appears wrong position in an iframe
https://bugs.webkit.org/show_bug.cgi?id=61023

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=93871&action=review

Chromium bots runs all of tests even in LayoutTests/platform/.	You need to
update test_expectations.txt.

>
LayoutTests/platform/chromium-mac/editing/input/ime-candidate-window-position-e
xpected.txt:7
> +PASS successfullyParsed is true
> +
> +TEST COMPLETE
> +rect[0] denotes x coodinate and rect[1] denotes y coodinate
> +PASS rect[0] is frame.offsetLeft + input.offsetLeft + 1
> +PASS rect[1] is frame.offsetTop + input.offsetTop

The output order looks very broken.
You should use jsTestIsAsync&finishJSTest().  See
http://trac.webkit.org/browser/trunk/LayoutTests/http/tests/websocket/tests/clo
se-event.html?rev=86315

>
LayoutTests/platform/chromium-mac/editing/input/ime-candidate-window-position.h
tml:19
> +	   textInputController.firstRectForCharacterRange(0, 0);
> +	   rect = textInputController.firstRectForCharacterRange(0, 0);

The first line looks unneecssary.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1164
> -    // When inside an text control, don't adjust the range.
> -    if (!selectionRoot)
> -	   rect = frame()->view()->contentsToWindow(rect);
> +    rect = frame()->view()->contentsToWindow(rect);

Is firstRectForCharacterRange() used only for DRT and IME? Won't his change
make regressions for other usages?