Bug 172299 - Web Automation: characters produced with shift modifier on QWERTY keyboard should be delivered as shift-down, char-down, char-up, shift-up events
Summary: Web Automation: characters produced with shift modifier on QWERTY keyboard sh...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Blaze Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-05-18 11:57 PDT by Blaze Burg
Modified: 2017-05-22 13:56 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.75 KB, patch)
2017-05-19 16:57 PDT, Blaze Burg
joepeck: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Blaze 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 Blaze 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 Blaze Burg 2017-05-22 13:56:54 PDT
Committed r217244: <http://trac.webkit.org/changeset/217244>