D16434: Fix keyboard layout change notifications

[Roman Gilg]

romangg requested changes to this revision.
romangg added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kglobalaccel_x11.cpp:96
>      if (!m_keySymbols) {
> -        return false;
> +        m_keySymbols = xcb_key_symbols_alloc(QX11Info::connection());
> +        if (!m_keySymbols) {

Before there was a check to `QX11Info::isPlatformX11()`. Probably we don't need the check, but did you test on Wayland?

> kglobalaccel_x11.cpp:220
>          default:
> +            if(m_xkb_first_event && responseType == m_xkb_first_event) {
> +                const uint8_t xkbEvent = event->pad0;

case m_xkb_first_event:
      if (m_xkb_first_event) {
          ...
      }
  default:
      return false;

Or directly do the switching on `responseType` with if-else statements.

> kglobalaccel_x11.cpp:264
> +	// Force reloading of the keySym mapping
> +	m_keySymbols = nullptr;
> +

Might leak? Use `xcb_key_symbols_free`.

REPOSITORY
  R268 KGlobalAccel

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

To: fvogt, #frameworks, #plasma, romangg
Cc: romangg, ngraham, anthonyfieroni, kde-frameworks-devel, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20181102/9629ab51/attachment-0001.html>