Re: user32: fix SetWindowPos to erase parent when child is hidden

[Clinton Stimpson]

Can someone help me with why this patch isn't correct enough?

Thanks,
Clinton

Clinton Stimpson wrote:
> When hiding windows, WM_ERASEBKGND is supposed to be sent to the parent.
> The message will now be sent with this patch.  An existing test was 
> modified to also check for all parent messages.
> Partial fix for bug #6006
>
> Thanks,
> Clinton Stimpson
>
> ChangeLog:
>  user32: send erase to parent when hiding children
> Index: dlls/user32/winpos.c
>
> RCS file: /home/wine/wine/dlls/user32/winpos.c,v
> retrieving revision 1.5
> diff -u -r1.5 winpos.c
> --- dlls/user32/winpos.c	26 Apr 2007 15:25:46 -0000	1.5
> +++ dlls/user32/winpos.c	23 Jun 2007 22:24:43 -0000
> @@ -1566,6 +1566,7 @@
>  {
>      RECT newWindowRect, newClientRect, valid_rects[2];
>      UINT orig_flags;
> -
>
> HWND parent = GetAncestor( winpos->hwnd, GA_PARENT );
>
> orig_flags = winpos->flags;
>
> @@ -1592,7 +1593,7 @@
>  if((winpos->flags & (SWP_NOZORDER | SWP_HIDEWINDOW | SWP_SHOWWINDOW)) != SWP_NOZORDER)
>  {
> -
>    if (GetAncestor( winpos->hwnd, GA_PARENT ) == GetDesktopWindow())
> -
>    if (parent == GetDesktopWindow())
>        winpos->hwndInsertAfter = SWP_DoOwnedPopups( winpos->hwnd, winpos->hwndInsertAfter );
> }
>
> @@ -1613,6 +1614,14 @@
>          RedrawWindow( winpos->hwnd, NULL, NULL, rdw_flags );
>      }
> - /* erase the parent for child windows that are being hidden */
> - if(parent && (orig_flags & SWP_HIDEWINDOW))
> - {
> -
>    UINT rdw_flags = RDW_ERASE | RDW_INVALIDATE;
> -
>    if ( !(orig_flags & SWP_DEFERERASE) ) rdw_flags |= RDW_ERASENOW;
> -
>    RedrawWindow( parent, &newClientRect, NULL, rdw_flags );
> - }
> - if( winpos->flags & SWP_HIDEWINDOW )
>        HideCaret(winpos->hwnd);
>    else if (winpos->flags & SWP_SHOWWINDOW)
> Index: dlls/user32/tests/msg.c
>
> RCS file: /home/wine/wine/dlls/user32/tests/msg.c,v
> retrieving revision 1.33
> diff -u -r1.33 msg.c
> --- dlls/user32/tests/msg.c	21 Jun 2007 14:49:29 -0000	1.33
> +++ dlls/user32/tests/msg.c	23 Jun 2007 22:24:43 -0000
> @@ -727,6 +727,15 @@
>      { WM_WINDOWPOSCHANGED, sent|wparam, SWP_HIDEWINDOW|SWP_NOACTIVATE|SWP_NOSIZE|SWP_NOMOVE|SWP_NOCLIENTSIZE|SWP_NOCLIENTMOVE },
>      { 0 }
>  };
> +/* ShowWindow(SW_HIDE) for a visible child window checking all parent events*/
> +static const struct message WmHideChildSeq2[] = {
> - { WM_SHOWWINDOW, sent|wparam, 0 },
> - { WM_WINDOWPOSCHANGING, sent|wparam, SWP_HIDEWINDOW|SWP_NOACTIVATE|SWP_NOSIZE|SWP_NOMOVE },
> - { EVENT_OBJECT_HIDE, winevent_hook|wparam|lparam, 0, 0 },
> - { WM_ERASEBKGND, sent|parent },
> - { WM_WINDOWPOSCHANGED, sent|wparam, SWP_HIDEWINDOW|SWP_NOACTIVATE|SWP_NOSIZE|SWP_NOMOVE|SWP_NOCLIENTSIZE|SWP_NOCLIENTMOVE },
> - { 0 }
>
> +};
>  /* SetWindowPos(SWP_SHOWWINDOW|SWP_NOSIZE|SWP_NOMOVE)
> - for a not visible child window
>
> */
> @@ -3982,8 +3991,10 @@
>      ShowWindow(hchild, SW_SHOW);
>      ok_sequence(WmShowChildSeq, "ShowWindow(SW_SHOW):child", FALSE);
> - log_all_parent_messages++;
> ShowWindow(hchild, SW_HIDE);
> - ok_sequence(WmHideChildSeq, "ShowWindow(SW_HIDE):child", FALSE);
> -
>
> ok_sequence(WmHideChildSeq2, "ShowWindow(SW_HIDE):child", FALSE);
> -
>
> log_all_parent_messages--;
>
> ShowWindow(hchild, SW_SHOW);
> ok_sequence(WmShowChildSeq, "ShowWindow(SW_SHOW):child", FALSE);

user32: fix SetWindowPos to erase parent when child is hidden

[Dmitry Timoshkov]

