Disable hiding window while in MINIMIZED, FULLSCREEN or `PRESENTA…

…TION` state and ignore no-op visibility requests (beeware#3109) Modifies the show() and hide() APIs to be a true no-op if the window is already visible/non-visible respectively, and raise an error if the window is in a minimized, fullscreen or presentation state. This is to avoid ambiguities in the interpretation of show()/hide() under certain conditions. Co-authored-by: Russell Keith-Magee <[email protected]>
proneon267 · Jan 21, 2025 · b1926ee · b1926ee
1 parent fbedd3f
commit b1926ee
Show file tree

Hide file tree

Showing 7 changed files with 129 additions and 25 deletions.
diff --git a/android/src/toga_android/window.py b/android/src/toga_android/window.py
@@ -82,7 +82,10 @@ def set_app(self, app):
         )
         self.set_title(self._initial_title)
 
-    def show(self):
+    def show(self):  # pragma: no cover
+        # The Window on Android is shown by default when the app starts.
+        # Requesting show() on an already shown window is a no-op and is
+        # ignored at the core level. So this method will never be reached.
         pass
 
     ######################################################################

diff --git a/changes/3109.removal.rst b/changes/3109.removal.rst
@@ -0,0 +1 @@
+The `show()` and `hide()` APIs can no longer be used on a window while it is in a `MINIMIZED`, `FULLSCREEN` or `PRESENTATION` state.
diff --git a/core/src/toga/window.py b/core/src/toga/window.py
@@ -311,8 +311,20 @@ def closed(self) -> bool:
 
     def show(self) -> None:
         """Show the window. If the window is already visible, this method has no
-        effect."""
-        self._impl.show()
+        effect.
+
+        :raises ValueError: If the window is currently in a minimized, full screen or
+            presentation state.
+        """
+        if not self.visible:
+            if self.state in {
+                WindowState.MINIMIZED,
+                WindowState.FULLSCREEN,
+                WindowState.PRESENTATION,
+            }:
+                raise ValueError(f"A window in {self.state} state cannot be shown.")
+            else:
+                self._impl.show()
 
     ######################################################################
     # Window content and resources
@@ -436,8 +448,20 @@ def screen_position(self, position: PositionT) -> None:
 
     def hide(self) -> None:
         """Hide the window. If the window is already hidden, this method has no
-        effect."""
-        self._impl.hide()
+        effect.
+
+        :raises ValueError: If the window is currently in a minimized, full screen or
+            presentation state.
+        """
+        if self.visible:
+            if self.state in {
+                WindowState.MINIMIZED,
+                WindowState.FULLSCREEN,
+                WindowState.PRESENTATION,
+            }:
+                raise ValueError(f"A window in {self.state} state cannot be hidden.")
+            else:
+                self._impl.hide()
 
     @property
     def visible(self) -> bool:

diff --git a/core/tests/window/test_window.py b/core/tests/window/test_window.py
@@ -242,45 +242,59 @@ def test_set_size_with_content(window):
 
 
 def test_show_hide(window, app):
-    """The window can be shown and hidden."""
+    """The window can be shown & hidden, but requesting visibility change when
+    the window is already in that requested visibility state is a no-op."""
+    # Window is assigned to the app, but is not visible
     assert window.app == app
+    assert window in app.windows
+    assert not window.visible
+
+    # Show the window
     window.show()
 
     # The window has been assigned to the app, and is visible
     assert window.app == app
     assert window in app.windows
+    assert window.visible
     assert_action_performed(window, "show")
+    EventLog.reset()
+
+    # The window is already shown, so this call will be a no-op
+    window.show()
+
+    # The window is still assigned to the app, and is visible
+    assert window.app == app
+    assert window in app.windows
     assert window.visible
+    assert_action_not_performed(window, "show")
 
-    # Hide with an explicit call
+    # Hide the window
     window.hide()
 
     # Window is still assigned to the app, but is not visible
     assert window.app == app
     assert window in app.windows
-    assert_action_performed(window, "hide")
     assert not window.visible
+    assert_action_performed(window, "hide")
+    EventLog.reset()
 
-
-def test_hide_show(window, app):
-    """The window can be hidden then shown."""
-    assert window.app == app
+    # The window is already hidden, so this call will be a no-op
     window.hide()
 
-    # The window has been assigned to the app, and is not visible
+    # Window is still assigned to the app, but is not visible
     assert window.app == app
     assert window in app.windows
-    assert_action_performed(window, "hide")
     assert not window.visible
+    assert_action_not_performed(window, "hide")
 
-    # Show with an explicit call
+    # Show the window
     window.show()
 
