Merge pull request #855 from freedomofpress/enter-does-not-close-dialog

enter no longer closes dialog

securedrop_client/gui/widgets.py

@@ -27,7 +27,7 @@
 from PyQt5.QtCore import Qt, pyqtSlot, pyqtSignal, QEvent, QTimer, QSize, pyqtBoundSignal, \
     QObject, QPoint
 from PyQt5.QtGui import QIcon, QPalette, QBrush, QColor, QFont, QLinearGradient, QKeySequence, \
-    QCursor
+    QCursor, QKeyEvent, QCloseEvent
 from PyQt5.QtWidgets import QApplication, QListWidget, QLabel, QWidget, QListWidgetItem, \
     QHBoxLayout, QVBoxLayout, QLineEdit, QScrollArea, QDialog, QAction, QMenu, QMessageBox, \
     QToolButton, QSizePolicy, QPlainTextEdit, QStatusBar, QGraphicsDropShadowEffect, QPushButton, \
@@ -2258,7 +2258,6 @@ def __init__(self):
         button_layout = QVBoxLayout()
         window_buttons.setLayout(button_layout)
         self.cancel_button = QPushButton(_('CANCEL'))
-        self.cancel_button.setAutoDefault(False)
         self.cancel_button.clicked.connect(self.close)
         self.continue_button = QPushButton(_('CONTINUE'))
         self.continue_button.setObjectName('primary_button')
@@ -2281,10 +2280,23 @@ def __init__(self):
         layout.addStretch()
         layout.addWidget(window_buttons)
 
-    def closeEvent(self, e):
+    def closeEvent(self, event: QCloseEvent):
         # ignore any close event that doesn't come from our custom close method
         if not self.internal_close_event_emitted:
-            e.ignore()
+            event.ignore()
+
+    def keyPressEvent(self, event: QKeyEvent):
+        # Since the dialog sets the Qt.Popup window flag (in order to achieve a frameless dialog
+        # window in Qubes), the default behavior is to close the dialog when the Enter or Return
+        # key is clicked, which we override here.
+        if (event.key() == Qt.Key_Enter or event.key() == Qt.Key_Return):
+            if self.cancel_button.hasFocus():
+                self.cancel_button.click()
+            else:
+                self.continue_button.click()
+            return
+
+        super().keyPressEvent(event)
 
     def close(self):
         self.internal_close_event_emitted = True

tests/gui/test_widgets.py

@@ -6,7 +6,7 @@
 from datetime import datetime
 
 from PyQt5.QtCore import Qt, QEvent
-from PyQt5.QtGui import QFocusEvent, QMovie
+from PyQt5.QtGui import QFocusEvent, QKeyEvent, QMovie
 from PyQt5.QtTest import QTest
 from PyQt5.QtWidgets import QWidget, QApplication, QVBoxLayout, QMessageBox, QMainWindow, \
     QLineEdit
@@ -1874,7 +1874,6 @@ def test_FramelessDialog_closeEvent_ignored_if_not_a_close_event_from_custom_clo
     mocker.patch(
          'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow())
     dialog = FramelessDialog()
-    dialog.internal_close_event_emitted = False
     close_event = QEvent(QEvent.Close)
     close_event.ignore = mocker.MagicMock()
 
@@ -1883,16 +1882,69 @@ def test_FramelessDialog_closeEvent_ignored_if_not_a_close_event_from_custom_clo
     close_event.ignore.assert_called_once_with()
 
 
+@pytest.mark.parametrize("key", [Qt.Key_Enter, Qt.Key_Return])
+def test_FramelessDialog_keyPressEvent_does_not_close_on_enter_or_return(mocker, key):
+    mocker.patch(
+         'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow())
+    dialog = FramelessDialog()
+    dialog.close = mocker.MagicMock()
+
+    event = QKeyEvent(QEvent.KeyPress, key, Qt.NoModifier)
+    dialog.keyPressEvent(event)
+
+    dialog.close.assert_not_called()
+
+
+@pytest.mark.parametrize("key", [Qt.Key_Enter, Qt.Key_Return])
+def test_FramelessDialog_keyPressEvent_cancel_on_enter_when_focused(mocker, key):
+    mocker.patch(
+         'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow())
+    dialog = FramelessDialog()
+    dialog.cancel_button.click = mocker.MagicMock()
+    dialog.cancel_button.hasFocus = mocker.MagicMock(return_value=True)
+
+    event = QKeyEvent(QEvent.KeyPress, key, Qt.NoModifier)
+    dialog.keyPressEvent(event)
+
+    dialog.cancel_button.click.assert_called_once_with()
+
+
+@pytest.mark.parametrize("key", [Qt.Key_Enter, Qt.Key_Return])
+def test_FramelessDialog_keyPressEvent_continue_on_enter(mocker, key):
+    mocker.patch(
+         'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow())
+    dialog = FramelessDialog()
+    dialog.continue_button.click = mocker.MagicMock()
+
+    event = QKeyEvent(QEvent.KeyPress, key, Qt.NoModifier)
+    dialog.keyPressEvent(event)
+
+    dialog.continue_button.click.assert_called_once_with()
+
+
+@pytest.mark.parametrize("key", [Qt.Key_Alt, Qt.Key_A])
+def test_FramelessDialog_keyPressEvent_does_not_close_for_other_keys(mocker, key):
+    mocker.patch(
+         'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow())
+    dialog = FramelessDialog()
+    dialog.close = mocker.MagicMock()
+
+    event = QKeyEvent(QEvent.KeyPress, key, Qt.NoModifier)
+    dialog.keyPressEvent(event)
+
+    dialog.close.assert_not_called()
+
+
 def test_FramelessDialog_close(mocker):
     mocker.patch(
          'securedrop_client.gui.widgets.QApplication.activeWindow', return_value=QMainWindow())
     dialog = FramelessDialog()
 
-    dialog.internal_close_event_emitted = False
+    assert dialog.internal_close_event_emitted is False
 
     dialog.close()
 
-    dialog.internal_close_event_emitted = True
+    assert dialog.internal_close_event_emitted is True
 
 
 def test_FramelessDialog_center_dialog(mocker):