D16434: Fix keyboard layout change notifications

2018-11-03 Thread Fabian Vogt
This revision was automatically updated to reflect the committed changes. Closed by commit R268:4d28bd4183d2: Fix keyboard layout change notifications (authored by fvogt). REPOSITORY R268 KGlobalAccel CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16434?vs=44765=44766 REVISION

D16434: Fix keyboard layout change notifications

2018-11-03 Thread Fabian Vogt
fvogt updated this revision to Diff 44765. fvogt marked 2 inline comments as done. fvogt added a comment. This file needs to be reformatted anyway. REPOSITORY R268 KGlobalAccel CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16434?vs=44715=44765 BRANCH master REVISION DETAIL

D16434: Fix keyboard layout change notifications

2018-11-03 Thread Roman Gilg
romangg accepted this revision. romangg added a comment. This revision is now accepted and ready to land. Pls do the coding style improvements and push. INLINE COMMENTS > kglobalaccel_x11.cpp:75 > + const xcb_query_extension_reply_t *reply = >

D16434: Fix keyboard layout change notifications

2018-11-02 Thread Fabian Vogt
fvogt updated this revision to Diff 44715. fvogt added a comment. Use if-else ladder instead of switch-case and fix a typo. REPOSITORY R268 KGlobalAccel CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16434?vs=44714=44715 BRANCH master REVISION DETAIL

D16434: Fix keyboard layout change notifications

2018-11-02 Thread Fabian Vogt
fvogt marked 2 inline comments as done. fvogt added inline comments. INLINE COMMENTS > romangg wrote in kglobalaccel_x11.cpp:96 > Before there was a check to `QX11Info::isPlatformX11()`. Probably we don't > need the check, but did you test on Wayland? The plugin is only loaded if the Qt

D16434: Fix keyboard layout change notifications

2018-11-02 Thread Fabian Vogt
fvogt updated this revision to Diff 44714. fvogt marked an inline comment as done. fvogt added a comment. Add Q_ASSERT and xcb_key_symbols_free. REPOSITORY R268 KGlobalAccel CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16434?vs=44247=44714 BRANCH master REVISION DETAIL

D16434: Fix keyboard layout change notifications

2018-11-02 Thread 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()); > +

D16434: Fix keyboard layout change notifications

2018-10-26 Thread Fabian Vogt
fvogt updated this revision to Diff 44247. fvogt added a comment. Don't use a union. Still works. REPOSITORY R268 KGlobalAccel CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16434?vs=44241=44247 BRANCH master REVISION DETAIL https://phabricator.kde.org/D16434 AFFECTED

D16434: Fix keyboard layout change notifications

2018-10-26 Thread Fabian Vogt
fvogt marked 4 inline comments as done. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D16434 To: fvogt, #frameworks, #plasma Cc: anthonyfieroni, kde-frameworks-devel, michaelh, ngraham, bruns

D16434: Fix keyboard layout change notifications

2018-10-26 Thread Fabian Vogt
fvogt added a comment. Note that the way it's done is ~~copied from~~ inspired by QXcbConnection. I don't like it either though, so I'll try getting rid of the union. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D16434 To: fvogt, #frameworks, #plasma Cc:

D16434: Fix keyboard layout change notifications

2018-10-26 Thread Anthony Fieroni
anthonyfieroni added inline comments. INLINE COMMENTS > kglobalaccel_x11.cpp:198-208 > +typedef union { > + /* All XKB events share these fields. */ > + struct { > + uint8_t response_type; > + uint8_t xkbType; > + uint16_t sequence; > +

D16434: Fix keyboard layout change notifications

2018-10-26 Thread Fabian Vogt
fvogt created this revision. fvogt added reviewers: Frameworks, Plasma. Herald added a project: Frameworks. fvogt requested review of this revision. REVISION SUMMARY This rework fixes several issues: - Qt wasn't informed about XCB_MAPPING_NOTIFY anymore - With XKB enabled in Qt, X won't