Summary: | [iOS] Draw caps lock indicator in password fields | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||||||||||||||
Component: | WebCore Misc. | Assignee: | Daniel Bates <dbates> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | dino, pvollan, ryanhaddad, webkit-bug-importer, wenson_hsieh | ||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
Version: | WebKit Local Build | ||||||||||||||||||||
Hardware: | iPhone / iPad | ||||||||||||||||||||
OS: | iOS 12 | ||||||||||||||||||||
Bug Depends on: | 190056, 191475 | ||||||||||||||||||||
Bug Blocks: | 190571, 191969, 197724 | ||||||||||||||||||||
Attachments: |
|
Description
Daniel Bates
2018-10-14 15:13:37 PDT
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. 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. Created attachment 353659 [details] Patch This patch will fail to apply because it depends on the patch for bug #190056. Created attachment 354002 [details] Patch Re-uploading patch now for EWS processing now that the patch for bug #190056 landed. Created attachment 354015 [details]
Patch
Created attachment 354102 [details]
Patch
Created attachment 354114 [details]
Patch
Created attachment 354125 [details]
Patch
Created attachment 354290 [details]
Patch
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. 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. Committed r238047: <https://trac.webkit.org/changeset/238047> (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 (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. This change introduced layout test failures on iOS, as documented in https://bugs.webkit.org/show_bug.cgi?id=191491 Reverted r238047 for reason: Introduced layout test failures on iOS simulator. Committed r238062: <https://trac.webkit.org/changeset/238062> (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 *** Bug 191491 has been marked as a duplicate of this bug. *** Committed r238078: <https://trac.webkit.org/changeset/238078> |