D6233: KKeyServer: fix handling of KeypadModifier.

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> graesslin wrote in kkeyserver.cpp:976-980
> This is a special handling from KGlobalAccel. Are you sure it belongs into the generic implementation?

I would say yes, because e.g. Ctrl+& (on qwerty) is going to include Shift no matter which application is calling this method, and it should be removed (just like Qt does when recording "Ctrl+&" rather than "Ctrl+Shift+&" or "Ctrl+Shift+7".

But anyway in practice the only other caller of this method is kwin, and it only cares for [Ctrl|Alt|None]+[Left|Right|Up|Down|Space|Return|Escape], if I read the code correctly.

See https://lxr.kde.org/ident?_i=xcbKeyPressEventToQt&_remember=1
and AbstractClient::keyPressEvent in kwin.

So, while technically this code could stay in kglobalaccel, it still seems more correct to me to have it here in the generic "xcb key event to keyQt conversion" code (and this avoids calling xcb_key_symbols_alloc again in kglobalaccel).

> graesslin wrote in kkeyserver_x11.h:161
> xkb has the modifier mask defined as uint32_t:
> 
> typedef uint32_t xkb_mod_mask_t

strange, I see this:

typedef struct xcb_key_press_event_t {
[...]

  uint16_t        state; /**<  */

[...]

and `grep modifiers /usr/include/xcb/xproto.h| grep uint` shows consistent use of uint16_t.

xkb_mod_mask_t seems to come from xcbcommon which is unrelated to the APIs used here, right?

REPOSITORY
  R278 KWindowSystem

REVISION DETAIL
  https://phabricator.kde.org/D6233

To: dfaure, graesslin
Cc: graesslin, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170618/f241944f/attachment.html>