bpo-6739: IDLE: Check for valid keybinding in config_keys (#2377)

Verify user-entered key sequences by trying to bind them with tk.
Add tests for all 3 validation functions.
Original patch by G Polo.  Tests added by Cheryl Sabella.

3 files changed, 118 insertions, 26 deletions

diff --git a/Lib/idlelib/config_key.py b/Lib/idlelib/config_key.py
index 479d6ad..8d575ec 100644
--- a/Lib/idlelib/config_key.py
+++ b/Lib/idlelib/config_key.py
@@ -3,11 +3,16 @@ Dialog for building Tkinter accelerator key bindings
 """
 from tkinter import *
 from tkinter.ttk import Scrollbar
-import tkinter.messagebox as tkMessageBox
+from tkinter import messagebox
 import string
 import sys
 
+
 class GetKeysDialog(Toplevel):
+
+    # Dialog title for invalid key sequence
+    keyerror_title = 'Key Sequence Error'
+
     def __init__(self, parent, title, action, currentKeySequences,
                  _htest=False, _utest=False):
         """
@@ -54,6 +59,10 @@ class GetKeysDialog(Toplevel):
             self.deiconify() #geometry set, unhide
             self.wait_window()
 
+    def showerror(self, *args, **kwargs):
+        # Make testing easier.  Replace in #30751.
+        messagebox.showerror(*args, **kwargs)
+
     def CreateWidgets(self):
         frameMain = Frame(self,borderwidth=2,relief=SUNKEN)
         frameMain.pack(side=TOP,expand=TRUE,fill=BOTH)
@@ -219,53 +228,70 @@ class GetKeysDialog(Toplevel):
         return key
 
     def OK(self, event=None):
-        if self.advanced or self.KeysOK():  # doesn't check advanced string yet
-            self.result=self.keyString.get()
-            self.destroy()
+        keys = self.keyString.get().strip()
+        if not keys:
+            self.showerror(title=self.keyerror_title, parent=self,
+                           message="No key specified.")
+            return
+        if (self.advanced or self.KeysOK(keys)) and self.bind_ok(keys):
+            self.result = keys
+        self.destroy()
 
     def Cancel(self, event=None):
         self.result=''
         self.destroy()
 
-    def KeysOK(self):
+    def KeysOK(self, keys):
         '''Validity check on user's 'basic' keybinding selection.
 
         Doesn't check the string produced by the advanced dialog because
         'modifiers' isn't set.
 
         '''
-        keys = self.keyString.get()
-        keys.strip()
         finalKey = self.listKeysFinal.get(ANCHOR)
         modifiers = self.GetModifiers()
         # create a key sequence list for overlap check:
         keySequence = keys.split()
         keysOK = False
-        title = 'Key Sequence Error'
-        if not keys:
-            tkMessageBox.showerror(title=title, parent=self,
-                                   message='No keys specified.')
-        elif not keys.endswith('>'):
-            tkMessageBox.showerror(title=title, parent=self,
-                                   message='Missing the final Key')
+        title = self.keyerror_title
+        if not keys.endswith('>'):
+            self.showerror(title, parent=self,
+                           message='Missing the final Key')
         elif (not modifiers
               and finalKey not in self.functionKeys + self.moveKeys):
-            tkMessageBox.showerror(title=title, parent=self,
-                                   message='No modifier key(s) specified.')
+            self.showerror(title=title, parent=self,
+                           message='No modifier key(s) specified.')
         elif (modifiers == ['Shift']) \
                  and (finalKey not in
                       self.functionKeys + self.moveKeys + ('Tab', 'Space')):
             msg = 'The shift modifier by itself may not be used with'\
                   ' this key symbol.'
-            tkMessageBox.showerror(title=title, parent=self, message=msg)
+            self.showerror(title=title, parent=self, message=msg)
         elif keySequence in self.currentKeySequences:
             msg = 'This key combination is already in use.'
-            tkMessageBox.showerror(title=title, parent=self, message=msg)
+            self.showerror(title=title, parent=self, message=msg)
         else:
             keysOK = True
         return keysOK
 
+    def bind_ok(self, keys):
+        "Return True if Tcl accepts the new keys else show message."
+
+        try:
+            binding = self.bind(keys, lambda: None)
+        except TclError as err:
+            self.showerror(
+                    title=self.keyerror_title, parent=self,
+                    message=(f'The entered key sequence is not accepted.\n\n'
+                             f'Error: {err}'))
+            return False
+        else:
+            self.unbind(keys, binding)
+            return True
+
 
 if __name__ == '__main__':
+    import unittest
+    unittest.main('idlelib.idle_test.test_config_key', verbosity=2, exit=False)
     from idlelib.idle_test.htest import run
     run(GetKeysDialog)
diff --git a/Lib/idlelib/idle_test/test_config_key.py b/Lib/idlelib/idle_test/test_config_key.py
index 8a24c96..a4227ed 100644
--- a/Lib/idlelib/idle_test/test_config_key.py
+++ b/Lib/idlelib/idle_test/test_config_key.py
@@ -4,29 +4,92 @@ Coverage: 56% from creating and closing dialog.
 '''
 from idlelib import config_key
 from test.support import requires
-requires('gui')
+import sys
 import unittest
 from tkinter import Tk
+from idlelib.idle_test.mock_idle import Func
+from idlelib.idle_test.mock_tk import Var, Mbox_func
 
 
-class GetKeysTest(unittest.TestCase):
+class ValidationTest(unittest.TestCase):
+    "Test validation methods: OK, KeysOK, bind_ok."
+
+    class Validator(config_key.GetKeysDialog):
+        def __init__(self, *args, **kwargs):
+            config_key.GetKeysDialog.__init__(self, *args, **kwargs)
+            class listKeysFinal:
+                get = Func()
+            self.listKeysFinal = listKeysFinal
+        GetModifiers = Func()
+        showerror = Mbox_func()
 
     @classmethod
     def setUpClass(cls):
+        requires('gui')
         cls.root = Tk()
         cls.root.withdraw()
+        cls.dialog = cls.Validator(
+            cls.root, 'Title', '<<Test>>', [['<Key-F12>']], _utest=True)
 
     @classmethod
     def tearDownClass(cls):
-        cls.root.update()  # Stop "can't run event command" warning.
+        cls.dialog.Cancel()
+        cls.root.update_idletasks()
         cls.root.destroy()
-        del cls.root
+        del cls.dialog, cls.root
+
+    def setUp(self):
+        self.dialog.showerror.message = ''
+    # A test that needs a particular final key value should set it.
+    # A test that sets a non-blank modifier list should reset it to [].
+
+    def test_ok_empty(self):
+        self.dialog.keyString.set(' ')
+        self.dialog.OK()
+        self.assertEqual(self.dialog.result, '')
+        self.assertEqual(self.dialog.showerror.message, 'No key specified.')
+
+    def test_ok_good(self):
+        self.dialog.keyString.set('<Key-F11>')
+        self.dialog.listKeysFinal.get.result = 'F11'
+        self.dialog.OK()
+        self.assertEqual(self.dialog.result, '<Key-F11>')
+        self.assertEqual(self.dialog.showerror.message, '')
+
+    def test_keys_no_ending(self):
+        self.assertFalse(self.dialog.KeysOK('<Control-Shift'))
+        self.assertIn('Missing the final', self.dialog.showerror.message)
+
+    def test_keys_no_modifier_bad(self):
+        self.dialog.listKeysFinal.get.result = 'A'
+        self.assertFalse(self.dialog.KeysOK('<Key-A>'))
+        self.assertIn('No modifier', self.dialog.showerror.message)
+
+    def test_keys_no_modifier_ok(self):
+        self.dialog.listKeysFinal.get.result = 'F11'
+        self.assertTrue(self.dialog.KeysOK('<Key-F11>'))
+        self.assertEqual(self.dialog.showerror.message, '')
+
+    def test_keys_shift_bad(self):
+        self.dialog.listKeysFinal.get.result = 'a'
+        self.dialog.GetModifiers.result = ['Shift']
+        self.assertFalse(self.dialog.KeysOK('<a>'))
+        self.assertIn('shift modifier', self.dialog.showerror.message)
+        self.dialog.GetModifiers.result = []
+
+    def test_keys_dup(self):
+        self.dialog.listKeysFinal.get.result = 'F12'
+        self.dialog.GetModifiers.result = []
+        self.assertFalse(self.dialog.KeysOK('<Key-F12>'))
+        self.assertIn('already in use', self.dialog.showerror.message)
 
+    def test_bind_ok(self):
+        self.assertTrue(self.dialog.bind_ok('<Control-Shift-Key-a>'))
+        self.assertEqual(self.dialog.showerror.message, '')
 
-    def test_init(self):
-        dia = config_key.GetKeysDialog(
-            self.root, 'test', '<<Test>>', ['<Key-F12>'], _utest=True)
-        dia.Cancel()
+    def test_bind_not_ok(self):
+        self.assertFalse(self.dialog.bind_ok('<Control-Shift>'))
+        self.assertIn('not accepted', self.dialog.showerror.message)
 
 
 if __name__ == '__main__':
diff --git a/Misc/NEWS.d/next/IDLE/2017-06-26-00-28-59.bpo-6739.x5MfhB.rst b/Misc/NEWS.d/next/IDLE/2017-06-26-00-28-59.bpo-6739.x5MfhB.rst
new file mode 100644
index 0000000..fee904d
--- /dev/null
+++ b/Misc/NEWS.d/next/IDLE/2017-06-26-00-28-59.bpo-6739.x5MfhB.rst
@@ -0,0 +1,3 @@
+IDLE: Verify user-entered key sequences by trying to bind them with tk. Add
+tests for all 3 validation functions. Original patch by G Polo.  Tests added
+by Cheryl Sabella.