Bug 191596 - [iOS] Shift + Tab does not focus previous field
Summary: [iOS] Shift + Tab does not focus previous field
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 12
: P2 Normal
Assignee: Daniel Bates
URL: http://www.facebook.com
Keywords: InRadar, PlatformOnly
Depends on: 173341
Blocks: 190571
  Show dependency treegraph
 
Reported: 2018-11-13 13:17 PST by Daniel Bates
Modified: 2018-11-15 11:15 PST (History)
5 users (show)

See Also:


Attachments
Patch and layout tests (27.01 KB, patch)
2018-11-13 13:50 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
For EWS (27.28 KB, patch)
2018-11-13 16:42 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
To Land (28.26 KB, patch)
2018-11-15 11:04 PST, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2018-11-13 13:17:14 PST
Pressing Shift + Tab does not focus the previous field on facebook.com.

Steps to reproduce:

The following assume that a hardware keyboard is attached.

1. Visit <https://www.facebook.com>.
2. Tap the password field.
3. Press Shift + Tab.

Then the Email or Phone field should be focused. But there is no change in focus.
Comment 1 Daniel Bates 2018-11-13 13:17:27 PST
<rdar://problem/45892053>
Comment 2 Daniel Bates 2018-11-13 13:50:24 PST
Created attachment 354699 [details]
Patch and layout tests
Comment 3 Wenson Hsieh 2018-11-13 15:03:52 PST
Comment on attachment 354699 [details]
Patch and layout tests

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

r=me, with non-internal iOS builds fixed

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:2427
> +        return [self isFirstResponder];

Have you considered consulting `AssistedNodeInformation.hasNextNode` or `AssistedNodeInformation.hasPreviousNode` as well?
Comment 4 Wenson Hsieh 2018-11-13 15:07:54 PST
Comment on attachment 354699 [details]
Patch and layout tests

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

> Tools/ChangeLog:21
> +        and an array of modifier keys. The behavior of this function is analagous to eventSender.keyDown().

Nit - analagous => analogous

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:405
> +    // Character can be either a single Unicode code point or a the name of a special key (e.g. "downArrow").

Nit - "or a the"
Comment 5 Daniel Bates 2018-11-13 16:42:04 PST
Created attachment 354728 [details]
For EWS
Comment 6 Daniel Bates 2018-11-13 16:42:53 PST
Comment on attachment 354728 [details]
For EWS

For EWS. Need to address review feedback.
Comment 7 Daniel Bates 2018-11-15 10:58:18 PST
(In reply to Wenson Hsieh from comment #3)
> Comment on attachment 354699 [details]
> Patch and layout tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354699&action=review
> 
> r=me, with non-internal iOS builds fixed
> 
> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:2427
> > +        return [self isFirstResponder];
> 
> Have you considered consulting `AssistedNodeInformation.hasNextNode` or
> `AssistedNodeInformation.hasPreviousNode` as well?

Will write in terms of AssistedNodeInformation.has{Next, Previous}Node such that the beginning of -canPerformActionForWebView reads:
[[
- (BOOL)canPerformActionForWebView:(SEL)action withSender:(id)sender
{        
{
    if (action == @selector(_nextAccessoryTab:))
        return hasAssistedNode(_assistedNodeInformation) && _assistedNodeInformation.hasNextNode;
    if (action == @selector(_previousAccessoryTab:))
        return hasAssistedNode(_assistedNodeInformation) && _assistedNodeInformation.hasPreviousNode;
...
]]
Comment 8 Daniel Bates 2018-11-15 10:58:33 PST
(In reply to Wenson Hsieh from comment #4)
> Comment on attachment 354699 [details]
> Patch and layout tests
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354699&action=review
> 
> > Tools/ChangeLog:21
> > +        and an array of modifier keys. The behavior of this function is analagous to eventSender.keyDown().
> 
> Nit - analagous => analogous
> 

Will fix.

> > Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:405
> > +    // Character can be either a single Unicode code point or a the name of a special key (e.g. "downArrow").
> 
> Nit - "or a the"

Will fix.
Comment 9 Daniel Bates 2018-11-15 11:04:55 PST
Created attachment 354957 [details]
To Land
Comment 10 Daniel Bates 2018-11-15 11:15:39 PST
Committed r238235: <https://trac.webkit.org/changeset/238235>