Bug 118566

Summary: [Qt] Implement more of DOM3 KeyEvent key-identifiers
Product: WebKit Reporter: Allan Sandfeld Jensen <allan.jensen>
Component: WebKit QtAssignee: Allan Sandfeld Jensen <allan.jensen>
Status: RESOLVED FIXED    
Severity: Normal CC: hausmann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 118445    
Attachments:
Description Flags
Patch
none
Patch jturcotte: review+

Description Allan Sandfeld Jensen 2013-07-11 08:22:19 PDT
We currently translate a small set of Qt keycodes to the DOM3 key-identifier value. Since we want the key-identifier to replace the uses of Windows keycodes, we should make sure we support them as well as we can.
Comment 1 Allan Sandfeld Jensen 2013-07-11 08:39:16 PDT
Created attachment 206468 [details]
Patch
Comment 2 Jocelyn Turcotte 2013-07-18 06:02:14 PDT
Comment on attachment 206468 [details]
Patch

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

> Source/WebCore/ChangeLog:7
> +

Extra line.

> Source/WebCore/platform/qt/PlatformKeyboardEventQt.cpp:331
> +        // FIXME: This is rather wrong.

It would be nice if the comment would say why it's wrong, it's not obvious to me.

> Source/WebCore/platform/qt/PlatformKeyboardEventQt.cpp:416
> -    case Qt::Key_Menu:
>      case Qt::Key_Alt:
>          return VK_MENU; // (12) ALT key

Does this change the behavior somehow? It might be worth at least adding a note in the changelog.
Comment 3 Allan Sandfeld Jensen 2013-07-22 02:17:56 PDT
(In reply to comment #2)
> (From update of attachment 206468 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206468&action=review
> 
> > Source/WebCore/ChangeLog:7
> > +
> 
> Extra line.
> 
> > Source/WebCore/platform/qt/PlatformKeyboardEventQt.cpp:331
> > +        // FIXME: This is rather wrong.
> 
> It would be nice if the comment would say why it's wrong, it's not obvious to me.
> 
It encodes the Qt keycode value into the string instead of anything standard. The other "U+" strings all encode char-codes (ASCII), but this encodes something entirely Qt specific. For ascii keys it is more or less the same but function keys Qt starts at 0x01000000 and can not even be encoded with 16bit hex.

> > Source/WebCore/platform/qt/PlatformKeyboardEventQt.cpp:416
> > -    case Qt::Key_Menu:
> >      case Qt::Key_Alt:
> >          return VK_MENU; // (12) ALT key
> 
> Does this change the behavior somehow? It might be worth at least adding a note in the changelog.

Yes, the VK_MENU key is old windows terminology for the Alt key. Our menu key is what Windows call VK_APPS (what brings up the context menu).
Comment 4 Allan Sandfeld Jensen 2013-07-22 02:39:32 PDT
Created attachment 207231 [details]
Patch
Comment 5 Allan Sandfeld Jensen 2013-07-22 04:42:57 PDT
Committed r152961: <http://trac.webkit.org/changeset/152961>