Fix double error dialogs (#1734)

* block text signals in update_object_on_scroll.  Note that scroll signals are already blocked in update_object_on_enter

* block signals when setting value ini update_object_on_enter.  If an error occurs error dialog can steal focus and cause a second editingFinsished signal to be emitted

* accidentally committed all my print statements

* add news fragment

* Update and rename 1733.bugfix.rst to 1734.bugfix.rst

* add one unittest

* make the proper fix, add regression test, and unskip old (very similar test) on macOS

* no longer need is_macos

docs/releases/upcoming/1734.bugfix.rst

@@ -0,0 +1 @@
+Fix double error dialogs (#1734)

traitsui/qt4/range_editor.py

@@ -168,13 +168,16 @@ def init(self, parent):
     def update_object_on_scroll(self, pos):
         """Handles the user changing the current slider value."""
         value = self._convert_from_slider(pos)
-        self.control.text.setText(self.string_value(value))
+        blocked = self.control.text.blockSignals(True)
         try:
             self.value = value
-        except Exception as exc:
+            self.control.text.setText(self.string_value(value))
+        except TraitError as exc:
             from traitsui.api import raise_to_debug
 
             raise_to_debug()
+        finally:
+            self.control.text.blockSignals(blocked)
 
     def update_object_on_enter(self):
         """Handles the user pressing the Enter key in the text field."""
@@ -184,20 +187,24 @@ def update_object_on_enter(self):
             return
 
         try:
-            try:
-                value = eval(str(self.control.text.text()).strip())
-            except Exception as ex:
-                # The entered something that didn't eval as a number, (e.g.,
-                # 'foo') pretend it didn't happen
-                value = self.value
-                self.control.text.setText(str(value))
-                # for compound editor, value may be non-numeric
-                if not isinstance(value, (int, float)):
-                    return
-
-            if not self.factory.is_float:
-                value = int(value)
+            value = eval(str(self.control.text.text()).strip())
+        except Exception as ex:
+            # They entered something that didn't eval as a number, (e.g.,
+            # 'foo') pretend it didn't happen
+            value = self.value
+            self.control.text.setText(str(value))
+            # for compound editor, value may be non-numeric
+            if not isinstance(value, (int, float)):
+                return
 
+        if not self.factory.is_float:
+            value = int(value)
+
+        # If setting the value yields an error, the resulting error dialog
+        # stealing focus could trigger another editingFinished signal so we
+        # block signals here
+        blocked = self.control.text.blockSignals(True)
+        try:
             self.value = value
             blocked = self.control.slider.blockSignals(True)
             try:
@@ -206,8 +213,12 @@ def update_object_on_enter(self):
                 )
             finally:
                 self.control.slider.blockSignals(blocked)
-        except TraitError as excp:
-            pass
+        except TraitError:
+            # They entered something invalid, pretend it didn't happen
+            value = self.value
+            self.control.text.setText(str(value))
+        finally:
+            self.control.text.blockSignals(blocked)
 
     def update_editor(self):
         """Updates the editor when the object trait changes externally to the
@@ -224,7 +235,6 @@ def update_editor(self):
             value = low
 
         ivalue = self._convert_to_slider(value)
-
         self.control.text.setText(text)
 
         blocked = self.control.slider.blockSignals(True)

traitsui/tests/editors/test_range_editor.py

@@ -11,7 +11,9 @@
 import platform
 import unittest
 
-from traits.api import HasTraits, Float, Int
+from pyface.constant import OK
+from pyface.toolkit import toolkit_object
+from traits.api import HasTraits, Float, Int, Range
 from traitsui.api import Item, RangeEditor, UItem, View
 from traitsui.testing.api import (
     DisplayedText,
@@ -29,6 +31,10 @@
     ToolkitName,
 )
 
+ModalDialogTester = toolkit_object(
+    "util.modal_dialog_tester:ModalDialogTester"
+)
+
 is_windows = platform.system() == "Windows"
 
 
@@ -64,6 +70,10 @@ class RangeModel(HasTraits):
     float_value = Float(0.1)
 
 
+class RangeExcludeLow(HasTraits):
+    x = Range(low=0.0, high=1.0, value=0.1, exclude_low=True)
+
+
 @requires_toolkit([ToolkitName.qt, ToolkitName.wx])
 class TestRangeEditor(BaseTestMixin, unittest.TestCase):
     def setUp(self):
@@ -381,3 +391,43 @@ def test_set_text_out_of_range(self):
             float_value_field.perform(KeyClick("Enter"))
 
             self.assertTrue(0.0 <= model.float_value <= 1)
+
+    # regression test for enthought/traitsui#1550
+    @requires_toolkit([ToolkitName.qt])
+    def test_modify_out_of_range(self):
+        obj = RangeExcludeLow()
+        tester = UITester(auto_process_events=False)
+        with tester.create_ui(obj) as ui:
+            number_field = tester.find_by_name(ui, "x")
+            text = number_field.locate(Textbox())
+
+            # should not fail
+            def set_out_of_range():
+                text.perform(KeyClick("Backspace"))
+                text.perform(KeyClick("0"))
+                text.perform(KeyClick("Enter"))
+
+            mdtester = ModalDialogTester(set_out_of_range)
+            mdtester.open_and_run(lambda x: x.close(accept=True))
+
+    # regression test for enthought/traitsui#1550
+    @requires_toolkit([ToolkitName.qt])
+    def test_modify_out_of_range_with_slider(self):
+        obj = RangeExcludeLow()
+        tester = UITester(auto_process_events=False)
+        with tester.create_ui(obj) as ui:
+            number_field = tester.find_by_name(ui, "x")
+            slider = number_field.locate(Slider())
+
+            # slider values are converted to a [0, 10000] scale.  A single
+            # step is a change of 100 on that scale and a page step is 1000.
+            # Our range in [0, 10] so these correspond to changes of .1 and 1.
+            # Note: when tested manually, the step size seen on OSX and Wx is
+            # different.
+
+            # should not fail
+            def move_slider_out_of_range():
+                slider.perform(KeyClick("Page Down"))
+
+            mdtester = ModalDialogTester(move_slider_out_of_range)
+            mdtester.open_and_run(lambda x: x.click_button(OK))

traitsui/tests/test_editor.py

@@ -33,7 +33,6 @@
 from traitsui.tests._tools import (
     BaseTestMixin,
     GuiTestAssistant,
-    is_mac_os,
     no_gui_test_assistant,
     requires_toolkit,
     ToolkitName,
@@ -940,10 +939,6 @@ def test_sync_value_both(self):
     # regression test for enthought/traitsui#1543
     @requires_toolkit([ToolkitName.qt])
     @unittest.skipIf(no_modal_dialog_tester, "ModalDialogTester unavailable")
-    @unittest.skipIf(
-        is_mac_os,
-        "There is a separate issue on OSX. See enthought/traitsui#1550",
-    )
     def test_editor_error_msg(self):
         from pyface.qt import QtCore, QtGui
 
@@ -957,10 +952,8 @@ class Foo(HasTraits):
             x_range = tester.find_by_name(ui, "x")
             x_range_textbox = x_range.locate(Textbox())
 
-            for _ in range(3):
-                x_range_textbox.perform(KeyClick('Backspace'))
-
-            x_range_textbox.perform(KeySequence('0.0'))
+            x_range_textbox.perform(KeyClick('Backspace'))
+            x_range_textbox.perform(KeySequence('0'))
 
             def trigger_error():
                 x_range_textbox.perform(KeyClick('Enter'))