How do I cleanly avoid creating a context menu inside a context menu?-CodePudding

I am adding a context menu to my window, and I seem to have discovered a quirk of Win32: because a popup menu forwards mouse messages to its owner window, and the default behavior of WM_RBUTTONUP is to send WM_CONTEXTMENU, if you open a popup menu in the handler for WM_CONTEXTMENU, you can right-click the menu to open another one. This fails because a menu is already open.

This seems like a really obvious, common bug but even the official WM_CONTEXTMENU guide from Microsoft doesn't even mention it. Nor does any other tutorial.

So, what is the cleanest way to not try to open a popup menu inside another one?

Of course, one can simply add a global flag to indicate that a menu is open, but that's not clean. One can also ignore the error from TrackPopupMenuEx, but that's also not clean. I can't imagine that the Win32 (horrendous as it is) would encourage programmers to do such things.

I investigated how some common controls behave:

The edit control doesn't pass WM_RBUTTONUP to DefWindowProc unless it received a previous WM_RBUTTONDOWN. Because the menu system handles WM_RBUTTONDOWN internally and does not dispatch it, this means it doesn't pass WM_RBUTTONUP to DefWindowProc when a menu is open.
The tree view in the Microsoft Management Console creates a temporary window to associate the menu with.
The details list view in Task Manager does receive a recursive WM_CONTEXTMENU; I assume that it tries to create the menu and ignores the error.

This test program demonstrates the phenomenon:

#include <Windows.h>
#include <iostream>

static LRESULT CALLBACK window_proc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) {
    switch(msg) {
    case WM_PAINT:
        {
            PAINTSTRUCT ps;
            HDC hdc = BeginPaint(hwnd, &ps);
            EndPaint(hwnd, &ps);
        }
        return 0;
    case WM_RBUTTONDOWN:
        std::cout << "got WM_RBUTTONDOWN\n";
        return DefWindowProc(hwnd, msg, wParam, lParam);
    case WM_RBUTTONUP:
        std::cout << "got WM_RBUTTONUP\n";
        return DefWindowProc(hwnd, msg, wParam, lParam);
    case WM_CONTEXTMENU:
        std::cout << "got WM_CONTEXTMENU\n";
        {
            HMENU hm = CreatePopupMenu();
            AppendMenu(hm, MF_STRING, 1234, L"Hello");
            AppendMenu(hm, MF_STRING, 4321, L"world");
            POINT pt;
            GetCursorPos(&pt);
            SetLastError(0);
            int res = TrackPopupMenuEx(hm, TPM_LEFTALIGN | TPM_TOPALIGN | TPM_NONOTIFY | TPM_RETURNCMD, pt.x, pt.y, hwnd, NULL);
            int err = GetLastError();
            std::cout << "TrackPopupMenuEx returned " << res << ", error " << err << std::endl;
            SetLastError(0);
        }
        return 0;
    default:
        return DefWindowProc(hwnd, msg, wParam, lParam);
    }
}

int main()
{
    WNDCLASSEX wcx = {0};
    wcx.cbSize = sizeof wcx;
    wcx.hbrBackground = (HBRUSH)(COLOR_BTNFACE   1);
    wcx.hCursor = LoadCursor(NULL, IDC_ARROW);
    wcx.hIcon = LoadIcon(NULL, IDI_APPLICATION);
    wcx.hInstance = (HINSTANCE)GetModuleHandle(NULL);
    wcx.lpfnWndProc = window_proc;
    wcx.lpszClassName = L"TestWindowClass";
    RegisterClassEx(&wcx);
    CreateWindowEx(0, wcx.lpszClassName, L"Popup menu message test", WS_VISIBLE | WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, 300, 300, NULL, NULL, wcx.hInstance, NULL);
    
    MSG msg;
    while(GetMessage(&msg, NULL, 0, 0) > 0) {
        TranslateMessage(&msg);
        DispatchMessage(&msg);
    }
    return 0;
}

CodePudding user response：

From documentation

TrackPopupMenuEx

... If you specify TPM_RETURNCMD in the fuFlags parameter, the return value is the menu-item identifier of the item that the user selected. If the user cancels the menu without making a selection, or if an error occurs, the return value is zero.

If popup menu is already open and user right-clicked it, or the user cancels the menu, the function returns FALSE. In either case you don't want to process the result, so just return immediately. If res is non-zero, process it.

BOOL res = TrackPopupMenuEx(hm, 
    TPM_LEFTALIGN | TPM_TOPALIGN | TPM_NONOTIFY | TPM_RETURNCMD,
    pt.x, pt.y, hwnd, NULL);
if (!res) return 0;
std::cout << "Menu selection: " << res << "\n";

Or you can use GetLastError to find the reason for failure. Was it because user canceled, right-click the menu, or something else

if (!res)
{
    int err = GetLastError();
    if (err != 0 && err != ERROR_POPUP_ALREADY_ACTIVE)
        std::cout << "Unexpected error " << err << "\n";
    return 0;
}

CodePudding user response：

I suppose TrackPopupMenu is one of those functions that nobody ever checks for errors. After all, as long as the input parameters are valid, it is only going to fail (in the normal non-recursive case) if you are out of resources (memory or gdi/user handles). And if it fails, MessageBox is probably also going to fail so you can't even inform the user.

The normal coding pattern is to just check for your menu command IDs and ignore everything else.

The TPM_RECURSE flag which was added in the IE 4/5 era allows you to put a popup menu on a popup menu but nobody ever does that.