[webkit-reviews] review canceled: [Bug 65027] Inserting text into scrolled out text controls should bring them into the center of the view, not on an edge : [Attachment 103684] Added tests for caret revealing in multiline edit

[bugzilla-daemon at webkit.org]

Mikhail Naganov <mnaganov at chromium.org> has canceled Mikhail Naganov
<mnaganov at chromium.org>'s request for review:
Bug 65027: Inserting text into scrolled out text controls should bring them
into the center of the view, not on an edge
https://bugs.webkit.org/show_bug.cgi?id=65027

Attachment 103684: Added tests for caret revealing in multiline edit
https://bugs.webkit.org/attachment.cgi?id=103684&action=review

------- Additional Comments from Mikhail Naganov <mnaganov at chromium.org>
(In reply to comment #30)
> (From update of attachment 103684 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=103684&action=review
> 
> >
LayoutTests/editing/input/reveal-contenteditable-on-input-vertically.html:20
> > +	 if (window.layoutTestController)
> > +	     layoutTestController.dumpAsText();
> 
> You shouldn't indent scripts like this.  Please outdent.
>

Fixed.
 
> >
LayoutTests/editing/input/reveal-contenteditable-on-paste-vertically.html:40
> > +	 textInputController.insertText("WebKit");
> 
> I'd prefer using document.execCommand('insertText') instead.
>

This way, it doesn't work. Perhaps I need to pass control to the renderer after
executing the command via setTimeout(0)? Using textInputController works and
looks simpler.
 
> > LayoutTests/editing/input/reveal-edit-on-input-vertically.html:30
> > +function offsetFromViewportTop(element)
> > +{
> > +	 return element.getClientRects()[0].top;
> > +}
> > + 
> > +function offsetOfMiddleFromViewportTop(element)
> > +{
> > +	 return element.getClientRects()[0].top +
Math.round(element.getClientRects()[0].height / 2);
> > +}
> 
> Please share code between tests by putting some of these code in a shared
file in resources directory.

I've factored out these functions. I also evaluated, if it makes sense to
extract control flow common for several scripts, but the result turned out to
be more cumbersome, as test-specific actions need access to variables that will
be extracted as a part of common flow.