Summary: | [Cocoa] Right Command key should have location DOM_KEY_LOCATION_RIGHT instead of DOM_KEY_LOCATION_STANDARD | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||||
Component: | Platform | Assignee: | Daniel Bates <dbates> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, darin, esprehn+autocc, ews-watchlist, kangil.han, webkit-bug-importer, wenson_hsieh | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Local Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Daniel Bates
2019-10-04 13:19:24 PDT
Created attachment 380245 [details]
For the bots
Created attachment 381555 [details]
Patch
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? (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. 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. Created attachment 382839 [details]
To Land
Comment on attachment 382839 [details] To Land Clearing flags on attachment: 382839 Committed r252066: <https://trac.webkit.org/changeset/252066> All reviewed patches have been landed. Closing bug. |