[Cocoa] Right Command key should have location DOM_KEY_LOCATION_RIGHT instead of DOM_KEY_LOCATION_STANDARD

[Daniel Bates]

This bug is only applicable to Cocoa platforms.

Steps to reproduce:

1. Visit <https://unixpapa.com/js/testkey.html>.
2. Ensure the show attribute values for DOM 3 are enabled.
3. Press the Command key on the right-side of the keyboard.

Then I see:

keydown  keyCode=93  (])   which=93  (])   charCode=0        
         key=Meta char=undefined location=0 repeat=false
keyup    keyCode=93  (])   which=93  (])   charCode=0        
         key=Meta char=undefined location=0 repeat=false

Notice the location attribute is 0 == DOM_KEY_LOCATION_STANDARD

But I should see:

keydown  keyCode=93  (])   which=93  (])   charCode=0        
         key=Meta char=undefined location=1 repeat=false
keyup    keyCode=93  (])   which=93  (])   charCode=0        
         key=Meta char=undefined location=1 repeat=false

The location attribute should be 1 == DOM_KEY_LOCATION_RIGHT

[Radar WebKit Bug Importer]

<rdar://problem/55992775>

[Daniel Bates]

Created attachment 380245 [details]
For the bots

[Daniel Bates]

Created attachment 381555 [details]
Patch

[Darin Adler]

Comment on attachment 381555 [details]
Patch

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

> Source/WebCore/dom/KeyboardEvent.cpp:93
> +#if PLATFORM(COCOA)
> +    case VK_APPS: // Right Command
> +#endif

What harm would it cause to compile this on non-Cocoa platforms?

[Daniel Bates]

(In reply to Darin Adler from comment #4)
> Comment on attachment 381555 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381555&action=review
> 
> > Source/WebCore/dom/KeyboardEvent.cpp:93
> > +#if PLATFORM(COCOA)
> > +    case VK_APPS: // Right Command
> > +#endif
> 
> What harm would it cause to compile this on non-Cocoa platforms?

Event.location == DOM_KEY_LOCATION_RIGHT for VK_APPS when I don't know where it is on keyboards for non-Cocoa platforms. I don't have a keyboard with this key, yet.

Longer term, need to audit if it is 100% correct to map right command to VK_APPS. Got to boot up Windows to double check. I think I recall USB HID spec saying it should be mapped to what corresponds to VK_RWIN.

Anyway, I didn't have a Windows setup to test so I patched ^^^ to match VK_APPS. Probably add a FIXME before landing to explain this if you bless this patch.

[Wenson Hsieh]

Comment on attachment 381555 [details]
Patch

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

>>> Source/WebCore/dom/KeyboardEvent.cpp:93
>>> +#endif
>> 
>> What harm would it cause to compile this on non-Cocoa platforms?
> 
> Event.location == DOM_KEY_LOCATION_RIGHT for VK_APPS when I don't know where it is on keyboards for non-Cocoa platforms. I don't have a keyboard with this key, yet.
> 
> Longer term, need to audit if it is 100% correct to map right command to VK_APPS. Got to boot up Windows to double check. I think I recall USB HID spec saying it should be mapped to what corresponds to VK_RWIN.
> 
> Anyway, I didn't have a Windows setup to test so I patched ^^^ to match VK_APPS. Probably add a FIXME before landing to explain this if you bless this patch.

A FIXME to clarify this sounds reasonable.

[Daniel Bates]

Created attachment 382839 [details]
To Land

[Daniel Bates]

Comment on attachment 382839 [details]
To Land

Clearing flags on attachment: 382839

Committed r252066: <https://trac.webkit.org/changeset/252066>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.