Review Request 123508: Shortcuts broken when user sets secondary shortcut

[Lindsay Roberts]

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

(Updated April 27, 2015, 6:01 p.m.)


Review request for KDE Frameworks.


Changes
-------

Clarified summary.


Summary (updated)
-----------------

Shortcuts broken when user sets secondary shortcut


Bugs: 337131, 339243, 340803, and 345411
    https://bugs.kde.org/show_bug.cgi?id=337131
    https://bugs.kde.org/show_bug.cgi?id=339243
    https://bugs.kde.org/show_bug.cgi?id=340803
    https://bugs.kde.org/show_bug.cgi?id=345411


Repository: kxmlgui


Description
-------

When a user configures both a primary and alternate shortcut for an action, they were lost on subsequent load of the app/part .rc file.

These values are persisted in the .rc file as two key sequence strings separated by "; " -- the format understood by `QKeySequence::listFromString`. When these files are reparsed, the current execution flow simply calls `QObject::setProperty("shortcut", semicolonSeparatedString)`, which is `Q_PROPERTY` bound in QAction to `setShortcut(QKeySequence)` -- invoking the (single) QKeySequence constructor. The embedded semicolon and secondary shortcut seem to break this constructor.

There is another specific flow in `KXMLGUIFactoryPrivate::configureAction`; one that specifically calls
    action->setShortcuts(QKeySequence::listFromString(attribute.value()));

but it was conditionalised by: 

    propertyType == QVariant::UserType && action->property(attrName.toLatin1().constData()).userType() == qMetaTypeId<QList<QKeySequence> >()

I have not been able to track down in history when that conditional would've run true for the QAction property `"shortcut"`, QVariant::KeySequence was added Nov 2011, and the conditional has been there since at least the monolithic git split Dec 2013.

Nor have I been able to track down any recent changes on the Qt side that would've implied the QKeySequence string constructor would've worked for multiple shortcuts in the recent past.

In any case, the fix is simply to add a second conditional - matching on QVariant::KeySequence.


Diffs
-----

  src/kxmlguifactory.cpp aeeb242 

Diff: https://git.reviewboard.kde.org/r/123508/diff/


Testing
-------

Primarily using trunk Konsole - saving/loading/using multiple/single shortcuts.


Thanks,

Lindsay Roberts

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