Bug 184510 – Implement input-security property (a standardized replacement for -webkit-text-security)

• Component: CSS
• Status: RESOLVED FIXED

[Theresa O'Connor]

As (will be) specced in CSS UI Level 4.

CSS WG issue: https://github.com/w3c/csswg-drafts/issues/2495

[Theresa O'Connor]

It's now specced: https://drafts.csswg.org/css-ui-4/#input-security

[Radar WebKit Bug Importer]

<rdar://problem/79979992>

[Aditya Keerthi]

Created attachment 438315 [details]
For EWS

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Aditya Keerthi]

Created attachment 438401 [details]
Patch

[Antti Koivisto]

Comment on attachment 438401 [details]
Patch

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

> Source/WebCore/style/StyleAdjuster.cpp:396
> +        if (is<HTMLInputElement>(*m_element) && downcast<HTMLInputElement>(*m_element).isPasswordField())
> +            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.

[Aditya Keerthi]

(In reply to Antti Koivisto from comment #6)
> Comment on attachment 438401 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438401&action=review
> 
> > Source/WebCore/style/StyleAdjuster.cpp:396
> > +        if (is<HTMLInputElement>(*m_element) && downcast<HTMLInputElement>(*m_element).isPasswordField())
> > +            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. 

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`.

[Antti Koivisto]

Comment on attachment 438401 [details]
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

[Aditya Keerthi]

Comment on attachment 438401 [details]
Patch

Thanks for the review!

[EWS]

Committed r282750 (241887@main): <https://commits.webkit.org/241887@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438401 [details].