Re: [ros-dev] [ros-diffs] [mjmartin] 40630: - edit.c: Add TPM_RETURNCMD flag to TrackPopupMenu so that WM_COMMAND messages are not sent to the edit control. Use WM_MENUSELECT vice WM_COMMAND to determine which clipboard message to send. - Fixes vb apps edit context menu and maybe others. Bug #4404

[Timo Kreuzer]

edit.c is shared with wine. Also using a static variable here looks like
a bad idea.
I would think that the problem is somewhere else.

[e-mail] schrieb:
> Author: mjmartin
> Date: Tue Apr 21 08:45:08 2009
> New Revision: 40630
>
> URL: http://svn.reactos.org/svn/reactos?rev=40630&view=rev
> Log:
> edit.c: Add TPM_RETURNCMD flag to TrackPopupMenu so that WM_COMMAND messages are not sent to the edit control.
>
> Use WM_MENUSELECT vice WM_COMMAND to determine which clipboard message to send.
> Fixes vb apps edit context menu and maybe others. Bug #4404
>
> Modified:
>     trunk/reactos/dll/win32/user32/controls/edit.c
>
> Modified: trunk/reactos/dll/win32/user32/controls/edit.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/user32/controls/e...
> ==============================================================================
> --- trunk/reactos/dll/win32/user32/controls/edit.c [iso-8859-1] (original)
> +++ trunk/reactos/dll/win32/user32/controls/edit.c [iso-8859-1] Tue Apr 21 08:45:08 2009
> @@ -255,7 +255,7 @@
> WM_XXX message handlers
>
> */
>  static LRESULT	EDIT_WM_Char(EDITSTATE *es, WCHAR c);
> -static void	EDIT_WM_Command(EDITSTATE *es, INT code, INT id, HWND conrtol);
> +static void	EDIT_WM_MenuSelect(EDITSTATE *es, INT code, INT id, HWND conrtol);
>  static void	EDIT_WM_ContextMenu(EDITSTATE *es, INT x, INT y);
>  static void	EDIT_WM_Copy(EDITSTATE *es);
>  static LRESULT	EDIT_WM_Create(EDITSTATE *es, LPCWSTR name);
> @@ -947,8 +947,8 @@
>    	EDIT_WM_Clear(es);
>    	break;
> case WM_COMMAND:
> EDIT_WM_Command(es, HIWORD(wParam), LOWORD(wParam), (HWND)lParam);
>
> case WM_MENUSELECT:
> EDIT_WM_MenuSelect(es, HIWORD(wParam), LOWORD(wParam), (HWND)lParam);
>
> break;
> case WM_CONTEXTMENU:
>
> @@ -4224,12 +4224,17 @@
> WM_COMMAND
>
> */
> -static void EDIT_WM_Command(EDITSTATE *es, INT code, INT id, HWND control)
> -{
> if (code || control)
>
> +static void EDIT_WM_MenuSelect(EDITSTATE *es, INT code, INT id, HWND control)
> +{
> static INT MenuSelected;
> if (id != 0) {
> MenuSelected = id;
> return;
> switch (id) {
> }
> switch (MenuSelected) {
> case EM_UNDO:
>                     SendMessageW(es->hwndSelf, WM_UNDO, 0, 0);
> 	break;
>
> @@ -4308,7 +4313,7 @@
>              y = rc.top + (rc.bottom - rc.top) / 2;
>          }
> TrackPopupMenu(popup, TPM_LEFTALIGN | TPM_RIGHTBUTTON, x, y, 0, es->hwndSelf, NULL);
> TrackPopupMenu(popup, TPM_LEFTALIGN | TPM_RIGHTBUTTON | TPM_RETURNCMD, x, y, 0, es->hwndSelf, NULL);
> DestroyMenu(menu);
>
> }

[Michael Martin]

I submitted the same changes to wine. Do we need to revert and wait till they make the changes, which may very well be different?
Even though I usually frown upon static variables myself, can you explain why using a static variable here a bad idea?
Fairly certain the problems in edit.c, did tests on XP and edit controls do not receive WM_COMMAND from menu controls.

martin
> Date: Tue, 21 Apr 2009 17:48:41 +0200
> From: [e-mail]
> To: [e-mail]
> Subject: Re: [ros-dev] [ros-diffs] [mjmartin] 40630: - edit.c: Add TPM_RETURNCMD flag to TrackPopupMenu so that WM_COMMAND messages are not sent to the edit control. Use WM_MENUSELECT vice WM_COMMAND to determine which clipboard message to send. - Fixes vb apps edit context menu and maybe others. Bug #4404
>
> edit.c is shared with wine. Also using a static variable here looks like
> a bad idea.
> I would think that the problem is somewhere else.
>
> [e-mail] schrieb:
> > Author: mjmartin
> > Date: Tue Apr 21 08:45:08 2009
> > New Revision: 40630
> >
> > URL: http://svn.reactos.org/svn/reactos?rev=40630&view=rev
> > Log:
> > edit.c: Add TPM_RETURNCMD flag to TrackPopupMenu so that WM_COMMAND messages are not sent to the edit control.
> >
> > Use WM_MENUSELECT vice WM_COMMAND to determine which clipboard message to send.
> > Fixes vb apps edit context menu and maybe others. Bug #4404
> >
> > Modified:
> >     trunk/reactos/dll/win32/user32/controls/edit.c
> >
> > Modified: trunk/reactos/dll/win32/user32/controls/edit.c
> > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/user32/controls/e...
> > ==============================================================================
> > --- trunk/reactos/dll/win32/user32/controls/edit.c [iso-8859-1] (original)
> > +++ trunk/reactos/dll/win32/user32/controls/edit.c [iso-8859-1] Tue Apr 21 08:45:08 2009
> > @@ -255,7 +255,7 @@
> > WM_XXX message handlers
> >
> > */
> >  static LRESULT	EDIT_WM_Char(EDITSTATE *es, WCHAR c);
> > -static void	EDIT_WM_Command(EDITSTATE *es, INT code, INT id, HWND conrtol);
> > +static void	EDIT_WM_MenuSelect(EDITSTATE *es, INT code, INT id, HWND conrtol);
> >  static void	EDIT_WM_ContextMenu(EDITSTATE *es, INT x, INT y);
> >  static void	EDIT_WM_Copy(EDITSTATE *es);
> >  static LRESULT	EDIT_WM_Create(EDITSTATE *es, LPCWSTR name);
> > @@ -947,8 +947,8 @@
> >    	EDIT_WM_Clear(es);
> >    	break;
> > case WM_COMMAND:
> > EDIT_WM_Command(es, HIWORD(wParam), LOWORD(wParam), (HWND)lParam);
> >
> > case WM_MENUSELECT:
> > EDIT_WM_MenuSelect(es, HIWORD(wParam), LOWORD(wParam), (HWND)lParam);
> >
> > break;
> > case WM_CONTEXTMENU:
> >
> > @@ -4224,12 +4224,17 @@
> > WM_COMMAND
> >
> > */
> > -static void EDIT_WM_Command(EDITSTATE *es, INT code, INT id, HWND control)
> > -{
> > if (code || control)
> >
> > +static void EDIT_WM_MenuSelect(EDITSTATE *es, INT code, INT id, HWND control)
> > +{
> > static INT MenuSelected;
> > if (id != 0) {
> > MenuSelected = id;
> > return;
> > switch (id) {
> > }
> > switch (MenuSelected) {
> > case EM_UNDO:
> >                     SendMessageW(es->hwndSelf, WM_UNDO, 0, 0);
> > 	break;
> >
> > @@ -4308,7 +4313,7 @@
> >              y = rc.top + (rc.bottom - rc.top) / 2;
> >          }
> > TrackPopupMenu(popup, TPM_LEFTALIGN | TPM_RIGHTBUTTON, x, y, 0, es->hwndSelf, NULL);
> > TrackPopupMenu(popup, TPM_LEFTALIGN | TPM_RIGHTBUTTON | TPM_RETURNCMD, x, y, 0, es->hwndSelf, NULL);
> > DestroyMenu(menu);
> >
> > }
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

_________________________________________________________________
Rediscover Hotmail®: Get e-mail storage that grows with you. 
http://windowslive.com/RediscoverHotmail?ocid=TXT_TAGLM_WL_HM_Rediscover_Sto...

[Timo Kreuzer]

Michael Martin wrote:
> I submitted the same changes to wine. Do we need to revert and wait till they make the changes, which may very well be different?
> Even though I usually frown upon static variables myself, can you explain why using a static variable here a bad idea?

Because the code isn't thread safe.
> Fairly certain the problems in edit.c, did tests on XP and edit controls do not receive WM_COMMAND from menu controls.

The question is not if Windows sends WM_COMMAND, but if the control
handles WM_COMMAND. And have you ever seen Windows send a WM_MENUSELECT
to an edit control?
> martin
> > Date: Tue, 21 Apr 2009 17:48:41 +0200
> > From: [e-mail]
> > To: [e-mail]
> > Subject: Re: [ros-dev] [ros-diffs] [mjmartin] 40630: - edit.c: Add TPM_RETURNCMD flag to TrackPopupMenu so that WM_COMMAND messages are not sent to the edit control. Use WM_MENUSELECT vice WM_COMMAND to determine which clipboard message to send. - Fixes vb apps edit context menu and maybe others. Bug #4404
> >
> > edit.c is shared with wine. Also using a static variable here looks like
> > a bad idea.
> > I would think that the problem is somewhere else.
> >
> > [e-mail] schrieb:
> > > Author: mjmartin
> > > Date: Tue Apr 21 08:45:08 2009
> > > New Revision: 40630
> > >
> > > URL: http://svn.reactos.org/svn/reactos?rev=40630&view=rev
> > > Log:
> > > edit.c: Add TPM_RETURNCMD flag to TrackPopupMenu so that WM_COMMAND messages are not sent to the edit control.
> > >
> > > Use WM_MENUSELECT vice WM_COMMAND to determine which clipboard message to send.
> > > Fixes vb apps edit context menu and maybe others. Bug #4404
> > >
> > > Modified:
> > >     trunk/reactos/dll/win32/user32/controls/edit.c
> > >
> > > Modified: trunk/reactos/dll/win32/user32/controls/edit.c
> > > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/user32/controls/e...
> > > ==============================================================================
> > > --- trunk/reactos/dll/win32/user32/controls/edit.c [iso-8859-1] (original)
> > > +++ trunk/reactos/dll/win32/user32/controls/edit.c [iso-8859-1] Tue Apr 21 08:45:08 2009
> > > @@ -255,7 +255,7 @@
> > > WM_XXX message handlers
> > >
> > > */
> > >  static LRESULT	EDIT_WM_Char(EDITSTATE *es, WCHAR c);
> > > -static void	EDIT_WM_Command(EDITSTATE *es, INT code, INT id, HWND conrtol);
> > > +static void	EDIT_WM_MenuSelect(EDITSTATE *es, INT code, INT id, HWND conrtol);
> > >  static void	EDIT_WM_ContextMenu(EDITSTATE *es, INT x, INT y);
> > >  static void	EDIT_WM_Copy(EDITSTATE *es);
> > >  static LRESULT	EDIT_WM_Create(EDITSTATE *es, LPCWSTR name);
> > > @@ -947,8 +947,8 @@
> > >    	EDIT_WM_Clear(es);
> > >    	break;
> > > case WM_COMMAND:
> > > EDIT_WM_Command(es, HIWORD(wParam), LOWORD(wParam), (HWND)lParam);
> > >
> > > case WM_MENUSELECT:
> > > EDIT_WM_MenuSelect(es, HIWORD(wParam), LOWORD(wParam), (HWND)lParam);
> > >
> > > break;
> > > case WM_CONTEXTMENU:
> > >
> > > @@ -4224,12 +4224,17 @@
> > > WM_COMMAND
> > >
> > > */
> > > -static void EDIT_WM_Command(EDITSTATE *es, INT code, INT id, HWND control)
> > > -{
> > > if (code || control)
> > >
> > > +static void EDIT_WM_MenuSelect(EDITSTATE *es, INT code, INT id, HWND control)
> > > +{
> > > static INT MenuSelected;
> > > if (id != 0) {
> > > MenuSelected = id;
> > > return;
> > > switch (id) {
> > > }
> > > switch (MenuSelected) {
> > > case EM_UNDO:
> > >                     SendMessageW(es->hwndSelf, WM_UNDO, 0, 0);
> > > 	break;
> > >
> > > @@ -4308,7 +4313,7 @@
> > >              y = rc.top + (rc.bottom - rc.top) / 2;
> > >          }
> > > TrackPopupMenu(popup, TPM_LEFTALIGN | TPM_RIGHTBUTTON, x, y, 0, es->hwndSelf, NULL);
> > > TrackPopupMenu(popup, TPM_LEFTALIGN | TPM_RIGHTBUTTON | TPM_RETURNCMD, x, y, 0, es->hwndSelf, NULL);
> > > DestroyMenu(menu);
> > >
> > > }
> >
> > Ros-dev mailing list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Rediscover HotmailŽ: Get e-mail storage that grows with you. 
> http://windowslive.com/RediscoverHotmail?ocid=TXT_TAGLM_WL_HM_Rediscover_Sto...
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Michael Martin]

Because the code isn't thread safe.

Indeed sir you are correct.

The question is not if Windows sends WM_COMMAND, but if the control
handles WM_COMMAND.

I should have stated it a bit more clearly. From my tests, the edit windows context menu for clip board operations does not send the WM_COMMAND message to the owning edit control window. 
But i see you point, does the edit control do anything with WM_COMMAND messages? Either way wine edit control only handles clipboard operations for WM_COMMAND, which is incorrect.

And have you ever seen Windows send a WM_MENUSELECT
to an edit control?
    Yes for the edit windows context menu it sends the message every time a menu item is highlighted.

I redid the patch and submitted it to wine. Did something a bit more logical, send the clipboard message based on return value of TrackPopupMenu.

martin
Date: Wed, 22 Apr 2009 00:29:15 +0200
From: [e-mail]
To: [e-mail]
Subject: Re: [ros-dev] [ros-diffs] [mjmartin] 40630: - edit.c: Add TPM_RETURNCMD flag to TrackPopupMenu so that WM_COMMAND messages are not sent to the edit control. Use WM_MENUSELECT vice WM_COMMAND to determine which clipboard message to send. - Fixes vb apps e

Michael Martin wrote:

I submitted the same changes to wine. Do we need to revert and wait till they make the changes, which may very well be different?
Even though I usually frown upon static variables myself, can you explain why using a static variable here a bad idea?

Because the code isn't thread safe.

Fairly certain the problems in edit.c, did tests on XP and edit controls do not receive WM_COMMAND from menu controls.

The question is not if Windows sends WM_COMMAND, but if the control
handles WM_COMMAND. And have you ever seen Windows send a WM_MENUSELECT
to an edit control?

martin

Date: Tue, 21 Apr 2009 17:48:41 +0200
From: [e-mail]
To: [e-mail]
Subject: Re: [ros-dev] [ros-diffs] [mjmartin] 40630: - edit.c: Add TPM_RETURNCMD flag to TrackPopupMenu so that WM_COMMAND messages are not sent to the edit control. Use WM_MENUSELECT vice WM_COMMAND to determine which clipboard message to send. - Fixes vb apps edit context menu and maybe others. Bug #4404

edit.c is shared with wine. Also using a static variable here looks like
a bad idea.
I would think that the problem is somewhere else.

_________________________________________________________________
Rediscover Hotmail®: Get e-mail storage that grows with you. 
http://windowslive.com/RediscoverHotmail?ocid=TXT_TAGLM_WL_HM_Rediscover_Sto...