Bug 193876

Summary: [iOS] Right command key has wrong value for property code
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebKit Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, webkit-bug-importer
Priority: P2 Keywords: InRadar, PlatformOnly
Version: WebKit Local Build   
Hardware: iPhone / iPad   
OS: iOS 12   
Bug Depends on:    
Bug Blocks: 190571    
Attachments:
Description Flags
Patch
none
To land none

Description Daniel Bates 2019-01-26 16:53:09 PST
Steps to reproduce

1. Visit <http://w3c.github.io/uievents/tools/key-event-viewer.html>
2. Focus the text field
3. Press the Command key on the right side of the keyboard.

The value of the code property is Unidentified. But it should be MetaRight. Compare to Mac.
Comment 1 Radar WebKit Bug Importer 2019-01-26 16:53:36 PST
<rdar://problem/47577308>
Comment 2 Daniel Bates 2019-05-09 14:46:30 PDT
Created attachment 369523 [details]
Patch
Comment 3 Daniel Bates 2019-05-09 14:49:20 PDT
Comment on attachment 369523 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        We're are looking for the wrong Windows virtual key code for the right command key.

We're are => We're
Comment 4 Brent Fulgham 2019-05-09 15:05:15 PDT
Comment on attachment 369523 [details]
Patch

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

r=me

> Source/WebCore/ChangeLog:11
> +        the correct value for the code property of the DOM key event.

Do we still need RWIN if the user is running a Microsoft keyboard?

> Source/WebCore/platform/ios/PlatformEventFactoryIOS.mm:278
> +    case VK_APPS: return "MetaRight"_s;

I wonder if we should just keep both of them?
Comment 5 Daniel Bates 2019-05-09 15:54:35 PDT
(In reply to Brent Fulgham from comment #4)
> Comment on attachment 369523 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=369523&action=review
> 
> r=me
> 
> > Source/WebCore/ChangeLog:11
> > +        the correct value for the code property of the DOM key event.
> 
> Do we still need RWIN if the user is running a Microsoft keyboard?
> 

I don't know. This is iOS code. 😀

> > Source/WebCore/platform/ios/PlatformEventFactoryIOS.mm:278
> > +    case VK_APPS: return "MetaRight"_s;
> 
> I wonder if we should just keep both of them?

This is iOS code. 😀
Comment 6 Daniel Bates 2019-05-09 15:56:56 PDT
Created attachment 369527 [details]
To land
Comment 7 Daniel Bates 2019-05-09 16:04:44 PDT
Comment on attachment 369527 [details]
To land

Clearing flags on attachment: 369527

Committed r245162: <https://trac.webkit.org/changeset/245162>
Comment 8 Daniel Bates 2019-05-09 16:04:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Daniel Bates 2019-05-09 16:08:09 PDT
(In reply to Brent Fulgham from comment #4)
> Comment on attachment 369523 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=369523&action=review
> 
> r=me
> 
> > Source/WebCore/ChangeLog:11
> > +        the correct value for the code property of the DOM key event.
> 
> Do we still need RWIN if the user is running a Microsoft keyboard?
> 

Oh I misread your comment. Anyway, the answer is No. These are Windows virtual key codes. How they map is entirely up to us (WebKit). iOS does not emit them. So, I chose to match Mac which maps the key code for the right command to VK_APPS.

> > Source/WebCore/platform/ios/PlatformEventFactoryIOS.mm:278
> > +    case VK_APPS: return "MetaRight"_s;
> 
> I wonder if we should just keep both of them?

No need to keep both. I defined the mapping!