[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    ShowOwnedPopups problems
From:       "Louis Philippe Gagnon" <louisphilippe () macadamian ! com>
Date:       2000-07-31 14:18:24
[Download RAW message or body]

While working on various z-order issues, I have come to notice some problems
with the current implementation of ShowOwnedPopups and its uses. I have
already submitted a patch to CorelWine, but I would like some feedback
before sending one to WineHQ.

Problem 1)

ShowOwnedPopups currently show windows starting from the highest and ending
with the lowest. Although this is what happens in Windows, it causes
problems in Wine because ShowWindow() places windows on top of the Z-Order
when showing them. This results in an inversion of the Z-Order, the highest
window ending up below the lowest window.

I have been unable to force ShowWindow() to show windows without placing
them on top. Even if the first ShowWindow() places the window correctly, the
resulting interaction between X and Wine (X Event handlers call Wine
functions which make changes to X windows, generating more events...) always
ends up placing the window on top (this interaction actually seems much more
complex in the CorelWine tree)

The solution I have used is to show windows starting form the lowest and
ending with the highest. This preserves the Z-Order but is the opposite of
what Windows does. Can anyone suggest a better solution?

Problem 2)
ShowOwnedPopups currently sends WM_SHOWWINDOW messages with either
SW_PARENTCLOSING or SW_PARENTOPENING depending on the state of the owner
window (IsIconic). This is not what Windows does : for SW_HIDE, Windows uses
SW_PARENTCLOSING, and for SW_SHOW, Windows uses SW_PARENTOPENING. I don't
think anyone will object to changing this.

Problem 3)
We are currently calling ShowOwnedPopups from EVENT_MapNotify and
EVENT_UnmapNotify to make sure owned windows are hidden when their owner is
unmapped. There are two problems with that :

a)
Since ShowOwnedPopups simply sends a WM_SHOWWINDOW message and leaves the
actual showing/hiding to DEFWND_DefWinProc(), a program can prevent the
expected behaviour simply by intercepting the WM_SHOWWINDOW message and not
calling DefWindowProc(). This is how the function works in Windows, but
isn't something that should be allowed in this case.

b)
ShowOwnedPopups works by setting a flag (WIN_NEEDS_SHOW_OWNEDPOPUP) when
hiding a window, so that when ShowOwnedPopups is called to show windows, it
can recognize the windows it previously hid and should show. This means that
there is no nesting : if ShowWindow is called twice to hide windows, it only
needs to be called once to show them.

Now take the following scenario : an application calls ShowOwnedPopups to
hide windows, and then the user minimizes the application (provoking a
second call to ShowOwnedPopups). When the user restores the application,
ShowOwnedPopups will be called, showing all previously hidden windows. These
windows will therefore become visible, even though the application wanted
them hidden.


The solution to both these problems would be to call a different function in
EVENT_Map/UnmapNotify. This function (I called it
WIN_InternalShowOwnedPopups) can use a different flag
(WIN_NEEDSINTERNALSOP), avoiding any interference with application calls to
ShowOwnedPopups. This function can also call ShowWindow directly, thus
preventing the application from intercepting the WM_SHOWWINDOW messages.

One inconvenient with this approach is that we introduce a new flag in the
WND structure. The WND::flags member is only 16bits, and with 12 flags
already defined, each remaining bit is precious. Is there any reason this
flag isn't 32bits? Can it be changed? And if not, are the remaining bits too
valuable to use one in this case?


Note that InternalShowOwnedPopups takes an additionnal parameter,
unmanagedOnly, to specify that we only want to show/hide unmanaged windows.
This was introduced in the Corel tree to avoid a crash when dragging a
window to a different desktop in KDE. (besides, what happens to managed
windows when unmapping the main window should be left up to the window
manager)

Anyway, here is the patch as it stands, I'd like some comments.
(and if anyone wants to suggest better/shorter names for the new function
and flag, please do)


Louis-Philippe Gagnon
Macadamian Technologies Inc.

ps.this patch also adds to EVENT_MapNotify the sanity check recently added
to EVENT_UnmapNotify by Dmitry Timoshkov

["showownedpopups.diff" (application/octet-stream)]

