Resubmit user32: TrackPopupMenu(Ex) fixes

[Reece Dunn]

2009/3/6 Rein Klazes <wijn at online.nl>:
> Reece Dunn schreef:
>>
>> = Implementation Comments:
>>
>> +    /* FIXME: this check is performed several times, here and in the
>> called
>> +       functions. That could be optimized */
>> +    if( !hMenu || !MENU_GetMenu( hMenu )) {
>> +        SetLastError( ERROR_INVALID_MENU_HANDLE) ;
>> +        return FALSE;
>> +    }
>>
>> Which called functions are doing this check? You should do the
>> SetLastError call there instead (in MENU_TrackMenu?).
>
> No, the check must be here, before any message's have been sent.

So do the checks in MENU_InitTracking instead.

>> -BOOL WINAPI TrackPopupMenuEx( HMENU hMenu, UINT wFlags, INT x, INT y,
>> -                                HWND hWnd, LPTPMPARAMS lpTpm )
>> +BOOL WINAPI TrackPopupMenu( HMENU hMenu, UINT wFlags, INT x, INT y,
>> +        INT nReserved, HWND hWnd, const RECT *lpRect )
>>  {
>> -    FIXME("not fully implemented\n" );
>>
>> Why are you removing this FIXME? Do you know that your changes fully
>> implement TPMEx (which does not have the FIXME in your patch)?
>
> No. But the FIXME is  referring to what I consider  a cosmetic feature.
> Since I noticed that it is confusing users, that link these messages to
> the crashes and other nasties that I am trying to fix, I decided to not to
> bother them with it any more. But a WARN might be considered if you
> realy realy want it.

Ok. So remove the FIXME in a separate patch.

>> -    return TrackPopupMenu( hMenu, wFlags, x, y, 0, hWnd,
>> -                             lpTpm ? &lpTpm->rcExclude : NULL );
>> +    return TrackPopupMenuEx( hMenu, wFlags, x, y, hWnd, NULL);
>>
>> What is the rationale behind calling TPMEx from TPM? Is it so you can
>> properly implement something in TPMEx that is not provided in TPM? If
>> so, are there any tests to show that there are differences that
>> require this change?
>
> I think here the MSDN documentation should  suffice for now. the
> Ex version does implement the excluded rectangle, the plain version
> ignores it.

MSDN states that for TrackPopupMenu
(http://msdn.microsoft.com/en-us/library/ms648002%28VS.85%29.aspx)
nReserved must be 0 and lpRect is ignored. TrackPopupMenuEx has an
optional (LP)TPMPARAMS structure that just passes an exclude RECT.

Calling TrackPopupMenuEx from TrackPopupMenu with LPTPMPARAMS being
NULL is a valid option. But so is passing the current Wine
implementation (possibly using a value of 1 or something in the
reserved value to signal that this is internal Wine code, not external
application code).

You need tests to prove this, though. Until you do, the existing Wine
implementation is valid. So:
1.  does TrackPopupMenu really ignore the RECT argument?
2.  what happens if you pass a nReserved value that is not 0?

- Reece