Bug 192788 - Wrong value for key property in keydown and keyup events generated holding Control key
Summary: Wrong value for key property in keydown and keyup events generated holding Co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 12
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks: 190571
  Show dependency treegraph
 
Reported: 2018-12-17 16:41 PST by Daniel Bates
Modified: 2018-12-20 09:52 PST (History)
4 users (show)

See Also:


Attachments
Patch (3.28 KB, patch)
2018-12-17 17:02 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-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>.