Index: include/win.h
===================================================================
RCS file: /home/wine/wine/include/win.h,v
retrieving revision 1.29
diff -u -r1.29 win.h
--- include/win.h	2000/07/28 23:04:55	1.29
+++ include/win.h	2000/07/31 13:38:03
@@ -158,6 +158,7 @@
 #define WIN_ISDIALOG           0x0200 /* Window is a dialog */
 #define WIN_ISWIN32            0x0400 /* Understands Win32 messages */
 #define WIN_NEEDS_SHOW_OWNEDPOPUP 0x0800 /* WM_SHOWWINDOW:SC_SHOW must be sent in \
the next ShowOwnedPopup call */ +#define WIN_NEEDS_INTERNALSOP  0x1000 /* Window was \
hidden by WIN_InternalShowOwnedPopups */  
   /* BuildWinArray() flags */
 #define BWA_SKIPDISABLED	0x0001
@@ -197,6 +198,7 @@
 extern BOOL WIN_IsWindowDrawable(WND*, BOOL );
 extern WND**  WIN_BuildWinArray( WND *wndPtr, UINT bwa, UINT* pnum );
 extern void   WIN_ReleaseWinArray(WND **wndArray);
+extern BOOL WIN_InternalShowOwnedPopups( HWND owner, BOOL fShow, BOOL unmanagedOnly \
);  
 extern HICON16 NC_IconForWindow( WND *wndPtr );
 
Index: windows/win.c
===================================================================
RCS file: /home/wine/wine/windows/win.c,v
retrieving revision 1.96
diff -u -r1.96 win.c
--- windows/win.c	2000/07/29 11:31:29	1.96
+++ windows/win.c	2000/07/31 13:38:03
@@ -2633,6 +2633,66 @@
     return GetWindow16( hwnd, flag );
 }
 
+/***********************************************************************
+ *           WIN_InternalShowOwnedPopups 
+ *
+ * Internal version of ShowOwnedPopups; Wine functions should use this
+ * to avoid interfering with application calls to ShowOwnedPopups
+ * and to make sure the application can't prevent showing/hiding.
+ *
+ * Set unmanagedOnly to TRUE to show/hide unmanaged windows only. 
+ *
+ */
+
+BOOL WIN_InternalShowOwnedPopups( HWND owner, BOOL fShow, BOOL unmanagedOnly )
+{
+    INT totalChild=0, count=0;
+
+    WND **pWnd = WIN_BuildWinArray(WIN_GetDesktop(), 0, &totalChild);
+
+    if (!pWnd) return TRUE;
+
+    /*
+     * Show windows Lowest first, Highest last to preserve Z-Order
+     */
+    for (count = totalChild-1 ; count >=0; count--)
+    {
+        if (pWnd[count]->owner && (pWnd[count]->owner->hwndSelf == owner) && \
(pWnd[count]->dwStyle & WS_POPUP)) +        {
+            if (fShow)
+            {
+                /* check in window was flagged for showing in previous \
WIN_InternalShowOwnedPopups call */ +                if (pWnd[count]->flags & \
WIN_NEEDS_INTERNALSOP)  +                {
+                    /*
+                     * Call ShowWindow directly because an application can intercept \
WM_SHOWWINDOW messages +                     */
+                    ShowWindow(pWnd[count]->hwndSelf,SW_SHOW);
+                    pWnd[count]->flags &= ~WIN_NEEDS_INTERNALSOP; /* remove the flag \
*/ +                }
+            }
+            else
+            {
+                if ( IsWindowVisible(pWnd[count]->hwndSelf) &&                   /* \
hide only if window is visible */ +                     !( pWnd[count]->flags & \
WIN_NEEDS_INTERNALSOP ) &&          /* don't hide if previous call already did it */ \
+                     !( unmanagedOnly && (pWnd[count]->flags & WIN_MANAGED ) ) ) /* \
don't hide managed windows if unmanagedOnly is TRUE */ +                {
+                    /*
+                     * Call ShowWindow directly because an application can intercept \
WM_SHOWWINDOW messages +                     */
+                    ShowWindow(pWnd[count]->hwndSelf,SW_HIDE);
+                    /* flag the window for showing on next \
WIN_InternalShowOwnedPopups call */ +                    pWnd[count]->flags |= \
WIN_NEEDS_INTERNALSOP; +                }
+            }
+        }
+    }
+    WIN_ReleaseDesktop();    
+    WIN_ReleaseWinArray(pWnd);
+    
+    return TRUE;
+}
+
 /*******************************************************************
  *         ShowOwnedPopups16  (USER.265)
  */
