[webkit-reviews] review granted: [Bug 191164] Caret disappears at end of password field when caps lock indicator is shown; password field not scrolled when caps lock indicator is shown : [Attachment 355651] Patch and layout test

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 191164: Caret disappears at end of password field when caps lock indicator
is shown; password field not scrolled when caps lock indicator is shown
https://bugs.webkit.org/show_bug.cgi?id=191164

Attachment 355651: Patch and layout test

https://bugs.webkit.org/attachment.cgi?id=355651&action=review




--- Comment #6 from Dean Jackson <dino at apple.com> ---
Comment on attachment 355651
  --> https://bugs.webkit.org/attachment.cgi?id=355651
Patch and layout test

View in context: https://bugs.webkit.org/attachment.cgi?id=355651&action=review

> Source/WebCore/editing/FrameSelection.h:162
> +    void setNeedsSelectionUpdate(bool forceRevealSelection = false);

Suggestion: declare an enum class RevealSelection { NotForced, Forced }; and
use that as the parameter.

> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:219
> +	   frame().selection().setNeedsSelectionUpdate(true /*
forceRevealSelection */);

... that way you don't need this comment.

> LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled.html:45
> +function runNextTest()
> +{
> +    if (currentTest >= tests.length) {
> +	   done();
> +	   return;
> +    }
> +    tests[currentTest++]();
> +}

Suggestion: declare const tests = [
function testFocusedEmptyFieldIsNotScrolled() { ... },
function testFieldDidNotScrollAfterTyping() { ... },
...
];

Then you just need
function runTests() {
  tests.forEach(t => { t(); });
  done();
}

Saves the nested stack in runNextTest() as well as all the calls to it.