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.

mjmartin(a)svn.reactos.org 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/…
>  ==============================================================================
>  --- 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: timo.kreuzer(a)web.de
>  To: ros-dev(a)reactos.org
>  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.
>  
>  mjmartin(a)svn.reactos.org 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/…
> >  ==============================================================================
> >  --- 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
>  Ros-dev(a)reactos.org
>  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_St…

[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: timo.kreuzer(a)web.de
> >  To: ros-dev(a)reactos.org
> >  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.
> >
> >  mjmartin(a)svn.reactos.org 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/…
> > >  ==============================================================================
> > >  --- 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
> >  Ros-dev(a)reactos.org
> >  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_St…
>    
>  ------------------------------------------------------------------------
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  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: timo.kreuzer(a)web.de
To: ros-dev(a)reactos.org
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: timo.kreuzer(a)web.de
To: ros-dev(a)reactos.org
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_St…