Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 21, 2024
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
37213a9
[Explorer] Don't use ID_SHELL_CMD IDs with CFSFolder menu
whindsaks May 12, 2024
e53b6ab
Invert condition to better maintain lines in diff
whindsaks May 12, 2024
e216405
Comment style
whindsaks May 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions base/shell/explorer/resource.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
78 changes: 42 additions & 36 deletions base/shell/explorer/startctxmnu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ourself 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;

Expand All @@ -49,26 +55,26 @@ class CStartMenuBtnCtxMenu :
hRet = m_Inner->QueryContextMenu(
hPopup,
0,
ID_SHELL_CMD_FIRST,
ID_SHELL_CMD_LAST,
idCmdFirst,
idCmdLast,
CMF_VERBSONLY);

if (SUCCEEDED(hRet))
{
return hRet;
}

DestroyMenu(hPopup);
}
}

return E_FAIL;
}

VOID AddStartContextMenuItems(IN HMENU hPopup)
{
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,
Expand Down Expand Up @@ -148,13 +154,13 @@ class CStartMenuBtnCtxMenu :
{
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));
Expand All @@ -168,49 +174,48 @@ class CStartMenuBtnCtxMenu :
hRet = psfDesktop->BindToObject(pidlStart, NULL, IID_PPV_ARG(IShellFolder, &m_Folder));
if (SUCCEEDED(hRet))
{
hRet = CreateContextMenuFromShellFolderPidl(hPopup);
m_idCmdCmLast = (SUCCEEDED(hRet)) ? HRESULT_CODE(hRet) : ID_SHELL_CMD_LAST;
AddStartContextMenuItems(hPopup);
hRet = CreateContextMenuFromShellFolderPidl(hPopup, idInnerFirst, idCmdLast);
}
}
}

ILFree(pidlStart);
}

return S_OK;
if (idCmdLast - idCmdFirst >= ID_SHELL_CMD_LAST - ID_SHELL_CMD_FIRST)
{
AddStartContextMenuItems(hPopup);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this code block be run even if the CreateContextMenuFromShellFolderPidl() call from above failed (or wasn't executed because either pidlStart was NULL or some of the if-tests failed) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you would then end up with a menu that just has Properties, Separator, Open/Explore All Users.

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];

/* Setup and invoke the shell command */
cmici.cbSize = sizeof(cmici);
cmici.hwnd = m_Owner;
cmici.lpVerb = MAKEINTRESOURCEA(uiCmdId - ID_SHELL_CMD_FIRST);
cmici.nShow = SW_NORMAL;

/* FIXME: Support Unicode!!! */
if (SHGetPathFromIDListA(m_FolderPidl, szDir))
{
cmici.lpDirectory = szDir;
}

m_Inner->InvokeCommand(&cmici);
}
CMINVOKECOMMANDINFO cmici = { 0 };
CHAR szDir[MAX_PATH];

/* Setup and invoke the shell command */
cmici.cbSize = sizeof(cmici);
cmici.hwnd = m_Owner;
if (IS_INTRESOURCE(lpici->lpVerb))
cmici.lpVerb = MAKEINTRESOURCEA(uiCmdId - INNERIDOFFSET);
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;
}

Expand All @@ -221,12 +226,13 @@ class CStartMenuBtnCtxMenu :
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()
Expand Down
5 changes: 2 additions & 3 deletions base/shell/explorer/traywnd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3763,6 +3763,8 @@ class CTrayWindowCtxMenu :
HMENU hMenuBase;

hMenuBase = LoadPopupMenu(hExplorerInstance, MAKEINTRESOURCEW(IDM_TRAYWND));
if (!hMenuBase)
return HResultFromWin32(GetLastError());

if (g_MinimizedAll.GetSize() != 0 && !::IsThereAnyEffectiveWindow(TRUE))
{
Expand All @@ -3775,9 +3777,6 @@ class CTrayWindowCtxMenu :
SetMenuItemInfoW(hMenuBase, ID_SHELL_CMD_SHOW_DESKTOP, FALSE, &mii);
}

if (!hMenuBase)
return HRESULT_FROM_WIN32(GetLastError());

if (SHRestricted(REST_CLASSICSHELL) != 0)
{
DeleteMenu(hPopup,
Expand Down