[webkit-reviews] review denied: [Bug 68198] Refactor WebViewImpl::scrollFocusedNodeIntoRect to a better place : [Attachment 108489] Patch

Fri Sep 23 11:28:43 PDT 2011

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Varun Jain
<varunjain at chromium.org>'s request for review:
Bug 68198: Refactor WebViewImpl::scrollFocusedNodeIntoRect to a better place
https://bugs.webkit.org/show_bug.cgi?id=68198

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108489&action=review

Looks good, except for the test.

> LayoutTests/fast/dom/scroll-element-to-rect-centered.html:17
> +	   if (window.internals) {
> +	     var box = document.getElementById('textbox');
> +	     window.internals.scrollElementToRect(box, 50, 30, 200, 250);
> +	   }
> +	 }
> +	 window.addEventListener("load", doTest);
> +    </script>
> +  </head>
> +  <body>
> +    This is test<br>
> +    <div style="position: relative; width: 2400px; height: 2400px;
background-color: white;">
> +	 <div style="position:fixed; left: 50px; top: 30px; width: 200px;
height: 250px; background-color: green;"></div>
> +	 <div style="position:absolute; left: 600px; top: 800px;"> <input
id="textbox" type="text"></div>

I am pretty sure we can make this test dumpAsText(), to avoid pixel baselines.
Can we?

> Source/WebKit/chromium/src/WebViewImpl.cpp:1824
> +    if (focusedNode && focusedNode->isElementNode()) {

early return?