Resubmit user32: TrackPopupMenu(Ex) fixes

Rein Klazes wijn at online.nl
Fri Mar 6 07:48:47 CST 2009


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.
> -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.

> -    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.

Rein.



More information about the wine-devel mailing list