@@ -2647,13 +2707,14 @@
  */
 BOOL WINAPI ShowOwnedPopups( HWND owner, BOOL fShow )
 {
-    UINT totalChild=0, count=0;
+    INT totalChild=0, count=0;
 
     WND **pWnd = WIN_BuildWinArray(WIN_GetDesktop(), 0, &totalChild);
 
     if (!pWnd) return TRUE;
 
-    for (; count < totalChild; count++)
+    /* show windows lowest first, highest last; this isn't what windows does but it \
is necessary to preserve the z-order*/ +    for (count=totalChild-1; count >=0; \
count--)  {
         if (pWnd[count]->owner && (pWnd[count]->owner->hwndSelf == owner) && \
(pWnd[count]->dwStyle & WS_POPUP))  {
@@ -2661,7 +2722,11 @@
             {
                 if (pWnd[count]->flags & WIN_NEEDS_SHOW_OWNEDPOPUP)
                 {
-                    SendMessageA(pWnd[count]->hwndSelf, WM_SHOWWINDOW, SW_SHOW, \
IsIconic(owner) ? SW_PARENTOPENING : SW_PARENTCLOSING); +                    /*
+                     * In Windows, ShowOwnedPopups(TRUE) generates WM_SHOWWINDOW \
messages with SW_PARENTOPENING,  +                     * regardless of the state of \
the owner +                     */
+                    SendMessageA(pWnd[count]->hwndSelf, WM_SHOWWINDOW, SW_SHOW, \
SW_PARENTOPENING);  pWnd[count]->flags &= ~WIN_NEEDS_SHOW_OWNEDPOPUP;
                 }
             }
@@ -2669,7 +2734,11 @@
             {
                 if (IsWindowVisible(pWnd[count]->hwndSelf))
                 {
-                    SendMessageA(pWnd[count]->hwndSelf, WM_SHOWWINDOW, SW_HIDE, \
IsIconic(owner) ? SW_PARENTOPENING : SW_PARENTCLOSING); +                    /*
+                     * In Windows, ShowOwnedPopups(FALSE) generates WM_SHOWWINDOW \
messages with SW_PARENTCLOSING,  +                     * regardless of the state of \
the owner +                     */
+                    SendMessageA(pWnd[count]->hwndSelf, WM_SHOWWINDOW, SW_HIDE, \
SW_PARENTCLOSING);  pWnd[count]->flags |= WIN_NEEDS_SHOW_OWNEDPOPUP;
                 }
             }
Index: windows/x11drv/event.c
===================================================================
RCS file: /home/wine/wine/windows/x11drv/event.c,v
retrieving revision 1.72
diff -u -r1.72 event.c
--- windows/x11drv/event.c	2000/07/28 22:21:20	1.72
+++ windows/x11drv/event.c	2000/07/31 13:38:04
@@ -1854,12 +1854,12 @@
   HWND hwndFocus = GetFocus();
   WND *wndFocus = WIN_FindWndPtr(hwndFocus);
   WND *pWnd = WIN_FindWndPtr(hWnd);
-  if (pWnd->flags & WIN_MANAGED)
+  if (pWnd && (pWnd->flags & WIN_MANAGED))
   {
       DCE_InvalidateDCE( pWnd, &pWnd->rectWindow );
       pWnd->dwStyle &= ~WS_MINIMIZE;
       pWnd->dwStyle |=  WS_VISIBLE;
-      ShowOwnedPopups(hWnd,TRUE);
+      WIN_InternalShowOwnedPopups(hWnd,TRUE,TRUE);
   }
   WIN_ReleaseWndPtr(pWnd);
 
@@ -1873,7 +1873,7 @@
 
 
 /**********************************************************************
- *              EVENT_MapNotify
+ *              EVENT_UnmapNotify
  */
 void EVENT_UnmapNotify( HWND hWnd, XUnmapEvent *event )
 {
@@ -1885,7 +1885,7 @@
       {
 	  pWnd->dwStyle |=  WS_MINIMIZE;
 	  pWnd->dwStyle &= ~WS_VISIBLE;
-          ShowOwnedPopups(hWnd,FALSE);
+            WIN_InternalShowOwnedPopups(hWnd,FALSE,TRUE);
       }
   }
   WIN_ReleaseWndPtr(pWnd);



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic