[EXPLORER] Don't use Explorers ID_SHELL_CMD IDs in IShellFolder menu (#6872)

Explorer does not control the IDs used by CFSFolder::QueryContext menu and therefore cannot share its ID range with it. The range passed to CFSFolder::QueryContext also cannot start at 0 because CTrayWindow::TrackCtxMenu would interpret that as user cancel.

base/shell/explorer/resource.h

@@ -201,8 +201,6 @@
 #define IDMA_RESTORE_OPEN 416
 #define IDMA_MINIMIZE_ALL 419
 
-#define ID_SHELL_CMD_FIRST              0xF
-#define ID_SHELL_CMD_LAST               0x7FEF
 #define ID_SHELL_CMD_PROPERTIES         (401)
 #define ID_SHELL_CMD_OPEN_ALL_USERS     (402)
 #define ID_SHELL_CMD_EXPLORE_ALL_USERS  (403)
@@ -216,3 +214,5 @@
 #define ID_SHELL_CMD_CUST_NOTIF         (411)
 #define ID_SHELL_CMD_ADJUST_DAT         (412)
 #define ID_SHELL_CMD_RESTORE_ALL        (413)
+#define ID_SHELL_CMD_FIRST              ID_SHELL_CMD_PROPERTIES
+#define ID_SHELL_CMD_LAST               ID_SHELL_CMD_RESTORE_ALL

base/shell/explorer/startctxmnu.cpp

@@ -29,15 +29,21 @@ class CStartMenuBtnCtxMenu :
     public CComObjectRootEx<CComMultiThreadModelNoCS>,
     public IContextMenu
 {
+    /* AddStartContextMenuItems uses ID_SHELL_CMD IDs directly and relies on idCmdFirst being 0.
+     * CTrayWindow::TrackCtxMenu must pass 0 because DeleteMenu ID_SHELL_CMD_UNDO_ACTION would
+     * delete the wrong item if it used 1. m_Inner->QueryContextMenu is not aware of this game
+     * so we have to reserve the entire ID_SHELL_CMD range for ourselves here. */
+    enum { INNERIDOFFSET = ID_SHELL_CMD_LAST + 1 };
+    static BOOL IsShellCmdId(UINT_PTR id) { return id < INNERIDOFFSET; }
+
     CComPtr<ITrayWindow>  m_TrayWnd;
     CComPtr<IContextMenu> m_Inner;
     CComPtr<IShellFolder> m_Folder;
-    UINT m_idCmdCmLast;
 
     HWND m_Owner;
     LPITEMIDLIST m_FolderPidl;
 
-    HRESULT CreateContextMenuFromShellFolderPidl(HMENU hPopup)
+    HRESULT CreateContextMenuFromShellFolderPidl(HMENU hPopup, UINT idCmdFirst, UINT idCmdLast)
     {
         HRESULT hRet;
 
@@ -49,19 +55,16 @@ class CStartMenuBtnCtxMenu :
                 hRet = m_Inner->QueryContextMenu(
                     hPopup,
                     0,
-                    ID_SHELL_CMD_FIRST,
+                    idCmdFirst,
-                    ID_SHELL_CMD_LAST,
+                    idCmdLast,
                     CMF_VERBSONLY);
 
                 if (SUCCEEDED(hRet))
                 {
                     return hRet;
                 }
-
-                DestroyMenu(hPopup);
             }
         }
-
         return E_FAIL;
     }
 
