Fix SIGSEGV when introducing a new shortcut (#9869)

removeShortcut() restores default settings, therefore was used incorrectly. I
introduce deactivateShortcuts() which only removes assignments.

Clean up a bit the lack of view / model distinction (getting rid of the crashing
code at the same time).

Repair inconsistency of the selection in the "modify" case. (regression at
717d19d3c)

Make the test for existing bindings a bit more robust. (Not perfect yet.)

Focus on the item that has just been added/modified. (cosmetic)

(cherry-picked from 20a9c7a)
This commit is contained in:
Guillaume Munch 2016-02-22 00:36:33 +00:00
parent dc6bc435a4
commit bc858ab69d
2 changed files with 48 additions and 17 deletions

View File

@ -3095,6 +3095,38 @@ void PrefShortcuts::removeShortcut()
} }
void PrefShortcuts::deactivateShortcuts(QList<QTreeWidgetItem*> const & items)
{
for (int i = 0; i < items.size(); ++i) {
string shortcut = fromqstr(items[i]->data(1, Qt::UserRole).toString());
string lfun = fromqstr(items[i]->text(0));
FuncRequest func = lyxaction.lookupFunc(lfun);
KeyMap::ItemType tag =
static_cast<KeyMap::ItemType>(items[i]->data(0, Qt::UserRole).toInt());
switch (tag) {
case KeyMap::System:
// for system bind, we do not touch the item
// but add an user unbind item
user_unbind_.bind(shortcut, func);
setItemType(items[i], KeyMap::UserUnbind);
break;
case KeyMap::UserBind: {
// for user_bind, we remove this bind
QTreeWidgetItem * parent = items[i]->parent();
int itemIdx = parent->indexOfChild(items[i]);
parent->takeChild(itemIdx);
user_bind_.unbind(shortcut, func);
break;
}
default:
break;
}
}
}
void PrefShortcuts::selectBind() void PrefShortcuts::selectBind()
{ {
QString file = form_->browsebind(internalPath(bindFileED->text())); QString file = form_->browsebind(internalPath(bindFileED->text()));
@ -3192,9 +3224,12 @@ void PrefShortcuts::shortcutOkPressed()
return; return;
// make sure this key isn't already bound---and, if so, prompt user // make sure this key isn't already bound---and, if so, prompt user
FuncCode const old_action = oldBinding.action();
FuncCode const unbind = user_unbind_.getBinding(k).action(); FuncCode const unbind = user_unbind_.getBinding(k).action();
docstring const action_string = makeCmdString(oldBinding); docstring const action_string = makeCmdString(oldBinding);
if (oldBinding.action() > LFUN_NOACTION && unbind == LFUN_UNKNOWN_ACTION // FIXME: this logic is wrong. Some bindings in the treewidget can (still)
// escape from this test.
if (old_action > LFUN_NOACTION && unbind != old_action
&& save_lfun_ != toqstr(action_string)) { && save_lfun_ != toqstr(action_string)) {
docstring const new_action_string = makeCmdString(func); docstring const new_action_string = makeCmdString(func);
docstring const text = bformat(_("Shortcut `%1$s' is already bound to " docstring const text = bformat(_("Shortcut `%1$s' is already bound to "
@ -3210,31 +3245,24 @@ void PrefShortcuts::shortcutOkPressed()
QString const sequence_text = toqstr(k.print(KeySequence::ForGui)); QString const sequence_text = toqstr(k.print(KeySequence::ForGui));
QList<QTreeWidgetItem*> items = shortcutsTW->findItems(sequence_text, QList<QTreeWidgetItem*> items = shortcutsTW->findItems(sequence_text,
Qt::MatchFlags(Qt::MatchExactly | Qt::MatchRecursive), 1); Qt::MatchFlags(Qt::MatchExactly | Qt::MatchRecursive), 1);
if (items.size() > 0) { deactivateShortcuts(items);
// should always happen }
bool expanded = items[0]->parent()->isExpanded();
shortcutsTW->setCurrentItem(items[0]); if (!save_lfun_.isEmpty()) {
removeShortcut(); // real modification of the lfun's shortcut,
shortcutsTW->setCurrentItem(0); // so remove the previous one
// make sure user doesn't see tree expansion if QList<QTreeWidgetItem*> to_modify = shortcutsTW->selectedItems();
// old binding wasn't in an expanded tree deactivateShortcuts(to_modify);
if (!expanded)
items[0]->parent()->setExpanded(false);
}
} }
shortcut_->accept(); shortcut_->accept();
if (!save_lfun_.isEmpty())
// real modification of the lfun's shortcut,
// so remove the previous one
removeShortcut();
QTreeWidgetItem * item = insertShortcutItem(func, k, KeyMap::UserBind); QTreeWidgetItem * item = insertShortcutItem(func, k, KeyMap::UserBind);
if (item) { if (item) {
user_bind_.bind(&k, func); user_bind_.bind(&k, func);
shortcutsTW->sortItems(0, Qt::AscendingOrder); shortcutsTW->sortItems(0, Qt::AscendingOrder);
shortcutsTW->setItemExpanded(item->parent(), true); shortcutsTW->setItemExpanded(item->parent(), true);
shortcutsTW->setCurrentItem(item);
shortcutsTW->scrollToItem(item); shortcutsTW->scrollToItem(item);
} else { } else {
Alert::error(_("Failed to create shortcut"), Alert::error(_("Failed to create shortcut"),

View File

@ -465,7 +465,10 @@ public:
void update(LyXRC const & rc); void update(LyXRC const & rc);
void updateShortcutsTW(); void updateShortcutsTW();
void modifyShortcut(); void modifyShortcut();
/// remove selected binding, restore default value
void removeShortcut(); void removeShortcut();
/// remove bindings, do not restore default values
void deactivateShortcuts(QList<QTreeWidgetItem*> const & items);
/// ///
void setItemType(QTreeWidgetItem * item, KeyMap::ItemType tag); void setItemType(QTreeWidgetItem * item, KeyMap::ItemType tag);
QTreeWidgetItem * insertShortcutItem(FuncRequest const & lfun, QTreeWidgetItem * insertShortcutItem(FuncRequest const & lfun,