Skip to content

Commit

Permalink
Verify client shortcuts are unique
Browse files Browse the repository at this point in the history
If you have two shortcuts that point to the same key combo, you'll get
"QAction::event: Ambiguous shortcut overload: <sequence>" in the logs
and neither action will be taken.

Centralize them in one place so it's easy for humans to spot duplicates.
A test verifies uniqueness as well.

A semgrep rule enforces that people use the new Shortcuts enum instead
of just writing the sequence in the function call directly.
  • Loading branch information
legoktm committed Sep 24, 2024
1 parent 79df813 commit 63228b3
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 5 deletions.
2 changes: 1 addition & 1 deletion client/securedrop_client/gui/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def __init__(
self._state = app_state
self._text = _("Download All")
super().__init__(self._text, parent)
self.setShortcut(Shortcuts.DOWNLOAD_CONVERSATION)
self.setShortcut(Shortcuts.DOWNLOAD_CONVERSATION.value)
self.triggered.connect(self.on_triggered)
self.setShortcutVisibleInContextMenu(True)

Expand Down
2 changes: 1 addition & 1 deletion client/securedrop_client/gui/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def __init__(
# Actions
quit = QAction(_("Quit"), self)
quit.setIcon(QIcon.fromTheme("application-exit"))
quit.setShortcut(Shortcuts.QUIT)
quit.setShortcut(Shortcuts.QUIT.value)
quit.triggered.connect(self.close)
self.addAction(quit)

Expand Down
3 changes: 1 addition & 2 deletions client/securedrop_client/gui/shortcuts.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
from enum import Enum

from PyQt5.QtCore import Qt
from PyQt5.QtGui import QKeySequence


class Shortcuts(Enum):
"""Central listing of all keyboard shortcuts"""

DOWNLOAD_CONVERSATION = Qt.CTRL + Qt.Key_D
QUIT = QKeySequence.Quit # Aliased to Ctrl+Q on all platforms
QUIT = Qt.CTRL + Qt.Key_Q # Same as QKeySequence.Quit
SEND = Qt.CTRL + Qt.Key_Return
2 changes: 1 addition & 1 deletion client/securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -3149,7 +3149,7 @@ def __init__(self, source: Source, controller: Controller) -> None:
send_button_icon.addPixmap(load_image("send-disabled.svg"), QIcon.Disabled)
self.send_button.setIcon(send_button_icon)
self.send_button.setIconSize(QSize(56, 47))
self.send_button.setShortcut(Shortcuts.SEND)
self.send_button.setShortcut(Shortcuts.SEND.value)
self.send_button.setDefault(True)

# Set cursor.
Expand Down
11 changes: 11 additions & 0 deletions client/tests/gui/test_shortcuts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from securedrop_client.gui.shortcuts import Shortcuts


def test_all_values_are_unique():
# Get all the values in the enum
enum_values = []
for value in Shortcuts.__members__.values():
enum_values.append(value.value)

# Check that all values are unique
assert len(enum_values) == len(set(enum_values)), "Enum values are not unique"

0 comments on commit 63228b3

Please sign in to comment.