Bug 180651 – Add more auto fill button types

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

Add more auto fill button types.

[Daniel Bates]

<rdar://problem/35891125>

[Daniel Bates]

Created attachment 328992 [details]
Patch and layout tests

[EWS Watchlist]

Comment on attachment 328992 [details]
Patch and layout tests

Attachment 328992 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5617441

New failing tests:
fast/forms/auto-fill-button/input-strong-confirmation-password-auto-fill-button.html
fast/forms/auto-fill-button/input-strong-password-auto-fill-button.html

[EWS Watchlist]

Created attachment 329001 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Daniel Bates]

Created attachment 329002 [details]
Patch and layout tests

[EWS Watchlist]

Comment on attachment 329002 [details]
Patch and layout tests

Attachment 329002 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5618641

New failing tests:
fast/forms/auto-fill-button/input-strong-confirmation-password-auto-fill-button.html
fast/forms/auto-fill-button/input-strong-password-auto-fill-button.html

[EWS Watchlist]

Created attachment 329013 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 329002 [details]
Patch and layout tests

Attachment 329002 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5618684

New failing tests:
fast/forms/auto-fill-button/input-strong-confirmation-password-auto-fill-button.html
fast/forms/auto-fill-button/input-strong-password-auto-fill-button.html

[EWS Watchlist]

Created attachment 329016 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 329002 [details]
Patch and layout tests

Attachment 329002 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5618938

New failing tests:
fast/forms/auto-fill-button/input-disabled-strong-password-and-strong-confirmation-password-auto-fill-buttons.html
fast/text/user-installed-fonts/shadow-postscript-family.html
fast/forms/auto-fill-button/input-readonly-strong-password-and-strong-confirmation-password-auto-fill-buttons.html

[EWS Watchlist]

Created attachment 329018 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 329002 [details]
Patch and layout tests

Attachment 329002 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5619101

New failing tests:
fast/forms/auto-fill-button/input-strong-confirmation-password-auto-fill-button.html
fast/forms/auto-fill-button/input-strong-password-auto-fill-button.html

[EWS Watchlist]

Created attachment 329019 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Daniel Bates]

Created attachment 329029 [details]
Patch and layout tests

[EWS Watchlist]

Comment on attachment 329029 [details]
Patch and layout tests

Attachment 329029 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5621112

New failing tests:
fast/forms/auto-fill-button/input-strong-confirmation-password-auto-fill-button.html
fast/forms/auto-fill-button/input-strong-password-auto-fill-button.html

[EWS Watchlist]

Created attachment 329038 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Daniel Bates]

Created attachment 329041 [details]
Patch and layout tests

[Daniel Bates]

Created attachment 329054 [details]
Patch and layout tests

[Brent Fulgham]

Comment on attachment 329054 [details]
Patch and layout tests

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

Looks good. I had a minor suggestion about RefPtr -> Ref, and I think you should have two separate AX labels. But otherwise this is great.

> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3089
> +            return @"strong password";

I think these should be separate roles. I think it would be confusing to get the same role reported by AX -- how would a blind person know which of the password fields they were on?

> Source/WebCore/html/HTMLInputElement.cpp:2046
> +    firstStop.m_position = CSSValuePool::singleton().createValue(3, CSSPrimitiveValue::UnitType::CSS_EMS);

Not to be addressed in this patch: It sure seems like these should be constructor arguments, with the CSSValuePool::singleton being used inside the constructor! This is a lot of extra typing!

> Source/WebCore/html/HTMLInputElement.cpp:2056
> +    return gradient;

It seems weird to return a RefPtr<>, when it's only use case immediately dereferences the pointer. Should we return a Ref<> here instead?

> Source/WebCore/html/HTMLInputElement.h:247
> +    AutoFillButtonType autoFillButtonType() const { return static_cast<AutoFillButtonType>(m_autoFillButtonType); }

+1 :-)

> Source/WebCore/html/TextFieldInputType.cpp:411
> +        return AXAutoFillStrongPasswordLabel();

I think these should be two separate AX labels.

> Source/WebCore/platform/LocalizedStrings.cpp:646
> +}

Please add a label for the confirmation password field.

[Daniel Bates]

Comment on attachment 329054 [details]
Patch and layout tests

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

>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3089
>> +            return @"strong password";
> 
> I think these should be separate roles. I think it would be confusing to get the same role reported by AX -- how would a blind person know which of the password fields they were on?

Will fix. The strong password and strong confirmation password buttons will likely trigger the same action and hence it may be sufficient for them to be considered having the same role. It does not hurt to consider them separate for now until we finalize the accessibility story for these buttons.

>> Source/WebCore/html/HTMLInputElement.cpp:2046
>> +    firstStop.m_position = CSSValuePool::singleton().createValue(3, CSSPrimitiveValue::UnitType::CSS_EMS);
> 
> Not to be addressed in this patch: It sure seems like these should be constructor arguments, with the CSSValuePool::singleton being used inside the constructor! This is a lot of extra typing!

Notice that the unit type of the gradient position can be arbitrary and depends on the look that a person is trying to achieve with the gradient. I am unclear of the value of using a such a constructor given this constraint.

>> Source/WebCore/html/HTMLInputElement.cpp:2056
>> +    return gradient;
> 
> It seems weird to return a RefPtr<>, when it's only use case immediately dereferences the pointer. Should we return a Ref<> here instead?

Will fix.

>> Source/WebCore/html/TextFieldInputType.cpp:411
>> +        return AXAutoFillStrongPasswordLabel();
> 
> I think these should be two separate AX labels.

Will do.

>> Source/WebCore/platform/LocalizedStrings.cpp:646
>> +}
> 
> Please add a label for the confirmation password field.

Will do.

[Daniel Bates]

Committed r225879: <https://trac.webkit.org/changeset/225879>

[Daniel Bates]

Committed Windows build fix in <https://trac.webkit.org/changeset/225885>.

[Daniel Bates]

Committed updated results in <https://trac.webkit.org/changeset/225889>.

[Daniel Bates]

Committed macOS El Capitan and Apple Windows results in <https://trac.webkit.org/changeset/225894>.