Merge pull request #2209 from freedomofpress/unique-shortcuts

Verify client shortcuts are unique

client/.semgrep/custom-rules.yml

@@ -180,3 +180,11 @@ rules:
     # Forbid any other use of assert
     - pattern-regex: |
         assert.*
+
+- id: enforce-setshortcut-style
+  patterns:
+    - pattern-not-regex: setShortcut\(Shortcuts\.(.*?)\)
+    - pattern-regex: setShortcut\((.*?)\)
+  message: "Use Shortcuts enum to register shortcuts"
+  languages: [python]
+  severity: WARNING

client/securedrop_client/gui/actions.py

@@ -10,7 +10,7 @@
 from gettext import gettext as _
 from pathlib import Path
 
-from PyQt5.QtCore import Qt, pyqtSlot
+from PyQt5.QtCore import pyqtSlot
 from PyQt5.QtWidgets import QAction, QApplication, QDialog, QMenu
 
 from securedrop_client import state
@@ -20,6 +20,7 @@
 from securedrop_client.gui.base import ModalDialog
 from securedrop_client.gui.conversation import PrintDialog
 from securedrop_client.gui.conversation.export import ExportWizard
+from securedrop_client.gui.shortcuts import Shortcuts
 from securedrop_client.logic import Controller
 from securedrop_client.utils import safe_mkdir
 
@@ -36,7 +37,7 @@ def __init__(
         self._state = app_state
         self._text = _("Download All")
         super().__init__(self._text, parent)
-        self.setShortcut(Qt.CTRL + Qt.Key_D)
+        self.setShortcut(Shortcuts.DOWNLOAD_CONVERSATION.value)
         self.triggered.connect(self.on_triggered)
         self.setShortcutVisibleInContextMenu(True)
 

client/securedrop_client/gui/main.py

@@ -22,12 +22,13 @@
 from gettext import gettext as _
 
 from PyQt5.QtCore import Qt
-from PyQt5.QtGui import QClipboard, QGuiApplication, QIcon, QKeySequence
+from PyQt5.QtGui import QClipboard, QGuiApplication, QIcon
 from PyQt5.QtWidgets import QAction, QApplication, QHBoxLayout, QMainWindow, QVBoxLayout, QWidget
 
 from securedrop_client import __version__, state
 from securedrop_client.db import Source, User
 from securedrop_client.gui.auth import LoginDialog
+from securedrop_client.gui.shortcuts import Shortcuts
 from securedrop_client.gui.widgets import BottomPane, LeftPane, MainView
 from securedrop_client.logic import Controller
 from securedrop_client.resources import load_all_fonts, load_css, load_icon
@@ -93,7 +94,7 @@ def __init__(
         # Actions
         quit = QAction(_("Quit"), self)
         quit.setIcon(QIcon.fromTheme("application-exit"))
-        quit.setShortcut(QKeySequence.Quit)
+        quit.setShortcut(Shortcuts.QUIT.value)
         quit.triggered.connect(self.close)
         self.addAction(quit)
 

client/securedrop_client/gui/shortcuts.py

@@ -0,0 +1,11 @@
+from enum import Enum
+
+from PyQt5.QtCore import Qt
+
+
+class Shortcuts(Enum):
+    """Central listing of all keyboard shortcuts"""
+
+    DOWNLOAD_CONVERSATION = Qt.CTRL + Qt.Key_D
+    QUIT = Qt.CTRL + Qt.Key_Q  # Same as QKeySequence.Quit
+    SEND = Qt.CTRL + Qt.Key_Return

client/securedrop_client/gui/widgets.py

@@ -36,7 +36,6 @@
     QFocusEvent,
     QFont,
     QIcon,
-    QKeySequence,
     QLinearGradient,
     QMouseEvent,
     QPalette,
@@ -84,6 +83,7 @@
 from securedrop_client.gui.base import SecureQLabel, SvgLabel, SvgPushButton, SvgToggleButton
 from securedrop_client.gui.conversation import DeleteConversationDialog
 from securedrop_client.gui.datetime_helpers import format_datetime_local
+from securedrop_client.gui.shortcuts import Shortcuts
 from securedrop_client.gui.source import DeleteSourceDialog
 from securedrop_client.logic import Controller
 from securedrop_client.resources import load_css, load_icon, load_image, load_movie
@@ -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(QKeySequence("Ctrl+Return"))
+        self.send_button.setShortcut(Shortcuts.SEND.value)
         self.send_button.setDefault(True)
 
         # Set cursor.

client/tests/gui/test_shortcuts.py

@@ -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"