"Clinton Stimpson"  wrote:
> Can someone help me with why this patch isn't correct enough?
>
> Thanks,
> Clinton
>
> Clinton Stimpson wrote:
> > When hiding windows, WM_ERASEBKGND is supposed to be sent to the parent.
> > The message will now be sent with this patch.  An existing test was modified to also check for all parent messages.
> > Partial fix for bug #6006
> >
> > Thanks,
> > Clinton Stimpson
> >
> > ChangeLog:
> >  user32: send erase to parent when hiding children
> > Index: dlls/user32/winpos.c
> >
> > RCS file: /home/wine/wine/dlls/user32/winpos.c,v
> > retrieving revision 1.5
> > diff -u -r1.5 winpos.c
> > --- dlls/user32/winpos.c 26 Apr 2007 15:25:46 -0000 1.5
> > +++ dlls/user32/winpos.c 23 Jun 2007 22:24:43 -0000
> > @@ -1566,6 +1566,7 @@
> >  {
> >      RECT newWindowRect, newClientRect, valid_rects[2];
> >      UINT orig_flags;
> > - HWND parent = GetAncestor( winpos->hwnd, GA_PARENT );
> > orig_flags = winpos->flags;
> > @@ -1592,7 +1593,7 @@
> >
> > if((winpos->flags & (SWP_NOZORDER | SWP_HIDEWINDOW | SWP_SHOWWINDOW)) != SWP_NOZORDER)
> >      {
> > -
> >    if (GetAncestor( winpos->hwnd, GA_PARENT ) == GetDesktopWindow())
> > -
> >    if (parent == GetDesktopWindow())
> >        winpos->hwndInsertAfter = SWP_DoOwnedPopups( winpos->hwnd, winpos->hwndInsertAfter );
> > }

The changes above look not related to me, and they require a separate test case to prove
their correctness.
> > @@ -1613,6 +1614,14 @@
> >          RedrawWindow( winpos->hwnd, NULL, NULL, rdw_flags );
> >      }
> > - /* erase the parent for child windows that are being hidden */
> > - if(parent && (orig_flags & SWP_HIDEWINDOW))
> > - {
> > -
> >    UINT rdw_flags = RDW_ERASE | RDW_INVALIDATE;
> > -
> >    if ( !(orig_flags & SWP_DEFERERASE) ) rdw_flags |= RDW_ERASENOW;
> > -
> >    RedrawWindow( parent, &newClientRect, NULL, rdw_flags );
> > - }

This change looks wrong. wineserver takes care of exposed children already.
> > - if( winpos->flags & SWP_HIDEWINDOW )
> >        HideCaret(winpos->hwnd);
> >    else if (winpos->flags & SWP_SHOWWINDOW)
> > Index: dlls/user32/tests/msg.c
> >
> > RCS file: /home/wine/wine/dlls/user32/tests/msg.c,v
> > retrieving revision 1.33
> > diff -u -r1.33 msg.c
> > --- dlls/user32/tests/msg.c 21 Jun 2007 14:49:29 -0000 1.33
> > +++ dlls/user32/tests/msg.c 23 Jun 2007 22:24:43 -0000
> > @@ -727,6 +727,15 @@
> >      { WM_WINDOWPOSCHANGED, sent|wparam, 
> > SWP_HIDEWINDOW|SWP_NOACTIVATE|SWP_NOSIZE|SWP_NOMOVE|SWP_NOCLIENTSIZE|SWP_NOCLIENTMOVE },
> >      { 0 }
> >  };
> > +/* ShowWindow(SW_HIDE) for a visible child window checking all parent events*/
> > +static const struct message WmHideChildSeq2[] = {
> > - { WM_SHOWWINDOW, sent|wparam, 0 },
> > - { WM_WINDOWPOSCHANGING, sent|wparam, SWP_HIDEWINDOW|SWP_NOACTIVATE|SWP_NOSIZE|SWP_NOMOVE },
> > - { EVENT_OBJECT_HIDE, winevent_hook|wparam|lparam, 0, 0 },
> > - { WM_ERASEBKGND, sent|parent },
> > - { WM_WINDOWPOSCHANGED, sent|wparam,
> >
> > SWP_HIDEWINDOW|SWP_NOACTIVATE|SWP_NOSIZE|SWP_NOMOVE|SWP_NOCLIENTSIZE|SWP_NOCLIENTMOVE },
> > - { 0 }
> >
> > +};
> >  /* SetWindowPos(SWP_SHOWWINDOW|SWP_NOSIZE|SWP_NOMOVE)
> > - for a not visible child window
> >
> > */
> > @@ -3982,8 +3991,10 @@
> >      ShowWindow(hchild, SW_SHOW);
> >      ok_sequence(WmShowChildSeq, "ShowWindow(SW_SHOW):child", FALSE);
> > - log_all_parent_messages++;
> > ShowWindow(hchild, SW_HIDE);
> > - ok_sequence(WmHideChildSeq, "ShowWindow(SW_HIDE):child", FALSE);
> > - ok_sequence(WmHideChildSeq2, "ShowWindow(SW_HIDE):child", FALSE);
> > - log_all_parent_messages--;

Existing message test (WmHideChildSeq) shows that current behaviour is correct. If you have
a test case that doesn't pass under Wine - add it first, only then send a proposed fix.

user32: fix SetWindowPos to erase parent when child is hidden

[Dmitry Timoshkov]

"Dmitry Timoshkov"  wrote:
> > > - HWND parent = GetAncestor( winpos->hwnd, GA_PARENT );
> > > orig_flags = winpos->flags;
> > > @@ -1592,7 +1593,7 @@
> > >
> > > if((winpos->flags & (SWP_NOZORDER | SWP_HIDEWINDOW | SWP_SHOWWINDOW)) != SWP_NOZORDER)
> > >      {
> > > -
> > >    if (GetAncestor( winpos->hwnd, GA_PARENT ) == GetDesktopWindow())
> > > -
> > >    if (parent == GetDesktopWindow())
> > >        winpos->hwndInsertAfter = SWP_DoOwnedPopups( winpos->hwnd, winpos->hwndInsertAfter );
> > > }
>
> The changes above look not related to me, and they require a separate test case to prove
> their correctness.

Oops, sorry for the useless comment.