Bug 192788

Summary: Wrong value for key property in keydown and keyup events generated holding Control key
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, megan_gardner, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: iPhone / iPad   
OS: iOS 12   
See Also: https://bugs.webkit.org/show_bug.cgi?id=192824
Bug Depends on:    
Bug Blocks: 190571    
Attachments:
Description Flags
Patch none

Description Daniel Bates 2018-12-17 16:41:29 PST
Steps to reproduce:

1. Visit <https://unixpapa.com/js/testkey.html>.
2. Ensure default handling is not suppressed for any of the offered events.
3. Enable attribute DOM 3.
4. Focus the text field.
5. Press some key while holding Control down, say Control + A.

The keydown and keyup events visually show an empty string for the key property. Compare to Mac, which has output of the form:

[[
keydown  keyCode=17        which=17        charCode=0        
         key=Control char=undefined location=1 repeat=false
keydown  keyCode=65  (A)   which=65  (A)   charCode=0        
         key=a char=undefined location=0 repeat=false
keyup    keyCode=65  (A)   which=65  (A)   charCode=0        
         key=a char=undefined location=0 repeat=false
keyup    keyCode=17        which=17        charCode=0        
         key=Control char=undefined location=1 repeat=false
]]
Comment 1 Daniel Bates 2018-12-17 16:41:51 PST
<rdar://problem/46795214>
Comment 2 Daniel Bates 2018-12-17 17:02:45 PST
Created attachment 357504 [details]
Patch

I did not include a test with this change as the patch uses the same code as we do on Mac, we do currently have infrastructure to generate a key event with a Control key modifier flag that has different unmodified and modified input strings, and I wanted to post this patch sooner. When I have a moment I will look to add testing support.
Comment 3 Daniel Bates 2018-12-18 16:23:28 PST
Comment on attachment 357504 [details]
Patch

Clearing flags on attachment: 357504

Committed r239361: <https://trac.webkit.org/changeset/239361>
Comment 4 Daniel Bates 2018-12-18 16:23:30 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 2018-12-20 09:20:23 PST
Comment on attachment 357504 [details]
Patch

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

> Source/WebCore/platform/ios/PlatformEventFactoryIOS.mm:199
> +    NSString *characters = isControlDown ? event.charactersIgnoringModifiers : event.characters;

Does this do the right thing for Control+Shift+A and other combinations that include both Control and other modifiers?
Comment 6 Daniel Bates 2018-12-20 09:52:01 PST
(In reply to Darin Adler from comment #5)
> Comment on attachment 357504 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=357504&action=review
> 
> > Source/WebCore/platform/ios/PlatformEventFactoryIOS.mm:199
> > +    NSString *characters = isControlDown ? event.charactersIgnoringModifiers : event.characters;
> 
> Does this do the right thing for Control+Shift+A and other combinations that
> include both Control and other modifiers?

It will once we fix <rdar://problem/46874406>.