Bug 190565 – [iOS] Draw caps lock indicator in password fields

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

[Daniel Bates]

We should show the caps lock indicator in a focused password field when caps lock is enabled so as to match iOS platform conventions.

We need to take care to track caps lock state changes when switching between apps and with respect to changes in keyboard availability.

[Daniel Bates]

The patch for bug #190056 adds a new WebKit message that the UI process can send to the web process when modifier key state has changes. We should make use of this to update caps lock state in Modern WebKit.

[Daniel Bates]

Created attachment 352298 [details]
Work-in-progress patch

Work-in-progress patch. This patch depends on the patch for bug #190056 and bug #190487 (in order) and some UIKit changes. Still need to listen for changes to hardware keyboard availability and update caps lock state. Need to verify correctness in legacy WebKit and Modern WebKit when caps lock state changes when switching between Safari and another app. Here are some app-switching test cases to consider assuming caps lock is initially disabled:

1. Press the caps lock, open Safari, focus on password field and ensure that the caps lock indicator is visible.
2. Open Safari, focus on password field, press caps lock, go to Home screen, press caps lock, open Safari and ensure that the caps lock indicator is not visible.
3. Open Safari, focus on password field, go to Home screen, press caps lock, open Safari and ensure that the caps lock indicator is visible.
4. Open Safari, focus on password field, press caps lock, go to app switcher, press caps lock, switch to Safari and ensure that the caps lock indicator is not visible.
5. Open Safari, focus on password field, go to app switcher, press caps lock, switch to Safari and ensure that the caps lock indicator is visible.
6. Open Safari, focus on password field, press caps lock, go to app switcher and switch to another app, press caps lock, go to app switcher and switch to Safari and ensure that the caps lock indicator is not visible.
7. Open Safari, focus on password field, go to app switcher and switch to another app, press caps lock, go to app switcher and switch to Safari and ensure that the caps lock indicator is visible.

[Radar WebKit Bug Importer]

<rdar://problem/45262343>

[Daniel Bates]

Created attachment 353659 [details]
Patch

This patch will fail to apply because it depends on the patch for bug #190056.

[Daniel Bates]

Created attachment 354002 [details]
Patch

Re-uploading patch now for EWS processing now that the patch for bug #190056 landed.

[Daniel Bates]

Created attachment 354015 [details]
Patch

[Daniel Bates]

Created attachment 354102 [details]
Patch

[Daniel Bates]

Created attachment 354114 [details]
Patch

[Daniel Bates]

Created attachment 354125 [details]
Patch

[Daniel Bates]

Created attachment 354290 [details]
Patch

[Dean Jackson]

Comment on attachment 354290 [details]
Patch

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

> Source/WebCore/platform/cocoa/KeyEventCocoa.mm:33
> +#import <wtf/MainThread.h>

Why did you need this?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3761
> +    bool isHardwareKeyboardEvent = !!event._hidEvent;
> +    if (isHardwareKeyboardEvent && ((UIPhysicalKeyboardEvent *)event)._inputFlags & kUIKeyboardInputModifierFlagsChanged)
> +        _page->updateCurrentModifierState();

Why do we only want this with the hardware keyboard? I feel like we should also do it if you've activated capslock on the software keyboard.

[Daniel Bates]

Comment on attachment 354290 [details]
Patch

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

>> Source/WebCore/platform/cocoa/KeyEventCocoa.mm:33
>> +#import <wtf/MainThread.h>
> 
> Why did you need this?

Oops! Will remove before landing.

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3761
>> +        _page->updateCurrentModifierState();
> 
> Why do we only want this with the hardware keyboard? I feel like we should also do it if you've activated capslock on the software keyboard.

We also want this for the software keyboard however we need to do more work to get shift state changes. Filed bug #191475 for this.

[Daniel Bates]

Committed r238047: <https://trac.webkit.org/changeset/238047>

[Ryan Haddad]

(In reply to Daniel Bates from comment #13)
> Committed r238047: <https://trac.webkit.org/changeset/238047>

This change broke iOS builds:
https://build.webkit.org/builders/Apple%20iOS%2012%20Release%20%28Build%29/builds/1177/steps/compile-webkit/logs/stdio

[Daniel Bates]

(In reply to Ryan Haddad from comment #14)
> (In reply to Daniel Bates from comment #13)
> > Committed r238047: <https://trac.webkit.org/changeset/238047>
> 
> This change broke iOS builds:
> https://build.webkit.org/builders/Apple%20iOS%2012%20Release%20%28Build%29/
> builds/1177/steps/compile-webkit/logs/stdio

We do not seem to be regenerating derived sources following the change to WebPage.messages.in. Filed bug #191477.

[Ryan Haddad]

This change introduced layout test failures on iOS, as documented in https://bugs.webkit.org/show_bug.cgi?id=191491

[Ryan Haddad]

Reverted r238047 for reason:

Introduced layout test failures on iOS simulator.

Committed r238062: <https://trac.webkit.org/changeset/238062>

[Ryan Haddad]

(In reply to Ryan Haddad from comment #17)
> Reverted r238047 for reason:
> 
> Introduced layout test failures on iOS simulator.
> 
> Committed r238062: <https://trac.webkit.org/changeset/238062>
These are the tests that started failing with this change:

fast/css/text-overflow-input.html [ Failure ]
fast/forms/basic-inputs.html [ Failure ]
fast/forms/input-appearance-height.html [ Failure ]
fast/forms/input-value.html [ Failure ]
fast/forms/placeholder-pseudo-style.html [ Failure ]
tables/mozilla_expected_failures/bugs/bug92647-1.html [ Failure ]

Diffs can be seen here: https://build.webkit.org/results/Apple%20iOS%2012%20Simulator%20Release%20WK2%20(Tests)/r238048%20(865)/results.html

[Ryan Haddad]

*** Bug 191491 has been marked as a duplicate of this bug. ***

[Daniel Bates]

Committed r238078: <https://trac.webkit.org/changeset/238078>