graesslin added a comment.
I need to point out that this creates a functional difference to Wayland and
according to the latest rules of Plasma such changes are no longer allowed
unless the implementation is done first for Wayland.
Personally I am not really comfortable with such a large change to X11
nowadays. The code will be pretty much untested till the release and recent
changes showed me that this is not a good idea. Especially I am no longer able
to test the code (that's why I sent a mail to frameworks to step down as
kglobalaccel maintainer, but so far nobody else stepped up).
So personally I am rather inclined to give you a -1 on it as I just think
it's too dangerous. The kglobalaccel code is not good, but it works mostly.
Proper fix will be in Wayland.
INLINE COMMENTS
> kglobalaccel_x11.cpp:278-287
> - if ((keyModQt & Qt::SHIFT) && !KKeyServer::isShiftAsModifierAllowed(
> keyCodeQt ) ) {
> -#ifdef KDEDGLOBALACCEL_TRACE
> - qCDebug(KGLOBALACCELD) << "removing shift modifier";
> -#endif
> - if (keyCodeQt != Qt::Key_Tab) { // KKeySequenceWidget does not map
> shift+tab to backtab
> - static const int FirstLevelShift = 1;
> - keySymX = xcb_key_symbols_get_keysym(m_keySymbols, keyCodeX,
> FirstLevelShift);
The shift handling code has shown regressions whenever it was touched. Also on
Wayland I needed several tries to get it right. I would prefer if it were not
touched any more.
This is not as simple as it looks. There are besties out there like
Alt+Shift+Backtab as a global shortcut and a generic implementation breaks
quickly there. It is quite likely that this change would break Alt+(Shift)+Tab
in KWin.
REPOSITORY
R268 KGlobalAccel
REVISION DETAIL
https://phabricator.kde.org/D6234
To: dfaure, graesslin
Cc: #frameworks
