[webkit-reviews] review denied: [Bug 64143] Page-Up or Page-Down inside textarea scrolls not just its content but the page : [Attachment 100455] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Kentaro Hara <haraken at google.com>'s
request for review:
Bug 64143: Page-Up or Page-Down inside textarea scrolls not just its content
but the page
https://bugs.webkit.org/show_bug.cgi?id=64143

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

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


I'm not familiar with RenderLayer and I won't set r+, but I have some comments.


> LayoutTests/ChangeLog:15
> +
> +	   * editing/input/pageup-and-pagedown-in-textarea-expected.txt: Added.

> +	   * editing/input/pageup-and-pagedown-in-textarea.html: Added.
> +

The file list is out-of-sync.  You updated other files.

> LayoutTests/editing/input/pageup-and-pagedown-in-textarea-expected.txt:33
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.
> +Many many lines here.

These lines are meaningless.  Please remove them before the test completion.

> LayoutTests/editing/input/pageup-and-pagedown-in-textarea.html:76
> +}

Please show something if there is no layoutTestController.
If we can test manually on a browser, show manual test instructions. Otherwise,
show an excuse message about the requirement of layoutTestController.

>
LayoutTests/platform/chromium-win/fast/layers/scroll-rect-to-visible-expected.t
xt:-62
>	   text run at (0,0) width 11: "A"
> -scrolled to 0,13

You updated a text expectation for chromium-win but not updated an image
expectation. So this will break Chromium-win buidbots.
Please add this test to LayoutTests/platform/chromium/test_expectations.txt to
avoid the breakage.