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
