From eb8c5905f617b31ec470358e0e23120fc586d695 Mon Sep 17 00:00:00 2001 From: Guillaume Munch Date: Mon, 4 Jul 2016 04:23:32 +0200 Subject: [PATCH] Prioritize the shortcuts from the work areas * Fix bug #10261 : KDE smartly adds conflicting accelerators. * Prevent bugs like #9495 in the future. Issues (non-regression): * It does not appear possible to prevent Ubuntu's Unity from grabbing the accelerators for the menus. For instance Alt+A still opens _Affichage in the French localization. --- src/frontends/qt4/GuiApplication.cpp | 42 +++++++++++++++++++++------ src/frontends/qt4/GuiApplication.h | 3 ++ src/frontends/qt4/GuiKeySymbol.cpp | 2 +- src/frontends/qt4/GuiKeySymbol.h | 2 +- src/frontends/qt4/GuiPrefs.cpp | 3 ++ src/frontends/qt4/GuiWorkArea.cpp | 43 ++++++++++++++++++++++------ src/frontends/qt4/GuiWorkArea.h | 5 +++- 7 files changed, 80 insertions(+), 20 deletions(-) diff --git a/src/frontends/qt4/GuiApplication.cpp b/src/frontends/qt4/GuiApplication.cpp index ef945c9e8c..81d5373a70 100644 --- a/src/frontends/qt4/GuiApplication.cpp +++ b/src/frontends/qt4/GuiApplication.cpp @@ -2119,19 +2119,43 @@ void GuiApplication::handleKeyFunc(FuncCode action) } +//Keep this in sync with GuiApplication::processKeySym below +bool GuiApplication::queryKeySym(KeySymbol const & keysym, + KeyModifier state) const +{ + // Do nothing if we have nothing + if (!keysym.isOK() || keysym.isModifier()) + return false; + // Do a one-deep top-level lookup for cancel and meta-fake keys. + KeySequence seq; + FuncRequest func = seq.addkey(keysym, state); + // When not cancel or meta-fake, do the normal lookup. + if ((func.action() != LFUN_CANCEL) && (func.action() != LFUN_META_PREFIX)) { + seq = d->keyseq; + func = seq.addkey(keysym, (state | d->meta_fake_bit)); + } + // Maybe user can only reach the key via holding down shift. + // Let's see. But only if shift is the only modifier + if (func.action() == LFUN_UNKNOWN_ACTION && state == ShiftModifier) + // If addkey looked up a command and did not find further commands then + // seq has been reset at this point + func = seq.addkey(keysym, NoModifier); + + LYXERR(Debug::KEY, " Key (queried) [action=" << func.action() << "][" + << seq.print(KeySequence::Portable) << ']'); + return func.action() != LFUN_UNKNOWN_ACTION; +} + + +//Keep this in sync with GuiApplication::queryKeySym above void GuiApplication::processKeySym(KeySymbol const & keysym, KeyModifier state) { LYXERR(Debug::KEY, "KeySym is " << keysym.getSymbolName()); // Do nothing if we have nothing (JMarc) - if (!keysym.isOK()) { - LYXERR(Debug::KEY, "Empty kbd action (probably composing)"); - if (current_view_) - current_view_->restartCursor(); - return; - } - - if (keysym.isModifier()) { + if (!keysym.isOK() || keysym.isModifier()) { + if (!keysym.isOK()) + LYXERR(Debug::KEY, "Empty kbd action (probably composing)"); if (current_view_) current_view_->restartCursor(); return; @@ -2177,6 +2201,8 @@ void GuiApplication::processKeySym(KeySymbol const & keysym, KeyModifier state) // Let's see. But only if shift is the only modifier if (func.action() == LFUN_UNKNOWN_ACTION && state == ShiftModifier) { LYXERR(Debug::KEY, "Trying without shift"); + // If addkey looked up a command and did not find further commands then + // seq has been reset at this point func = d->keyseq.addkey(keysym, NoModifier); LYXERR(Debug::KEY, "Action now " << func.action()); } diff --git a/src/frontends/qt4/GuiApplication.h b/src/frontends/qt4/GuiApplication.h index 861810daec..de6013d4c7 100644 --- a/src/frontends/qt4/GuiApplication.h +++ b/src/frontends/qt4/GuiApplication.h @@ -165,6 +165,9 @@ public: #endif } + /// return true if the key is part of a shortcut + bool queryKeySym(KeySymbol const & key, KeyModifier state) const; + /// void processKeySym(KeySymbol const & key, KeyModifier state); /// return the status bar state string docstring viewStatusMessage(); diff --git a/src/frontends/qt4/GuiKeySymbol.cpp b/src/frontends/qt4/GuiKeySymbol.cpp index d058bce50e..ef9425deb1 100644 --- a/src/frontends/qt4/GuiKeySymbol.cpp +++ b/src/frontends/qt4/GuiKeySymbol.cpp @@ -612,7 +612,7 @@ static char encode(string const & encoding, QString const & str) #endif -void setKeySymbol(KeySymbol * sym, QKeyEvent * ev) +void setKeySymbol(KeySymbol * sym, QKeyEvent const * ev) { sym->setKey(ev->key()); if (ev->text().isNull()) { diff --git a/src/frontends/qt4/GuiKeySymbol.h b/src/frontends/qt4/GuiKeySymbol.h index d178185a53..e4a59b7f5e 100644 --- a/src/frontends/qt4/GuiKeySymbol.h +++ b/src/frontends/qt4/GuiKeySymbol.h @@ -18,7 +18,7 @@ class QKeyEvent; namespace lyx { /// delayed constructor -void setKeySymbol(KeySymbol * sym, QKeyEvent * ev); +void setKeySymbol(KeySymbol * sym, QKeyEvent const * ev); /// return the LyX key state from Qt's KeyModifier q_key_state(Qt::KeyboardModifiers state); diff --git a/src/frontends/qt4/GuiPrefs.cpp b/src/frontends/qt4/GuiPrefs.cpp index 2f168beee9..0748295ce1 100644 --- a/src/frontends/qt4/GuiPrefs.cpp +++ b/src/frontends/qt4/GuiPrefs.cpp @@ -3227,6 +3227,9 @@ void PrefShortcuts::shortcutOkPressed() shortcutsTW->setCurrentItem(item); shortcutsTW->scrollToItem(item); } else { + // FIXME: The error message could be more explicit. This can happen in + // particular if the user wants to introduce a LFUN which is Hidden such + // as self-insert. Alert::error(_("Failed to create shortcut"), _("Can not insert shortcut to the list")); return; diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index c13beec225..109ea7eed3 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -505,6 +505,14 @@ void GuiWorkArea::redraw(bool update_metrics) } +// Keep in sync with GuiWorkArea::processKeySym below +bool GuiWorkArea::queryKeySym(KeySymbol const & key, KeyModifier mod) const +{ + return guiApp->queryKeySym(key, mod); +} + + +// Keep in sync with GuiWorkArea::queryKeySym above void GuiWorkArea::processKeySym(KeySymbol const & key, KeyModifier mod) { if (d->lyx_view_->isFullScreen() && d->lyx_view_->menuBar()->isVisible() @@ -712,6 +720,12 @@ bool GuiWorkArea::event(QEvent * e) return true; } + case QEvent::ShortcutOverride: + // keyPressEvent is ShortcutOverride-aware and only accepts the event in + // this case + keyPressEvent(static_cast(e)); + return e->isAccepted(); + case QEvent::KeyPress: { // We catch this event in order to catch the Tab or Shift+Tab key press // which are otherwise reserved to focus switching between controls @@ -1028,6 +1042,10 @@ void GuiWorkArea::generateSyntheticMouseEvent() void GuiWorkArea::keyPressEvent(QKeyEvent * ev) { + // this is also called for ShortcutOverride events. In this case, one must + // not act but simply accept the event explicitly. + bool const act = (ev->type() != QEvent::ShortcutOverride); + // Do not process here some keys if dialog_mode_ is set if (d->dialog_mode_ && (ev->modifiers() == Qt::NoModifier @@ -1045,7 +1063,8 @@ void GuiWorkArea::keyPressEvent(QKeyEvent * ev) switch (ev->key()) { case Qt::Key_Enter: case Qt::Key_Return: - d->completer_->activate(); + if (act) + d->completer_->activate(); ev->accept(); return; } @@ -1055,7 +1074,9 @@ void GuiWorkArea::keyPressEvent(QKeyEvent * ev) // (the auto repeated events come too fast) // it looks like this is only needed on X11 #if defined(Q_WS_X11) || defined(QPA_XCB) - if (qApp->hasPendingEvents() && ev->isAutoRepeat()) { + // FIXME: this is a weird way to implement event compression. Also, this is + // broken with IBus. + if (act && qApp->hasPendingEvents() && ev->isAutoRepeat()) { switch (ev->key()) { case Qt::Key_PageDown: case Qt::Key_PageUp: @@ -1070,7 +1091,7 @@ void GuiWorkArea::keyPressEvent(QKeyEvent * ev) } #endif - KeyModifier m = q_key_state(ev->modifiers()); + KeyModifier const m = q_key_state(ev->modifiers()); std::string str; if (m & ShiftModifier) @@ -1081,16 +1102,20 @@ void GuiWorkArea::keyPressEvent(QKeyEvent * ev) str += "Alt-"; if (m & MetaModifier) str += "Meta-"; - - LYXERR(Debug::KEY, " count: " << ev->count() << " text: " << ev->text() - << " isAutoRepeat: " << ev->isAutoRepeat() << " key: " << ev->key() - << " keyState: " << str); + + if (act) + LYXERR(Debug::KEY, " count: " << ev->count() << " text: " << ev->text() + << " isAutoRepeat: " << ev->isAutoRepeat() << " key: " << ev->key() + << " keyState: " << str); KeySymbol sym; setKeySymbol(&sym, ev); if (sym.isOK()) { - processKeySym(sym, q_key_state(ev->modifiers())); - ev->accept(); + if (act) { + processKeySym(sym, m); + ev->accept(); + } else + ev->setAccepted(queryKeySym(sym, m)); } else { ev->ignore(); } diff --git a/src/frontends/qt4/GuiWorkArea.h b/src/frontends/qt4/GuiWorkArea.h index 5039e45b84..df4b0c1177 100644 --- a/src/frontends/qt4/GuiWorkArea.h +++ b/src/frontends/qt4/GuiWorkArea.h @@ -68,6 +68,8 @@ public: /// void redraw(bool update_metrics); + /// return true if the key is part of a shortcut + bool queryKeySym(KeySymbol const & key, KeyModifier mod) const; /// Process Key pressed event. /// This needs to be public because it is accessed externally by GuiView. void processKeySym(KeySymbol const & key, KeyModifier mod); @@ -141,7 +143,8 @@ private: void mouseMoveEvent(QMouseEvent * ev); /// wheel event void wheelEvent(QWheelEvent * ev); - /// key press + /// key press event. It also knows how to handle ShortcutOverride events to + /// avoid code duplication. void keyPressEvent(QKeyEvent * ev); /// IM events void inputMethodEvent(QInputMethodEvent * ev);