----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123508/#review79611 -----------------------------------------------------------
Could you make a testcase for this? - Mark Gaiser On apr 27, 2015, 6:01 p.m., Lindsay Roberts wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123508/ > ----------------------------------------------------------- > > (Updated apr 27, 2015, 6:01 p.m.) > > > Review request for KDE Frameworks. > > > 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
