[webkit-reviews] review granted: [Bug 184510] Implement input-security property (a standardized replacement for -webkit-text-security) : [Attachment 438401] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Aditya Keerthi
<akeerthi at apple.com>'s request for review:
Bug 184510: Implement input-security property (a standardized replacement for
-webkit-text-security)
https://bugs.webkit.org/show_bug.cgi?id=184510

Attachment 438401: Patch

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




--- Comment #8 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 438401
  --> https://bugs.webkit.org/attachment.cgi?id=438401
Patch

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

>>> Source/WebCore/style/StyleAdjuster.cpp:396
>>> +		 style.setTextSecurity(style.inputSecurity() ==
InputSecurity::Auto ? TextSecurity::Disc : TextSecurity::None);
>> 
>> This is not an ideal way to do this because -webkit-test-security is
observable. 
>> 
>> Maybe you could keep this approach and but move this to
HTMLInputElement::createInnerTextStyle? Then the mutated -webkit-test-security
only affects the shadow tree.
> 
> This implementation was intentional (and reflected in the test
`computed-text-security-for-input-security.html` I added). I mentioned the
reasoning in the ChangeLog, but will explain again here.
> 
> I know of at least one script which uses the computed value of
`-webkit-text-security` to determine whether or not an input has obscured text.
Without making the change here, a password field with `input-security: auto`
(which obscures the text) could have `-webkit-text-security: none` in its
computed style. This would lead to the script incorrectly assuming that the
text is unobscured. Consequently, for compatibility, my patch ensures that the
observed value of `-webkit-text-security` matches the `input-security`.

Ok, sounds reasonable