Bug 172299

Summary: Web Automation: characters produced with shift modifier on QWERTY keyboard should be delivered as shift-down, char-down, char-up, shift-up events
Product: WebKit Reporter: BJ Burg <bburg>
Component: WebKit2Assignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, enrica, joepeck, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch joepeck: review+

Description BJ Burg 2017-05-18 11:57:26 PDT
Currently, we deliver this as keydown(A), keyup(A) but WebDriver tests expect this to be delivered as keydown(SHIFT), keydown(A), keyup(A), keyup(SHIFT) if the shift modifier is not already set via an earlier key press. Note that this means the text HELLO would keydown/keyup the shift key 5 times, because it's not sustained across keystrokes (even if a user might type it that way).
Comment 1 Radar WebKit Bug Importer 2017-05-18 12:07:16 PDT
<rdar://problem/32277988>
Comment 2 BJ Burg 2017-05-19 16:57:26 PDT
Created attachment 310727 [details]
Patch
Comment 3 Joseph Pecoraro 2017-05-22 11:36:09 PDT
Comment on attachment 310727 [details]
Patch

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

r=me

> Source/WebKit2/UIProcess/Automation/mac/WebAutomationSessionMac.mm:521
> +    case 'A':
> +    case 'B':
> +    case 'C':
> +    case 'E':
> +    case 'F':
> +    case 'G':
> +    case 'H':
> +    case 'I':
> +    case 'J':
> +    case 'K':
> +    case 'L':
> +    case 'M':
> +    case 'N':
> +    case 'O':
> +    case 'P':
> +    case 'Q':
> +    case 'R':
> +    case 'S':
> +    case 'T':
> +    case 'U':
> +    case 'V':
> +    case 'W':
> +    case 'X':
> +    case 'Y':
> +    case 'Z':

Can we turn this into an easier to read if statement:

    if (c >= 'A' && c <= 'Z')
        return YES;

The rest can still be in this switch. I realize by putting them all in the switch the compiler might manage some smarter codegen, but I think readability is more important here.
Comment 4 BJ Burg 2017-05-22 13:56:54 PDT
Committed r217244: <http://trac.webkit.org/changeset/217244>