@@ -69,6 +72,9 @@ class CStartMenuBtnCtxMenu :
     {
         WCHAR szBuf[MAX_PATH];
         HRESULT hRet;
+        C_ASSERT(ID_SHELL_CMD_FIRST != 0);
+         /* If this ever asserts, let m_Inner use 1..ID_SHELL_CMD_FIRST-1 instead */
+        C_ASSERT(ID_SHELL_CMD_LAST < 0xffff / 2);
 
         /* Add the "Open All Users" menu item */
         if (LoadStringW(hExplorerInstance,
@@ -148,13 +154,13 @@ public:
     {
         LPITEMIDLIST pidlStart;
         CComPtr<IShellFolder> psfDesktop;
-        HRESULT hRet;
+        HRESULT hRet = S_OK;
+        UINT idInnerFirst = idCmdFirst + INNERIDOFFSET;
 
         psfDesktop = NULL;
         m_Inner = NULL;
 
         pidlStart = SHCloneSpecialIDList(m_Owner, CSIDL_STARTMENU, TRUE);
-
         if (pidlStart != NULL)
         {
             m_FolderPidl = ILClone(ILFindLastID(pidlStart));
@@ -168,49 +174,48 @@ public:
                     hRet = psfDesktop->BindToObject(pidlStart, NULL, IID_PPV_ARG(IShellFolder, &m_Folder));
                     if (SUCCEEDED(hRet))
                     {
-                        hRet = CreateContextMenuFromShellFolderPidl(hPopup);
+                        hRet = CreateContextMenuFromShellFolderPidl(hPopup, idInnerFirst, idCmdLast);
-                        m_idCmdCmLast = (SUCCEEDED(hRet)) ? HRESULT_CODE(hRet) : ID_SHELL_CMD_LAST;
-                        AddStartContextMenuItems(hPopup);
                     }
                 }
             }
 
             ILFree(pidlStart);
         }
-
+        if (idCmdLast - idCmdFirst >= ID_SHELL_CMD_LAST - ID_SHELL_CMD_FIRST)
-        return S_OK;
+        {
+            AddStartContextMenuItems(hPopup);
+            hRet = SUCCEEDED(hRet) ? hRet + idInnerFirst : idInnerFirst;
+        }
+        return hRet;
     }
 
     virtual HRESULT STDMETHODCALLTYPE
         InvokeCommand(LPCMINVOKECOMMANDINFO lpici)
     {
         UINT uiCmdId = PtrToUlong(lpici->lpVerb);
-        if (uiCmdId != 0)
+        if (!IsShellCmdId((UINT_PTR)lpici->lpVerb))
         {
-            if ((uiCmdId >= ID_SHELL_CMD_FIRST) && (uiCmdId < m_idCmdCmLast))
+            CMINVOKECOMMANDINFO cmici = { 0 };
-            {
+            CHAR szDir[MAX_PATH];
-                CMINVOKECOMMANDINFO cmici = { 0 };
-                CHAR szDir[MAX_PATH];
 
-                /* Setup and invoke the shell command */
+            /* Setup and invoke the shell command */
-                cmici.cbSize = sizeof(cmici);
+            cmici.cbSize = sizeof(cmici);
-                cmici.hwnd = m_Owner;
+            cmici.hwnd = m_Owner;
-                cmici.lpVerb = MAKEINTRESOURCEA(uiCmdId - ID_SHELL_CMD_FIRST);
+            if (IS_INTRESOURCE(lpici->lpVerb))
-                cmici.nShow = SW_NORMAL;
+                cmici.lpVerb = MAKEINTRESOURCEA(uiCmdId - INNERIDOFFSET);
-
-                /* FIXME: Support Unicode!!! */
-                if (SHGetPathFromIDListA(m_FolderPidl, szDir))
-                {
-                    cmici.lpDirectory = szDir;
-                }
-
-                m_Inner->InvokeCommand(&cmici);
-            }
             else
+                cmici.lpVerb = lpici->lpVerb;
+            cmici.nShow = SW_NORMAL;
+
+            /* FIXME: Support Unicode!!! */
+            if (SHGetPathFromIDListA(m_FolderPidl, szDir))
             {
-                m_TrayWnd->ExecContextMenuCmd(uiCmdId);
+                cmici.lpDirectory = szDir;
             }
+
+            return m_Inner->InvokeCommand(&cmici);
         }
+        m_TrayWnd->ExecContextMenuCmd(uiCmdId);
         return S_OK;
     }
 
@@ -221,12 +226,13 @@ public:
                          LPSTR pszName,
                          UINT cchMax)
     {
+        if (!IsShellCmdId(idCmd) && m_Inner)
+            return m_Inner->GetCommandString(idCmd, uType, pwReserved, pszName, cchMax);
         return E_NOTIMPL;
     }
 
     CStartMenuBtnCtxMenu()
     {
-        m_idCmdCmLast = ID_SHELL_CMD_LAST;
     }
 
     virtual ~CStartMenuBtnCtxMenu()

base/shell/explorer/traywnd.cpp

@@ -3763,6 +3763,8 @@ public:
         HMENU hMenuBase;
 
         hMenuBase = LoadPopupMenu(hExplorerInstance, MAKEINTRESOURCEW(IDM_TRAYWND));
+        if (!hMenuBase)
+            return HResultFromWin32(GetLastError());
 
         if (g_MinimizedAll.GetSize() != 0 && !::IsThereAnyEffectiveWindow(TRUE))
         {
@@ -3775,9 +3777,6 @@ public:
             SetMenuItemInfoW(hMenuBase, ID_SHELL_CMD_SHOW_DESKTOP, FALSE, &mii);
         }
 
-        if (!hMenuBase)
-            return HRESULT_FROM_WIN32(GetLastError());
-
         if (SHRestricted(REST_CLASSICSHELL) != 0)
         {
             DeleteMenu(hPopup,