Re: transient windows on win32

["Charles Reilly" <gtk-devel-list 2005 charlesreilly com>]

Thanks for looking at this.  Sorry about the comment style; I've been
programming in C++ for too long.

I've redone the patch with C-style comments.  I've also added my name to
the copyright section at the top.  I'm not familiar with the etiquette of
this - let me know if it's inappropriate.

> Sorry for not following up on this earlier. I had marked your message
> for later follow-up, but then never got around to... and then noticed
> it only now when switching to October's mail folder.
>
> Charles Reilly writes:
>  > The trivial problem was that gdk_window_set_skip_taskbar_hint set the
>  > window owner.  This overwrote the window owner set by
>  > gdk_window_set_transient_for.  (I guess this was deliberate, since
>  > transients didn't work anyway?)
>
> Hmm. I don't remember exactly. Might have been just an oversight, or
> maybe the use cases I knew of didn't have windows for which both
> _set_transient_for() and _set_skip_taskbar_hint() were called.
>
>  > Any comments would be most welcome.
>
> You seem to have looked into the problem thoroughly, your patch looks
> quite clean (except the C++-style comments), so I do think it has a
> good chance of being in GTK+ 2.8.5. I will need to think and test a
> bit more, though.
>
> --tml


--- cvs\gdkwindow-win32.h	Sun Sep 04 17:33:52 2005
+++ ownerManagement\gdkwindow-win32.h	Sat Sep 03 20:59:39 2005
@@ -68,4 +68,7 @@ struct _GdkWindowImplWin32
 {
   GdkDrawableImplWin32 parent_instance;
+  GdkWindow *owner_window;
+
+  gboolean skips_taskbar;

   gint width;


--- cvs\gdkwindow-win32.c	Sun Sep 04 17:33:31 2005
+++ ownerManagement\gdkwindow-win32.c	Wed Oct 12 14:02:23 2005
@@ -3,4 +3,5 @@
  * Copyright (C) 1998-2004 Tor Lillqvist
  * Copyright (C) 2001-2004 Hans Breuer
+ * Copyright (C) 2005 Charles Reilly
  *
  * This library is free software; you can redistribute it and/or
@@ -85,4 +86,12 @@ static void gdk_window_impl_win32_init
 static void gdk_window_impl_win32_class_init (GdkWindowImplWin32Class
*klass);
 static void gdk_window_impl_win32_finalize   (GObject                
*object);
+static void gdk_window_win32_set_window_ownership(GdkWindow* window,
+							gboolean is_owner_minimized);
+static GdkFilterReturn gdk_window_win32_owner_window_filter (GdkXEvent *xev,
+							GdkEvent  *event,
+							gpointer   data);
+static GdkFilterReturn gdk_window_win32_owned_window_filter (GdkXEvent *xev,
+							GdkEvent  *event,
+							gpointer   data);

 static gpointer parent_class = NULL;
@@ -661,4 +670,8 @@ gdk_window_new_internal (GdkWindow     *
     }

+  /* If the window is not a child then its parent is its owner */
+  if ((dwStyle & WS_CHILDWINDOW) == 0)
+    impl->owner_window = parent;
+
   _gdk_window_init_position (GDK_WINDOW (private));

@@ -885,4 +898,6 @@ void
 gdk_window_destroy_notify (GdkWindow *window)
 {
+  GdkWindowImplWin32 *impl;
+
   g_return_if_fail (window != NULL);
   g_return_if_fail (GDK_IS_WINDOW (window));
@@ -893,4 +908,14 @@ gdk_window_destroy_notify (GdkWindow *wi
 		     (GDK_WINDOW_DESTROYED (window) ? " (destroyed)" : "")));

+  /* Ideally, this would be handled in
gdk_window_win32_owned_window_filter but
+   * the filter list isn't reentrant, so we remove the owner window
filter here
+   */
+  impl = GDK_WINDOW_IMPL_WIN32 (GDK_WINDOW_OBJECT (window)->impl);
+  if (impl->owner_window)
+  {
+    gdk_window_remove_filter(impl->owner_window,
gdk_window_win32_owner_window_filter, window);
+    impl->owner_window = NULL;
+  }
+
   if (!GDK_WINDOW_DESTROYED (window))
     {
@@ -1778,12 +1803,74 @@ gdk_window_set_role (GdkWindow   *window
 }

+static GdkFilterReturn
+gdk_window_win32_owner_window_filter (GdkXEvent *xev,
+		      GdkEvent  *event,
+		      gpointer   data)
+{
+  MSG *msg = (MSG *) xev;
+  /* This is a window owned by the window that received the message */
+  GdkWindow *window = (GdkWindow*)data;
+  GdkWindowImplWin32 *impl;
+
+  g_return_if_fail (GDK_IS_WINDOW (window));
+
+  if (msg->message == WM_ACTIVATE)
+  {
+    if (LOWORD(msg->wParam) == WA_INACTIVE)
+      gdk_window_win32_set_window_ownership(window, HIWORD(msg->wParam));
+  }
+  else if (msg->message == WM_SIZE)
+  {
+    if ((msg->wParam == SIZE_MAXIMIZED) || (msg->wParam == SIZE_RESTORED))
+		gdk_window_win32_set_window_ownership(window, FALSE);
+  }
+  else if (msg->message == WM_DESTROY)
+  {
+    impl = GDK_WINDOW_IMPL_WIN32 (GDK_WINDOW_OBJECT (window)->impl);
+    gdk_window_remove_filter(impl->owner_window,
gdk_window_win32_owner_window_filter, window);
+	impl->owner_window = NULL;
+  }
+
+  return GDK_FILTER_CONTINUE;
+}
+
+static GdkFilterReturn
+gdk_window_win32_owned_window_filter (GdkXEvent *xev,
+		      GdkEvent  *event,
+		      gpointer   data)
+{
+  MSG *msg = (MSG *) xev;
+  GdkWindow* window = event->any.window;
+  GdkWindowImplWin32 *impl;
+
+  g_return_if_fail (GDK_IS_WINDOW (window));
+
+  if (msg->message == WM_SHOWWINDOW)
+  {
+    if (!msg->wParam && (msg->lParam == SW_PARENTCLOSING))
+	{
+      gdk_window_win32_set_window_ownership(window, TRUE);
+    }
+  }
+  else if (msg->message == WM_DESTROY)
+  {
+    gdk_window_remove_filter(window,
gdk_window_win32_owned_window_filter, 0);
+	/* We can't remove the other filter here because the filter list isn't
reentrant.
+	 * This is handled in gdk_window_destroy_notify
+	 */
+  }
+
+  return GDK_FILTER_CONTINUE;
+}
+
 void
 gdk_window_set_transient_for (GdkWindow *window,
 			      GdkWindow *parent)
 {
-  HWND window_id, parent_id;
+  gboolean is_minimized = FALSE;

   g_return_if_fail (window != NULL);
   g_return_if_fail (GDK_IS_WINDOW (window));
+  g_return_if_fail ((parent == NULL) || GDK_IS_WINDOW (parent));

   GDK_NOTE (MISC, g_print ("gdk_window_set_transient_for: %p: %p\n",
@@ -1800,16 +1887,112 @@ gdk_window_set_transient_for (GdkWindow
     }

-  window_id = GDK_WINDOW_HWND (window);
-  parent_id = GDK_WINDOW_HWND (parent);
+  /* This may be called several times, but the gdk_window_add_filter
function
+   * discards duplicate entries.
+   */
+  gdk_window_add_filter(window, gdk_window_win32_owned_window_filter, 0);

-  /* This changes the *owner* of the window, despite the misleading
-   * name. (Owner and parent are unrelated concepts.) At least that's
-   * what people who seem to know what they talk about say on
-   * USENET. Search on Google.
+  GdkWindowImplWin32 *impl;
+  impl = GDK_WINDOW_IMPL_WIN32 (GDK_WINDOW_OBJECT (window)->impl);
+
+  if (impl->owner_window != parent)
+  {
+    if (NULL != impl->owner_window)
+    {
+      /* Remove the previous filter on the parent window */
+      gdk_window_remove_filter(impl->owner_window,
gdk_window_win32_owner_window_filter, window);
+    }
+
+    impl->owner_window = parent;
+
+	if (NULL != parent)
+	{
+      /* Set a filter on the parent window so that when it's restored
+       * this window gets hooked up again.
+	   */
+      gdk_window_add_filter(parent, gdk_window_win32_owner_window_filter,
window);
+    }
+  }
+
+  if (parent)
+  {
+	is_minimized = IsIconic(GDK_WINDOW_HWND (parent));
+  }
+
+  /* This window is unlikely to get a new transient if it's minimized,
+   * but it can happen if you right-click on a minimized application
+   * and choose close; the "do you want to save" dialog is shown.
+   * Note that that code is broken because it positions the dialog
+   * off screen.
    */
-  SetLastError (0);
-  if (SetWindowLong (window_id, GWL_HWNDPARENT, (long) parent_id) == 0 &&
-      GetLastError () != 0)
-    WIN32_API_FAILED ("SetWindowLong");
+  gdk_window_win32_set_window_ownership(window, is_minimized);
+}
+
+/* Set window ownership based on the taskbar hint, whether it's transient
+ * and whether the owner is minimizing.
+ * This is a lot of bother.  The problem is with the Windows taskbar.  If
+ * you minimize an owner window but don't let Windows hide the owned
+ * windows, then when you click on one of the owned windows the taskbar
+ * starts treating it as the application window.  So if you click on
+ * the taskbar button the application window doesn't reappear.  We fix
+ * this by disconnecting owned windows from their owners when the owners
+ * are minimized.  The gdk_window_win32_restore_transients function
+ * reattaches owned windows to their owner when the owner is restored.
+ */
+static void
+gdk_window_win32_set_window_ownership(GdkWindow* window, gboolean
is_owner_minimized)
+{
+	GdkWindowImplWin32 *impl;
+    static GdkWindow *owner = 0;
+    GdkWindowAttr wa;
+
+    g_return_if_fail (GDK_IS_WINDOW (window));
+
+	impl = GDK_WINDOW_IMPL_WIN32 (((GdkWindowObject *) window)->impl);
+
+	if (!is_owner_minimized && impl->owner_window)
+	{
+      g_return_if_fail (GDK_IS_WINDOW (impl->owner_window));
+
+      SetWindowLong(GDK_WINDOW_HWND (window), GWL_HWNDPARENT, (LONG_PTR)
GDK_WINDOW_HWND (impl->owner_window));
+      /* Ensure the owner is behind the owned window */
+      SetWindowPos(GDK_WINDOW_HWND (impl->owner_window), GDK_WINDOW_HWND
(window), 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);
+	}
+	else if (impl->skips_taskbar || (is_owner_minimized && impl->owner_window))
+	{
+      /* Don't overwrite an existing owner unless it's minimizing */
+      if (!(impl->owner_window && is_owner_minimized))
+      {
+        if (GetWindowLong(GDK_WINDOW_HWND (window), GWL_HWNDPARENT) != NULL)
+          return;
+	  }
+
+      if (NULL == owner)
+      {
+        /* Create an invisible window to act as the owner.  The variable
is static
+         * so it gets reused.
+		 */
+        wa.window_type = GDK_WINDOW_TEMP;
+        wa.wclass = GDK_INPUT_OUTPUT;
+        wa.width = wa.height = 1;
+        wa.event_mask = 0;
+        owner = gdk_window_new_internal (NULL, &wa, 0, TRUE);
+	  }
+
+      SetWindowLongPtr (GDK_WINDOW_HWND (window), GWL_HWNDPARENT,
+        (LONG_PTR) GDK_WINDOW_HWND (owner));
+
+#if 0 /* Should we also turn off the minimize and maximize buttons? */
+      SetWindowLong (GDK_WINDOW_HWND (window), GWL_STYLE,
+		     GetWindowLong (GDK_WINDOW_HWND (window), GWL_STYLE) &
~(WS_MINIMIZEBOX|WS_MAXIMIZEBOX|WS_SYSMENU));
+      SetWindowPos (GDK_WINDOW_HWND (window), NULL,
+		    0, 0, 0, 0,
+		    SWP_FRAMECHANGED | SWP_NOACTIVATE | SWP_NOMOVE |
+		    SWP_NOREPOSITION | SWP_NOSIZE | SWP_NOZORDER);
+#endif
+	}
+	else
+	{
+      SetWindowLong(GDK_WINDOW_HWND (window), GWL_HWNDPARENT, 0);
+	}
 }

@@ -3153,6 +3336,6 @@ gdk_window_set_skip_taskbar_hint (GdkWin
 				  gboolean   skips_taskbar)
 {
-  static GdkWindow *owner = NULL;
-  GdkWindowAttr wa;
+  GdkWindowImplWin32 *impl;
+  gboolean is_minimized = FALSE;

   g_return_if_fail (GDK_IS_WINDOW (window));
@@ -3162,31 +3345,13 @@ gdk_window_set_skip_taskbar_hint (GdkWin
 			   skips_taskbar ? "TRUE" : "FALSE"));

-  if (skips_taskbar)
-    {
-      if (owner == NULL)
-	{
-	  wa.window_type = GDK_WINDOW_TEMP;
-	  wa.wclass = GDK_INPUT_OUTPUT;
-	  wa.width = wa.height = 1;
-	  wa.event_mask = 0;
-	  owner = gdk_window_new_internal (NULL, &wa, 0, TRUE);
-	}
+  impl = GDK_WINDOW_IMPL_WIN32 (((GdkWindowObject *) window)->impl);
+  impl->skips_taskbar = skips_taskbar;

-      SetWindowLong (GDK_WINDOW_HWND (window), GWL_HWNDPARENT,
-		     (long) GDK_WINDOW_HWND (owner));
+  if (impl->owner_window)
+  {
+	is_minimized = IsIconic(GDK_WINDOW_HWND (impl->owner_window));
+  }

-#if 0 /* Should we also turn off the minimize and maximize buttons? */
-      SetWindowLong (GDK_WINDOW_HWND (window), GWL_STYLE,
-		     GetWindowLong (GDK_WINDOW_HWND (window), GWL_STYLE) &
~(WS_MINIMIZEBOX|WS_MAXIMIZEBOX|WS_SYSMENU));
-      SetWindowPos (GDK_WINDOW_HWND (window), NULL,
-		    0, 0, 0, 0,
-		    SWP_FRAMECHANGED | SWP_NOACTIVATE | SWP_NOMOVE |
-		    SWP_NOREPOSITION | SWP_NOSIZE | SWP_NOZORDER);
-#endif
-    }
-  else
-    {
-      SetWindowLong (GDK_WINDOW_HWND (window), GWL_HWNDPARENT, 0);
-    }
+  gdk_window_win32_set_window_ownership(window, is_minimized);
 }