toolbar.c problem (ATTN: Duane)

[Dusan Vujosevic]

Hi,

I am having a problem with the way toolbar.c was modified between revisions 1.107 & 1.108. My application (Signlab) pops flyout toolbars that stay around until they lose focus. The parent toolbar receives a 'TBN_DROPDOWN' message when someone clicks on one of its flyout buttons (button style: TBSTYLE_DROPDOWN). It creates a flyout at that point.

With revision 1.108 of toolbar.c in Wine the parent toolbar also receives an WN_COMMAND after TBN_DROPDOWN message which causes Wine (rightly so) to send an WM_SETFOCUS (and WM_ACTIVATE). That causes the parent to regain focus and kills the flyout. The line that sends the WN_COMMAND is in toolbar.c at line 4540 (TOOLBAR_LButtonUp() function):

SendMessageA (infoPtr->hwndNotify, WM_COMMAND,
          MAKEWPARAM(infoPtr->buttons[nHit].idCommand, 0), (LPARAM)hwnd);

In revision 1.107 it used to be:

if (bSendMessage)
     SendMessageA (infoPtr->hwndNotify, WM_COMMAND,
     MAKEWPARAM(btnPtr->idCommand, 0), (LPARAM)hwnd);

And that worked fine because it would never send WM_COMMAND for a flyout button. Note that WinXP (that's where I tested) doesn't send WM_COMMAND after TBN_DROPDOWN has been handled. I would like to suggest revert that line back to 1.106 because it is the correct way.

If other apps are were having problems with 1.106 (Duane was mentioning Acrobat Reader - I will test that..) I suggest that the problem might be elsewhere.

Should I supply a patch?

Cheers,
Dusan

[Duane Clark]

Dusan Vujosevic wrote:
> Hi,
>
> I am having a problem with the way toolbar.c was modified between 
> revisions 1.107 & 1.108. My application (Signlab) pops flyout toolbars 
> that stay around until they lose focus. The parent toolbar receives a 
> 'TBN_DROPDOWN' message when someone clicks on one of its flyout buttons 
> (button style: TBSTYLE_DROPDOWN). It creates a flyout at that point.
>
> With revision 1.108 of toolbar.c in Wine the parent toolbar also 
> receives an WN_COMMAND after TBN_DROPDOWN message which causes Wine 
> (rightly so) to send an WM_SETFOCUS (and WM_ACTIVATE). That causes the 
> parent to regain focus and kills the flyout. The line that sends 
> the WN_COMMAND is in toolbar.c at line 4540 (TOOLBAR_LButtonUp() function):
>         SendMessageA (infoPtr->hwndNotify, WM_COMMAND,
>           MAKEWPARAM(infoPtr->buttons[nHit].idCommand, 0), (LPARAM)hwnd);
> In revision 1.107 it used to be:
>
> if (bSendMessage)
>      SendMessageA (infoPtr->hwndNotify, WM_COMMAND,
>      MAKEWPARAM(btnPtr->idCommand, 0), (LPARAM)hwnd);
>
> And that worked fine because it would never send WM_COMMAND for a flyout 
> button. Note that WinXP (that's where I tested) doesn't send WM_COMMAND 
> after TBN_DROPDOWN has been handled. I would like to suggest revert that 
> line back to 1.106 because it is the correct way.

It is not "the correct way", since I have tested it with control spy 
under WinNT and Wine. That is not to say that my patch is correct 
either, but it fixes two different applications I have and in my mind is 
closer to the correct way. There may well be some conditions under which 
the message should not be sent, but the previous way was not correct. So 
I guess Alexandre gets to pick which incorrect version to use ;)

And there is more going on in your app, because the flying toolbars also 
disappear when the mouse is simply slid over them, without clicking. 
This behavior is not there under Windows, and has nothing to do with the 
ButtonUp change I made. The two may not be related, but I cannot help 
thinking they are. Unfortunately, I was not able to get a spy program to 
work with your app in windows, so I have not been able to figure out 
what messages should really be sent, and when. Certainly a common 
problem is that a fix in one area exposes a problem elsewhere, and that 
possibly is what happened here.

[Dusan Vujosevic]

Hello,

----- Original Message -----
From: "Duane Clark" 
To: "Dusan Vujosevic" 
Cc: 
Sent: Thursday, April 18, 2002 12:44 PM
Subject: Re: toolbar.c problem (ATTN: Duane)
> Dusan Vujosevic wrote:
> > Hi,
> >
> > I am having a problem with the way toolbar.c was modified between
> > revisions 1.107 & 1.108. My application (Signlab) pops flyout toolbars
> > that stay around until they lose focus. The parent toolbar receives a
> > 'TBN_DROPDOWN' message when someone clicks on one of its flyout buttons
> > (button style: TBSTYLE_DROPDOWN). It creates a flyout at that point.
> >
> > With revision 1.108 of toolbar.c in Wine the parent toolbar also
> > receives an WN_COMMAND after TBN_DROPDOWN message which causes Wine
> > (rightly so) to send an WM_SETFOCUS (and WM_ACTIVATE). That causes the
> > parent to regain focus and kills the flyout. The line that sends
> > the WN_COMMAND is in toolbar.c at line 4540 (TOOLBAR_LButtonUp()

function):
> >     SendMessageA (infoPtr->hwndNotify, WM_COMMAND,
> >       MAKEWPARAM(infoPtr->buttons[nHit].idCommand, 0),

(LPARAM)hwnd);
> > In revision 1.107 it used to be:
> >
> > if (bSendMessage)
> >      SendMessageA (infoPtr->hwndNotify, WM_COMMAND,
> >      MAKEWPARAM(btnPtr->idCommand, 0), (LPARAM)hwnd);
> >
> > And that worked fine because it would never send WM_COMMAND for a flyout
> > button. Note that WinXP (that's where I tested) doesn't send WM_COMMAND
> > after TBN_DROPDOWN has been handled. I would like to suggest revert that
> > line back to 1.106 because it is the correct way.
>
> It is not "the correct way", since I have tested it with control spy
> under WinNT and Wine. That is not to say that my patch is correct
> either, but it fixes two different applications I have and in my mind is
> closer to the correct way. There may well be some conditions under which
> the message should not be sent, but the previous way was not correct.

How about we test whether the button has TBSTYLE_DROPDOWN
style set and not send the message in that case???
> So
> I guess Alexandre gets to pick which incorrect version to use ;)

Hm :/
> And there is more going on in your app, because the flying toolbars also
> disappear when the mouse is simply slid over them, without clicking.

Wine or Windows? I don't follow this / can't reproduce it under either one.
> This behavior is not there under Windows, and has nothing to do with the
> ButtonUp change I made. The two may not be related, but I cannot help
> thinking they are. Unfortunately, I was not able to get a spy program to
> work with your app in windows, so I have not been able to figure out
> what messages should really be sent, and when.

I use Spy++ on it... What was the problem?
> Certainly a common
> problem is that a fix in one area exposes a problem elsewhere, and that
> possibly is what happened here.

Probably. I am really intersted in helping track this one down (although I
can work around it in SL and that's what I'll do for the time being ...).

Cheers,
 Dusan

[Duane Clark]

Dusan Vujosevic wrote:
> Hello,
>
> ----- Original Message -----
> From: "Duane Clark" 
> ...
> > It is not "the correct way", since I have tested it with control spy
> > under WinNT and Wine. That is not to say that my patch is correct
> > either, but it fixes two different applications I have and in my mind is
> > closer to the correct way. There may well be some conditions under which
> > the message should not be sent, but the previous way was not correct.
>
> How about we test whether the button has TBSTYLE_DROPDOWN
> style set and not send the message in that case???

Okay by me. I certainly did not test for that case.
> > And there is more going on in your app, because the flying toolbars also
> > disappear when the mouse is simply slid over them, without clicking.
>
> Wine or Windows? I don't follow this / can't reproduce it under either one.

Under Wine, I can make the flying toolbar stick (even with my patch) by 
holding the mouse button down and sliding it out of the main toolbar. 
Once out of the toolbar, I can let up the mouse button without the 
toolbar going away. Once stuck, if I simply slide the mouse across the 
flying toolbar, it disappears. This does not happen under Windows.

Note that I have my window manager set for "focus follows mouse", which 
perhaps could possibly have some effect here. However, other programs I 
have tried with flying toolbars, including Word2000 and Acroread, do not 
have this problem (nor the mouseup problem), so it is not a simple 
generic problem.
> > This behavior is not there under Windows, and has nothing to do with the
> > ButtonUp change I made. The two may not be related, but I cannot help
> > thinking they are. Unfortunately, I was not able to get a spy program to
> > work with your app in windows, so I have not been able to figure out
> > what messages should really be sent, and when.
>
> I use Spy++ on it... What was the problem?

I tried APIS32, but it barfed on this particular program. Unfortunately 
for me, my real work is once again interfering severely with the amount 
of time I have to spend on Wine :-(  But I will try to get in a couple 
of tests with ControlSpy with the TBSTYLE_DROPDOWN style, since it 
should not take much time, perhaps this weekend.

[Andreas Mohr]

On Thu, Apr 18, 2002 at 04:18:47PM -0700, Duane Clark wrote:
> Dusan Vujosevic wrote:
> > I use Spy++ on it... What was the problem?
>
> I tried APIS32, but it barfed on this particular program. Unfortunately 
> for me, my real work is once again interfering severely with the amount 
> of time I have to spend on Wine :-(  But I will try to get in a couple 
> of tests with ControlSpy with the TBSTYLE_DROPDOWN style, since it 
> should not take much time, perhaps this weekend.

Did you *really* only enable logging when arriving at this part ??
Or maybe you did not enable logging even though you should have
at some critical part or another...

[Duane Clark]

Andreas Mohr wrote:
> On Thu, Apr 18, 2002 at 04:18:47PM -0700, Duane Clark wrote:
> > Dusan Vujosevic wrote:
> > > I use Spy++ on it... What was the problem?
> >
> > I tried APIS32, but it barfed on this particular program. Unfortunately 
> > for me, my real work is once again interfering severely with the amount 
> > of time I have to spend on Wine :-(  But I will try to get in a couple 
> > of tests with ControlSpy with the TBSTYLE_DROPDOWN style, since it 
> > should not take much time, perhaps this weekend.
>
> Did you *really* only enable logging when arriving at this part ??
> Or maybe you did not enable logging even though you should have
> at some critical part or another...

As I recall, the program would not even run. It was the first time I 
tried APIS32, though, so it could easily be operator error. I did try 
running another program under APIS32, and was able to get it that one to 
work.

[Dusan Vujosevic]

----- Original Message -----
From: "Duane Clark" 
To: "Dusan Vujosevic" 
Cc: 
Sent: Thursday, April 18, 2002 7:18 PM
Subject: Re: toolbar.c problem (ATTN: Duane)
> Dusan Vujosevic wrote:
> > Hello,
> >
> > ----- Original Message -----
> > From: "Duane Clark" 
> > ...
> > > It is not "the correct way", since I have tested it with control spy
> > > under WinNT and Wine. That is not to say that my patch is correct
> > > either, but it fixes two different applications I have and in my mind is
> > > closer to the correct way. There may well be some conditions under which
> > > the message should not be sent, but the previous way was not correct.
> >
> > How about we test whether the button has TBSTYLE_DROPDOWN
> > style set and not send the message in that case???
>
> Okay by me. I certainly did not test for that case.
> > > And there is more going on in your app, because the flying toolbars also
> > > disappear when the mouse is simply slid over them, without clicking.
> >
> > Wine or Windows? I don't follow this / can't reproduce it under either

one.
> Under Wine, I can make the flying toolbar stick (even with my patch) by
> holding the mouse button down and sliding it out of the main toolbar.
> Once out of the toolbar, I can let up the mouse button without the
> toolbar going away. Once stuck, if I simply slide the mouse across the
> flying toolbar, it disappears. This does not happen under Windows.

That is true. If release the left mouse button anywhere else except over the
toolbars the flyout will stick. The reason is that TOOLBAR_LButtonUp()
doesn't get executed in that case and the WM_COMMAND is not sent.
(Everything works perfectly under any Windows as you have noticed because
Win32 doesn't send the WM_COMMAND as I said before).
> Note that I have my window manager set for "focus follows mouse", which
> perhaps could possibly have some effect here. However, other programs I
> have tried with flying toolbars, including Word2000 and Acroread, do not
> have this problem (nor the mouseup problem), so it is not a simple
> generic problem.

I will spy on Acrobat 4 over the weekend (don't have Word ...). That
*probably* isn't a TBSTYLE_DROPDOWN button (just judging from its looks).
> > > This behavior is not there under Windows, and has nothing to do with the
> > > ButtonUp change I made. The two may not be related, but I cannot help
> > > thinking they are. Unfortunately, I was not able to get a spy program to
> > > work with your app in windows, so I have not been able to figure out
> > > what messages should really be sent, and when.
> >
> > I use Spy++ on it... What was the problem?
>
> I tried APIS32, but it barfed on this particular program.

I will try that as well.
> Unfortunately
> for me, my real work is once again interfering severely with the amount
> of time I have to spend on Wine :-(

I'd be all too happy to help. I will take off my own time to test & fix
this. I'll try give you something over the weekend (patch).
> But I will try to get in a couple
> of tests with ControlSpy with the TBSTYLE_DROPDOWN style, since it
> should not take much time, perhaps this weekend.

Thanks,
Dusan

[Duane Clark]

Dusan Vujosevic wrote:
> > Under Wine, I can make the flying toolbar stick (even with my patch) by
> > holding the mouse button down and sliding it out of the main toolbar.
> > Once out of the toolbar, I can let up the mouse button without the
> > toolbar going away. Once stuck, if I simply slide the mouse across the
> > flying toolbar, it disappears. This does not happen under Windows.
>
> That is true. If release the left mouse button anywhere else except over the
> toolbars the flyout will stick. The reason is that TOOLBAR_LButtonUp()
> doesn't get executed in that case and the WM_COMMAND is not sent.
> (Everything works perfectly under any Windows as you have noticed because
> Win32 doesn't send the WM_COMMAND as I said before).

I think you misunderstood my explanation. Once the menu is stuck and the 
button released, I slide the mouse into the flying menu and back out 
again, without pressing any mouse buttons. When the cursor leaves the 
flying menu, it disappears, but without calling TOOLBAR_LButtonUp(). 
This is the second behavior, which may not be related.
> > Unfortunately
> > for me, my real work is once again interfering severely with the amount
> > of time I have to spend on Wine :-(
>
> I'd be all too happy to help. I will take off my own time to test & fix
> this. I'll try give you something over the weekend (patch).
> > But I will try to get in a couple
> > of tests with ControlSpy with the TBSTYLE_DROPDOWN style, since it
> > should not take much time, perhaps this weekend.

A suggestion I made a couple weeks ago would make things easier, I would 
think. That would be to duplicate your flying menus within ControlSpy. I 
don't know how easy that would be to implement, but once implemented, it 
would likely make debugging much easier.

[Dusan Vujosevic]

----- Original Message -----
From: "Duane Clark" 
To: 
Sent: Friday, April 19, 2002 11:41 AM
Subject: Re: toolbar.c problem (ATTN: Duane)
> I think you misunderstood my explanation. Once the menu is stuck and the
> button released, I slide the mouse into the flying menu and back out
> again, without pressing any mouse buttons. When the cursor leaves the
> flying menu, it disappears, but without calling TOOLBAR_LButtonUp().
> This is the second behavior, which may not be related.

I am working with the latest build of SL and this doesn't happen. I guess
you have the demo and that it does have that problem in which case it is
irrelevant since it has been fixed.
> > > Unfortunately
> > > for me, my real work is once again interfering severely with the amount
> > > of time I have to spend on Wine :-(
> >
> > I'd be all too happy to help. I will take off my own time to test & fix
> > this. I'll try give you something over the weekend (patch).
> > > But I will try to get in a couple
> > > of tests with ControlSpy with the TBSTYLE_DROPDOWN style, since it
> > > should not take much time, perhaps this weekend.
>
> A suggestion I made a couple weeks ago would make things easier, I would
> think. That would be to duplicate your flying menus within ControlSpy. I
> don't know how easy that would be to implement, but once implemented, it
> would likely make debugging much easier.

OK, that's what I'll do. I will implement it (not a big deal) and forward it
to you. Expect it this weekend some time..

Thanks,
Dusan

toolbar.c problem

[Rick Romero]

On Fri, 2002-04-19 at 13:14, Dusan Vujosevic wrote:
> ----- Original Message -----
> From: "Duane Clark" 
> To: 
> Sent: Friday, April 19, 2002 11:41 AM
> Subject: Re: toolbar.c problem (ATTN: Duane)
> > I think you misunderstood my explanation. Once the menu is stuck and the
> > button released, I slide the mouse into the flying menu and back out
> > again, without pressing any mouse buttons. When the cursor leaves the
> > flying menu, it disappears, but without calling TOOLBAR_LButtonUp().
> > This is the second behavior, which may not be related.
>
> I am working with the latest build of SL and this doesn't happen. I guess
> you have the demo and that it does have that problem in which case it is
> irrelevant since it has been fixed.
> > > > Unfortunately
> > > > for me, my real work is once again interfering severely with the amount
> > > > of time I have to spend on Wine :-(
> > >
> > > I'd be all too happy to help. I will take off my own time to test & fix
> > > this. I'll try give you something over the weekend (patch).
> > > > But I will try to get in a couple
> > > > of tests with ControlSpy with the TBSTYLE_DROPDOWN style, since it
> > > > should not take much time, perhaps this weekend.
> >
> > A suggestion I made a couple weeks ago would make things easier, I would
> > think. That would be to duplicate your flying menus within ControlSpy. I
> > don't know how easy that would be to implement, but once implemented, it
> > would likely make debugging much easier.
>
> OK, that's what I'll do. I will implement it (not a big deal) and forward it
> to you. Expect it this weekend some time..
>
> Thanks,
> Dusan

Forgive the intrusion, but I've been watching your thread, because it
sounds quite like bug # 434.
http://bugs.codeweavers.com/long_list.cgi?buglist=434

I believe it was your toolbar.c patch that initially fixed the toolbar
issues, but out of curiosity, before the patch, were you able to check
if it was a regression? (Maybe back to 6/20/01?) I found your comments(
http://www.winehq.com/hypermail/wine-devel/2002/03/0109.html ), but I
didn't see any comments indicating a regression test.

I think what you described is happening in other areas (see the bug),
with other apps, but for your app, it only manifested in the toolbars.

Rick

toolbar.c problem

[Duane Clark]

Rick Romero wrote:
> Forgive the intrusion, but I've been watching your thread, because it
> sounds quite like bug # 434.
> http://bugs.codeweavers.com/long_list.cgi?buglist=434

My own feeling is that this probably is not related, because my patch 
was pretty specific to mouse up events within the toolbar. The 
WM_LBUTTONUP message that you refer to in this bug is sent from many 
different routines, depending on where the mouse is in the app.

Also, this bug appears to be a problem of a missing mouse capture 
release. In my case, the cursor was being released just fine. It was 
just that the expected action was never invoked.
> I believe it was your toolbar.c patch that initially fixed the toolbar
> issues, but out of curiosity, before the patch, were you able to check
> if it was a regression? (Maybe back to 6/20/01?) I found your comments(
> http://www.winehq.com/hypermail/wine-devel/2002/03/0109.html ), but I
> didn't see any comments indicating a regression test.

My memory of events that far back is vague, but my recollection is that 
the toolbar problem with my two apps (fpga_editor and Designer) has been 
there since I started using wine, about May 2000. There were indeed a 
number of other regressions triggered by the June 2001 changes, so I do 
remember that date fairly well ;)
> I think what you described is happening in other areas (see the bug),
> with other apps, but for your app, it only manifested in the toolbars.

Possible, but I suspect just a coincidence. The TOOLBAR_LButtonUp() 
routine should only be called very specifically when the mouse is in the 
toolbar. It certainly is possible that there is a very similar problem 
in another routine, though.

toolbar.c problem

[Rick Romero]

On Tue, 2002-04-23 at 10:51, Duane Clark wrote:
> Rick Romero wrote:
> > Forgive the intrusion, but I've been watching your thread, because it
> > sounds quite like bug # 434.
> > http://bugs.codeweavers.com/long_list.cgi?buglist=434
>
> My own feeling is that this probably is not related, because my patch 
> was pretty specific to mouse up events within the toolbar. The 
> WM_LBUTTONUP message that you refer to in this bug is sent from many 
> different routines, depending on where the mouse is in the app.
> Also, this bug appears to be a problem of a missing mouse capture 
> release. In my case, the cursor was being released just fine. It was 
> just that the expected action was never invoked.
> From what I understand some applications (MS VC++ apps) will make some

of those calls directly.  It appears that my app (MS Visual FoxPro) does
this.. So I would assume that VFP itself is sending a LBUTTONUP call
whether it's run from Wine or Win9x or Win2k.  THEREFORE (caps to
indicate massive assumption coming:) the problem is the action isn't
invoked..  I support that assumption with my funky 'slowness' test (that
nobody else can replicate - how appropriate).   The action IS taken when
the system is ungodly slow.  <shrug>
> > I believe it was your toolbar.c patch that initially fixed the toolbar
> > issues, but out of curiosity, before the patch, were you able to check
> > if it was a regression? (Maybe back to 6/20/01?) I found your comments(
> > http://www.winehq.com/hypermail/wine-devel/2002/03/0109.html ), but I
> > didn't see any comments indicating a regression test.
>
> My memory of events that far back is vague, but my recollection is that 
> the toolbar problem with my two apps (fpga_editor and Designer) has been 
> there since I started using wine, about May 2000. There were indeed a 
> number of other regressions triggered by the June 2001 changes, so I do 
> remember that date fairly well ;)

Yes, for that bug, it appears to be something with the wineserver. 
Maybe it's also releated to the server serialization issues another user
posted.
> > I think what you described is happening in other areas (see the bug),
> > with other apps, but for your app, it only manifested in the toolbars.
>
> Possible, but I suspect just a coincidence. The TOOLBAR_LButtonUp() 
> routine should only be called very specifically when the mouse is in the 
> toolbar. It certainly is possible that there is a very similar problem 
> in another routine, though.

I hope it's not a coincidence, but unfortunately I don't know how you
got to the point you did to fix it :)

Rick

toolbar.c problem

[Duane Clark]

Rick Romero wrote:
> I hope it's not a coincidence, but unfortunately I don't know how you
> got to the point you did to fix it :)

Wasn't easy for someone with as little wine experience as me, that's for
sure :-)

Basically, fpga_editor actually had two different toolbars. The main one
did not work, but some sub-windows had a toolbar that did work. So I
traced the application in both cases, and compared what was different.
The difference was as I mentioned in the posting you linked to,
regarding the call of ReleaseCapture(). I could tell from the return
addresses in the traces that ReleaseCapture() was being called directly
by the application in the broken case, immediately before
TOOLBAR_LButtonUp(), but by wine in the working case.

From that point, it was a matter duplicating the procedure in a more
controlled environment. I had just recently figured how to use
ControlSpy (for some listview bugs), and that turned out to be an ideal
testbed for this too.