-    # Window is still assigned to the app, but is not visible
+    # The window is still assigned to the app, and is visible
     assert window.app == app
     assert window in app.windows
-    assert_action_performed(window, "show")
     assert window.visible
+    assert_action_performed(window, "show")
 
 
 def test_visibility(window, app):
@@ -304,6 +318,61 @@ def test_visibility(window, app):
     assert not window.visible
 
 
+@pytest.mark.parametrize(
+    "state",
+    [
+        WindowState.MINIMIZED,
+        WindowState.FULLSCREEN,
+        WindowState.PRESENTATION,
+    ],
+)
+def test_show_hide_disallowed_on_window_state(window, app, state):
+    """A window in MINIMIZED, FULLSCREEN or PRESENTATION state cannot be
+    shown or hidden."""
+    window.show()
+
+    window.state = state
+    assert window.state == state
+    assert window.visible is True
+    EventLog.reset()
+
+    with pytest.raises(
+        ValueError,
+        match=f"A window in {state} state cannot be hidden.",
+    ):
+        window.hide()
+        assert_action_not_performed(window, "hide")
+
+    with pytest.raises(
+        ValueError,
+        match=f"A window in {state} state cannot be hidden.",
+    ):
+        window.visible = False
+        assert_action_not_performed(window, "hide")
+
+    # Using only the Toga API, it shouldn't be possible to get a window into a hidden
+    # state while minimized; but if you're poking underlying APIs it might be possible.
+    # It's also good from the point of view of symmetry that the same error conditions
+    # exist. So - fake using "native APIs" to make the window hidden
+    window._impl._visible = False
+    assert window.state == state
+    assert window.visible is False
+
+    with pytest.raises(
+        ValueError,
+        match=f"A window in {state} state cannot be shown.",
+    ):
+        window.show()
+        assert_action_not_performed(window, "show")
+
+    with pytest.raises(
+        ValueError,
+        match=f"A window in {state} state cannot be shown.",
+    ):
+        window.visible = True
+        assert_action_not_performed(window, "show")
+
+
 @pytest.mark.parametrize(
     "initial_state, final_state",
     [

diff --git a/dummy/src/toga_dummy/window.py b/dummy/src/toga_dummy/window.py
@@ -58,6 +58,7 @@ def __init__(self, interface, title, position, size):
         self.set_size(size)
 
         self._state = WindowState.NORMAL
+        self._visible = False
 
     ######################################################################
     # Window properties
@@ -82,7 +83,7 @@ def set_app(self, app):
 
     def show(self):
         self._action("show")
-        self._set_value("visible", True)
+        self._visible = True
 
     ######################################################################
     # Window content and resources
@@ -122,11 +123,14 @@ def set_position(self, position):
     ######################################################################
 
     def get_visible(self):
-        return self._get_value("visible", False)
+        # We cannot store the visibility value on the EventLog, since the value
+        # would be cleared on EventLog.reset(), thereby preventing us from
+        # testing no-op condition of requesting the same visibility as current.
+        return self._visible
 
     def hide(self):
         self._action("hide")
-        self._set_value("visible", False)
+        self._visible = False
 
     ######################################################################
     # Window state

diff --git a/iOS/src/toga_iOS/window.py b/iOS/src/toga_iOS/window.py
@@ -132,8 +132,12 @@ def set_position(self, position):
     ######################################################################
 
     def get_visible(self):
-        # The window is always visible
-        return True
+        # The window is hidden as default by the system, unless makeKeyAndVisible
+        # has been called on the UIWindow. Requesting the same visibility as the
+        # current visibility state is a no-op and is ignored at the core level.
+        # So, always check if the window is currently hidden or not, to ensure that
+        # the other APIs that are dependent on get_visible() work correctly.
+        return not bool(self.native.isHidden())
 
     def hide(self):
         # A no-op, as the window cannot be hidden.

diff --git a/testbed/tests/window/test_window.py b/testbed/tests/window/test_window.py
@@ -917,10 +917,9 @@ async def test_window_state_content_size_increase(
         "state",
         [
             WindowState.NORMAL,
-            WindowState.MINIMIZED,
             WindowState.MAXIMIZED,
-            WindowState.FULLSCREEN,
-            WindowState.PRESENTATION,
+            # Window cannot be hidden while in MINIMIZED, FULLSCREEN or
+            # PRESENTATION. So, those states are excluded from this test.
         ],
     )
     @pytest.mark.parametrize(
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		The `show()` and `hide()` APIs can no longer be used on a window while it is in a `MINIMIZED`, `FULLSCREEN` or `PRESENTATION` state.