Make sure keys with modifiers get handled as special

3 files changed, 31 insertions, 9 deletions

diff --git a/qutebrowser/keyinput/keyutils.py b/qutebrowser/keyinput/keyutils.py
index dfe4fb3ff..a56efeab8 100644
--- a/qutebrowser/keyinput/keyutils.py
+++ b/qutebrowser/keyinput/keyutils.py
@@ -48,11 +48,19 @@ def _assert_plain_modifier(key):
     assert not key & ~Qt.KeyboardModifierMask, hex(key)
 
 
-def is_printable(key):
+def _is_printable(key):
     _assert_plain_key(key)
     return key <= 0xff and key not in [Qt.Key_Space, 0x0]
 
 
+def is_special(key, modifiers):
+    """Check whether this key requires special key syntax."""
+    _assert_plain_key(key)
+    _assert_plain_modifier(modifiers)
+    return not (_is_printable(key) and
+                modifiers in [Qt.ShiftModifier, Qt.NoModifier])
+
+
 def is_modifier_key(key):
     """Test whether the given key is a modifier.
 
@@ -277,7 +285,7 @@ class KeyInfo:
         if self.key in _MODIFIER_MAP:
             # Don't return e.g. <Shift+Shift>
             modifiers &= ~_MODIFIER_MAP[self.key]
-        elif is_printable(self.key):
+        elif _is_printable(self.key):
             # "normal" binding
             if not key_string:  # pragma: no cover
                 raise ValueError("Got empty string for key 0x{:x}!"
@@ -285,14 +293,17 @@ class KeyInfo:
 
             assert len(key_string) == 1, key_string
             if self.modifiers == Qt.ShiftModifier:
+                assert not is_special(self.key, self.modifiers)
                 return key_string.upper()
             elif self.modifiers == Qt.NoModifier:
+                assert not is_special(self.key, self.modifiers)
                 return key_string.lower()
             else:
                 # Use special binding syntax, but <Ctrl-a> instead of <Ctrl-A>
                 key_string = key_string.lower()
 
         # "special" binding
+        assert is_special(self.key, self.modifiers)
         modifier_string = _modifiers_to_string(modifiers)
         return '<{}{}>'.format(modifier_string, key_string)
 
@@ -309,7 +320,7 @@ class KeyInfo:
 
         if self.key in control:
             return control[self.key]
-        elif not is_printable(self.key):
+        elif not _is_printable(self.key):
             return ''
 
         text = QKeySequence(self.key).toString()
@@ -490,7 +501,7 @@ class KeySequence:
         # In addition, Shift also *is* relevant when other modifiers are
         # involved. Shift-Ctrl-X should not be equivalent to Ctrl-X.
         if (modifiers == Qt.ShiftModifier and
-                is_printable(ev.key()) and
+                _is_printable(ev.key()) and
                 not ev.text().isupper()):
             modifiers = Qt.KeyboardModifiers()
 
diff --git a/qutebrowser/keyinput/modeparsers.py b/qutebrowser/keyinput/modeparsers.py
index dd9704c87..238fe2722 100644
--- a/qutebrowser/keyinput/modeparsers.py
+++ b/qutebrowser/keyinput/modeparsers.py
@@ -264,8 +264,7 @@ class HintKeyParser(CommandKeyParser):
         if dry_run:
             return dry_run_match
 
-        if (not keyutils.is_printable(e.key()) and
-                dry_run_match == QKeySequence.NoMatch):
+        if keyutils.is_special(e.key(), e.modifiers()):
             log.keyboard.debug("Got special key, clearing keychain")
             self.clear_keystring()
 
@@ -346,7 +345,7 @@ class RegisterKeyParser(CommandKeyParser):
         if match or dry_run:
             return match
 
-        if not keyutils.is_printable(e.key()):
+        if keyutils.is_special(e.key(), e.modifiers()):
             # this is not a proper register key, let it pass and keep going
             return QKeySequence.NoMatch
 
diff --git a/tests/unit/keyinput/test_keyutils.py b/tests/unit/keyinput/test_keyutils.py
index 0557b3c3f..0bc78ca12 100644
--- a/tests/unit/keyinput/test_keyutils.py
+++ b/tests/unit/keyinput/test_keyutils.py
@@ -469,7 +469,19 @@ def test_key_info_to_int():
     (Qt.Key_X, True),
 ])
 def test_is_printable(key, printable):
-    assert keyutils.is_printable(key) == printable
+    assert keyutils._is_printable(key) == printable
+    assert keyutils.is_special(key, Qt.NoModifier) != printable
+
+
+@pytest.mark.parametrize('key, modifiers, special', [
+    (Qt.Key_Escape, Qt.NoModifier, True),
+    (Qt.Key_Escape, Qt.ShiftModifier, True),
+    (Qt.Key_Escape, Qt.ControlModifier, True),
+    (Qt.Key_X, Qt.ControlModifier, True),
+    (Qt.Key_X, Qt.NoModifier, False),
+])
+def test_is_special(key, modifiers, special):
+    assert keyutils.is_special(key, modifiers) == special
 
 
 @pytest.mark.parametrize('key, ismodifier', [
@@ -484,7 +496,7 @@ def test_is_modifier_key(key, ismodifier):
 @pytest.mark.parametrize('func', [
     keyutils._assert_plain_key,
     keyutils._assert_plain_modifier,
-    keyutils.is_printable,
+    keyutils._is_printable,
     keyutils.is_modifier_key,
     keyutils._key_to_string,
     keyutils._modifiers_to_string,