[webkit-reviews] review granted: [Bug 200037] Allow Clients to make an input field considered autofilled and secure while viewable : [Attachment 375633] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Priyanka <pagarwal999 at apple.com>'s
request for review:
Bug 200037: Allow Clients to make an input field considered autofilled and
secure while viewable
https://bugs.webkit.org/show_bug.cgi?id=200037

Attachment 375633: Patch

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




--- Comment #12 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 375633
  --> https://bugs.webkit.org/attachment.cgi?id=375633
Patch

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

This looks really good.

> Source/WebCore/ChangeLog:3
> +	   Allow Clients to make an input field considered autofilled and
secure while viewable

Bug title should be fixed ��:

Clients => clients

> Source/WebCore/ChangeLog:6
> +

I feel that the title is hard to read and confuses me. I don't understand how a
field can be considered "secure" and viewable. <-- I know what you mean by
reading the patch, but that's not how I would describe this change. Can we
please improve the title? If not, please provide a more elaborated description
of the purpose of this change under the "Reviewed by" line.

> Source/WebCore/css/html.css:740
> +    -webkit-text-security: none !important;
> +    cursor: default !important;
> +    font-family: monospace;

This is duplicating the properties of input:-webkit-autofill-strong-password
(above). Please share.

>
LayoutTests/fast/forms/auto-fill-button/resources/process-auto-fill-button-type
-and-invoke-runTest-for-strong-password-viewable.js:13
> +window.onload = function ()
> +{
> +    if (!window.internals) {
> +	   console.log("This test must be run in DumpRenderTree or
WebKitTestRunner.");
> +	   return;
> +    }
> +    let inputElements = document.getElementsByTagName("input");
> +    for (let inputElement of inputElements) {
> +	   internals.setAutoFilledAndViewable(inputElement,
inputElement.dataset.autoFilledAndViewable == "true");
> +    }
> +    if (window.runTest)
> +	   window.runTest();
> +}

This duplicates the code in
<https://trac.webkit.org/browser/webkit/trunk/LayoutTests/fast/forms/auto-fill-
button/resources/process-auto-fill-button-type-and-invoke-runTest.js>. Only
difference is checking for inputElement.dataset.autoFilledAndViewable. Please
modify process-auto-fill-button-type-and-invoke-runTest.js instead of
duplicating it.