Review Request 123376: Fix handling of mod-shift-number shortcuts.

[Thomas Lübking]

> On Aug. 12, 2015, 10:17 vorm., Thomas Lübking wrote:
> > bump, we should get this fixed for 5.4
> > 
> > Shui, do you want to work on improving the patch and adding the testcase?
> 
> Thomas Lübking wrote:
>     -100
>     THE PATCH BREAKS BACKTABBING!!
>     
>     Btw. entirely not figured by autotests :-P
>     
>     @Martin
>     test might be quite a problem since this
>     ```diff
>     diff --git a/autotests/kglobalshortcuttest.cpp b/autotests/kglobalshortcuttest.cpp
>     index 30c932c..92c3f86 100644
>     --- a/autotests/kglobalshortcuttest.cpp
>     +++ b/autotests/kglobalshortcuttest.cpp
>     @@ -37,6 +37,7 @@
>      #include <QX11Info>
>      #define XK_MISCELLANY
>      #define XK_XKB_KEYS
>     +#define XK_LATIN1
>      #include <X11/keysymdef.h>
>      #include <xcb/xcb_keysyms.h>
>      #include <xcb/xtest.h>
>     @@ -51,7 +52,7 @@
>       *
>      */
>      
>     -const QKeySequence sequenceA = QKeySequence(Qt::SHIFT + Qt::META + Qt::CTRL + Qt::ALT + Qt::Key_F12);
>     +const QKeySequence sequenceA = QKeySequence(Qt::SHIFT + Qt::META + Qt::CTRL + Qt::ALT + Qt::Key_0/*Qt::Key_F12*/);
>      const QKeySequence sequenceB = QKeySequence(Qt::Key_F29);
>      const QKeySequence sequenceC = QKeySequence(Qt::SHIFT + Qt::META + Qt::CTRL + Qt::Key_F28);
>      const QKeySequence sequenceD = QKeySequence(Qt::META + Qt::ALT + Qt::Key_F30);
>     @@ -175,15 +176,16 @@ void KGlobalShortcutTest::testActivateShortcut()
>          const xcb_keycode_t control = getCode(XK_Control_L);
>          const xcb_keycode_t alt = getCode(XK_Alt_L);
>          const xcb_keycode_t f12 = getCode(XK_F12);
>     +    const xcb_keycode_t _0 = getCode(XK_0);
>          xcb_key_symbols_free(syms);
>      
>          xcb_test_fake_input(c, XCB_KEY_PRESS, meta,    XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>          xcb_test_fake_input(c, XCB_KEY_PRESS, control, XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>          xcb_test_fake_input(c, XCB_KEY_PRESS, alt,     XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>          xcb_test_fake_input(c, XCB_KEY_PRESS, shift,   XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>     -    xcb_test_fake_input(c, XCB_KEY_PRESS, f12,     XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>     +    xcb_test_fake_input(c, XCB_KEY_PRESS, _0,     XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>      
>     -    xcb_test_fake_input(c, XCB_KEY_RELEASE, f12,     XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>     +    xcb_test_fake_input(c, XCB_KEY_RELEASE, _0,     XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>          xcb_test_fake_input(c, XCB_KEY_RELEASE, shift,   XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>          xcb_test_fake_input(c, XCB_KEY_RELEASE, meta,    XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>          xcb_test_fake_input(c, XCB_KEY_RELEASE, control, XCB_TIME_CURRENT_TIME, w, 0, 0, 0);
>     @@ -191,6 +193,7 @@ void KGlobalShortcutTest::testActivateShortcut()
>          xcb_flush(c);
>      
>          QVERIFY(actionASpy.wait());
>     +
>          QCOMPARE(actionASpy.count(), 1);
>      #else
>          QSKIP("This test requires to be compiled with XCB-XTEST");
>     diff --git a/src/runtime/plugins/xcb/kglobalaccel_x11.cpp b/src/
>     ```
>     
>     simple modification already fails on the trigger test, despite the shortcut allocation on the live system works.
>     The problem is that the registered key sequence is actually Qt::META + Qt::CTRL + Qt::ALT + Qt::Key_Equal, and the bigger problem is, that this is because of a german keyboard layout ...
>     
>     I assume we must fake in a key press and read the generated event (iow, clone the shortcut editor behavior) for this.
>     
>     However: moot point. This patch breaks backtabbing, so can NO WAY GO IN!

Weirdness may be in KKEyServer::isShiftAsModifierAllowed() which allows Backtab, but not Tab.


kkeysequencewidget.cpp however has special handling (which kglobaccel would have to adapt, but this is a total mess)
```
               if ((keyQt == Qt::Key_Backtab) && (d->modifierKeys & Qt::SHIFT)) {
0758                 keyQt = Qt::Key_Tab | d->modifierKeys;
0759             } else if (KKeyServer::isShiftAsModifierAllowed(keyQt)) {
0760                 keyQt |= d->modifierKeys;
```


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123376/#review83734
-----------------------------------------------------------


On April 25, 2015, 7:54 nachm., Yuxuan Shui wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123376/
> -----------------------------------------------------------
> 
> (Updated April 25, 2015, 7:54 nachm.)
> 
> 
> Review request for KDE Frameworks, Martin Gräßlin and Thomas Lübking.
> 
> 
> Bugs: 341959
>     https://bugs.kde.org/show_bug.cgi?id=341959
> 
> 
> Repository: kglobalaccel
> 
> 
> Description
> -------
> 
> For details, see discussion in corresponding bug report.
> 
> 
> Diffs
> -----
> 
>   src/runtime/kglobalaccel_x11.cpp abee5bc 
> 
> Diff: https://git.reviewboard.kde.org/r/123376/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Yuxuan Shui
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150815/271bf241/attachment-0001.html>