Bug 118566 – [Qt] Implement more of DOM3 KeyEvent key-identifiers

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Allan Sandfeld Jensen]

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.

[Allan Sandfeld Jensen]

Created attachment 206468 [details]
Patch

[Jocelyn Turcotte]

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.

[Allan Sandfeld Jensen]

(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).

[Allan Sandfeld Jensen]

Created attachment 207231 [details]
Patch

[Allan Sandfeld Jensen]

Committed r152961: <http://trac.webkit.org/changeset/152961>