v2.7.0: Fix verb truncation, & stripping, atomic SendInput; enhance context menu matching

[LiHua81]

Summary

v2.7.0 update for Click on Empty Explorer (#4500).

Bugfixes

• Verb truncation: GCS_VERB → GCS_VERBA — Unicode build was writing wide chars into CHAR* buffer, truncating all verbs to 1 character
• & markers: StrContainsNorm now strips & so "Git Bash" matches "Open Git Ba&sh here"
• SendInput: merged split press/release calls into single atomic call

Improvements

• Perf: subclass procs skip CopySettings() for unhandled messages
• Debug: dump shows normalized match text per entry (→ match:)
• Docs: expanded Context Menu Match section with rules table

[windhawk-reviewer]

Thanks for the pull request! This repository uses a two-stage review: an AI review that you run yourself, followed by a human review.

To get started, comment /ai-review. Once you're happy with the result, comment /ready-for-reviewer to hand it over to a human reviewer.

See the pull request review process for the full details.

[Copilot]

Pull request overview

This PR updates the Click on Empty Explorer Windhawk mod to expand configurable click triggers (triple-click and modifier+click), make hotkey injection more reliable by batching SendInput, and add new actions that invoke real Explorer background context menu items (e.g., “Open in VS Code/Terminal/Cursor” or any matched entry).

Changes:

• Add triple-click + Ctrl/Alt/Shift+click triggers with corresponding settings and action dispatch.
• Implement context-menu based launching by enumerating and invoking a matched background context menu entry (with normalization that ignores spaces and &).
• Make hotkey injection “atomic” by sending press+release in a single SendInput call.

Suppressed comments (1)

mods/click-on-empty-explorer.wh.cpp:656

• Same issue as above in the diagnostic dump: MIIM_TYPE is deprecated and not needed to retrieve wID/hSubMenu. Dropping it avoids incidental string/type retrieval semantics and keeps the struct usage simpler.

        MENUITEMINFOW mii = { sizeof(mii) };
        mii.fMask = MIIM_ID | MIIM_TYPE | MIIM_SUBMENU;
        if (!GetMenuItemInfoW(hMenu, i, TRUE, &mii)) continue;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[LiHua81]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

The review covers the whole file, not only the diff — several of the items below are inherited from the 2.3.0 base (and from explorer-double-click-up.wh.cpp, which this is based on), but they are real and worth fixing while you're in here.

1. Subclasses installed after init are never recorded, so Wh_ModUninit doesn't remove them → Explorer crash on disable/update.

CreateWindowExW_hook installs the subclass unconditionally, but records the HWND only into an already existing g_Wrappers entry:

if (wcscmp(className, L"SysListView32") == 0) {
    WindhawkUtils::SetWindowSubclassFromAnyThread(hWnd, SysListViewSubclass, 0);
    { std::lock_guard<std::mutex> lk(g_wrappersMutex);
      for (auto& w : g_Wrappers)
        if (w.hShellTab == shellTab) { w.hListView = hWnd; break; }   // no entry yet
    }
}

For a freshly opened Explorer window/tab that entry does not exist yet: FileCabinet_CreateViewWindow2Hook calls the original first, and the SHELLDLL_DefView / SysListView32 / DirectUIHWND are created inside that call, so CreateWindowExW_hook runs before the g_Wrappers.push_back(...) that follows it. The wrapper is then stored with hListView == NULL, and Wh_ModUninit only removes subclasses for entries with a non-null hListView.

Repro: enable the mod, open a new Explorer window, disable (or update) the mod, click in that window → the subclass proc is invoked in the unmapped mod image. Only windows that already existed at Wh_ModAfterInit time (enumerated by InitEnumChildWindowsProc, which does set hListView) get cleaned up.

Fix: track subclassed windows in their own container at the moment you subclass, independent of g_Wrappers, and drop the entry from the subclass proc on WM_NCDESTROY:

static std::mutex g_subclassMutex;
static std::vector<std::pair<HWND, bool /*isListView*/>> g_subclassed;

// in CreateWindowExW_hook, after a successful SetWindowSubclassFromAnyThread:
{ std::lock_guard<std::mutex> lk(g_subclassMutex); g_subclassed.push_back({hWnd, true}); }

Then in Wh_ModUninit iterate that container with the same copy-under-lock-then-call pattern you already use.

2. Timer callbacks live in the mod image and can survive the unload.

SetTimer(hWnd, 0x4D43/0x4D44/0x4D45, ..., NavigateNewTabProc / MidClickTimerProc / DblClickTimerProc) registers callbacks that live in the mod DLL, on windows owned by Explorer UI threads. Wh_ModUninit runs on an arbitrary thread and tries to cancel them with KillTimer — window timers belong to the thread that created them, so a cross-thread KillTimer is not something to rely on. Worse, only one pending mid-click and one pending double-click timer is tracked (single globals), so a timer armed by a second Explorer window is never cancelled at all. Anything that survives gets dispatched into unloaded code.

Two things make it worse:

• DblClickTimerProc and NavigateNewTabProc return before KillTimer when g_initialized == 0, so a timer that does fire during teardown just stays armed and keeps firing:

static VOID CALLBACK DblClickTimerProc(HWND hwnd, UINT, UINT_PTR idEvent, DWORD) {
    CHECK_INIT_OR_RETURN_VOID();   // <-- timer is left running
    KillTimer(hwnd, idEvent);

MidClickTimerProc gets this right — do the same in the other two.

• Cleanest fix: don't use a TIMERPROC from the mod image at all. Use SetTimer(hWnd, id, ms, nullptr) and handle WM_TIMER in the subclass proc — the subclass is removed at uninit, and a stray WM_TIMER after removal falls through to DefWindowProc harmlessly. Several mods do this, e.g. classic-explorer-statusbar.wh.cpp#L317. Note the duplicate-tab timer (0x4D43) is set on the ShellTabWindowClass window, which is not subclassed — either subclass it too, or set that timer on the (subclassed) list view / def view instead.

3. The IShellBrowser obtained via CWM_GETISHELLBROWSER is over-released.

auto browser = winrt::com_ptr<IShellBrowser>{
    reinterpret_cast<IShellBrowser*>((void*)SendMessage(shellTab, WM_USER + 7, 0, 0)),
    winrt::take_ownership_from_abi
};

CWM_GETISHELLBROWSER returns a borrowed pointer — it does not AddRef. Every other mod in the repo treats it that way and never releases it (explorer-status-metadata.wh.cpp#L591, add-virtual-folders-to-nav-top.wh.cpp#L2569, paste-clipboard-content-to-explorer.wh.cpp#L533). take_ownership_from_abi claims a reference that was never granted: the local com_ptr releases at end of scope, cancelling out the copy_from inside ExplorerWrapper, so the wrapper holds a strong pointer with no reference behind it — and g_Wrappers.clear() in Wh_ModUninit then decrements a count that was never incremented. Under-counting CShellBrowser can free it out from under Explorer.

Fix — drop take_ownership_from_abi and take a real reference:

winrt::com_ptr<IShellBrowser> browser;
browser.copy_from(reinterpret_cast<IShellBrowser*>(
    (void*)SendMessage(shellTab, WM_USER + 7, 0, 0)));

4. Per-window click state is kept in process-wide globals, but each Explorer window runs on its own thread.

g_currentClick / g_lastClick, g_pendingDblClickHwnd / g_pendingDblClickTimerId / g_pendingDblClickAction / g_pendingDblClickCombo, g_midClickPendingHwnd / g_midClickTimerId, and g_pendingNavPath / g_pendingNavBrowser are written and read from the subclass procs of different CabinetWClass windows, each of which lives on its own Explorer thread — with no synchronization anywhere.

This PR makes it materially worse: ClickHelper::className changed from wchar_t[256] to std::wstring, and g_pendingDblClickAction/g_pendingDblClickCombo are new std::wstring globals. A fixed buffer raced benignly; a std::wstring assignment frees the old heap buffer, so CancelPendingDblClick() on window B's thread can free the buffer that window A's DblClickTimerProc is reading through g_pendingDblClickAction.c_str(). Two Explorer windows in use at once is the normal case.

There is a functional side to it too: CancelPendingDblClick() from window B clears window A's pending state, so a double-click in one window can be silently swallowed by activity in another.

Fix: make this state per-window. thread_local is the cheapest change here (one Explorer window == one thread), or stash it in the subclass's dwRefData. If you keep globals, all of it needs a mutex.

5. g_Wrappers is never pruned — unbounded growth, leaked browsers, stale lookups.

FileCabinet_CreateViewWindow2Hook does g_Wrappers.push_back(...) on every view creation (so on every navigation, every new tab, every new window) and nothing ever removes an entry. Over a session the vector grows without bound and holds strong IShellBrowser references for long-closed tabs, so Explorer never reclaims them. And since FindShellTabAndDoAction takes the first entry matching hShellTab, once Windows recycles a closed tab's HWND the action is dispatched to the old, dead browser and silently does nothing.

Fix — drop dead/duplicate entries before inserting:

std::lock_guard<std::mutex> lock(g_wrappersMutex);
std::erase_if(g_Wrappers, [&](const ExplorerWrapper& w) {
    return w.hShellTab == shellTab || !IsWindow(w.hShellTab);
});
g_Wrappers.push_back(ExplorerWrapper(shellTab, pBrowser));

Removing the entry on the shell tab's WM_NCDESTROY would be tidier still. Alternatively, don't cache IShellBrowser at all — you already have CWM_GETISHELLBROWSER, so g_Wrappers could hold just the HWNDs and query the browser at action time, which makes items 3, 5 and 6 all go away.

6. Globals holding COM pointers run their destructors at process shutdown.

std::vector<ExplorerWrapper> g_Wrappers (each element owns a winrt::com_ptr<IShellBrowser>) and winrt::com_ptr<IShellBrowser> g_pendingNavBrowser are globals with non-trivial destructors. Wh_ModUninit is not called when explorer.exe itself terminates (Explorer restart, sign-out, reboot) — but the CRT still runs those destructors, on the shutdown thread, after every other thread has already been killed. That means IShellBrowser::Release() on STA objects whose owning threads are gone. See https://github.com/ramensoftware/windhawk/wiki/Global-objects-and-process-shutdown — sections 3. WinRT or COM object and 5. Containers of resource-owning or thread-affine elements.

Fix — suppress the automatic destructor while keeping the explicit release in Wh_ModUninit:

// com_ptr is nullable/handle-like, so the bare attribute is fine.
// Released with `g_pendingNavBrowser = nullptr;` in Wh_ModUninit.
[[clang::no_destroy]] static winrt::com_ptr<IShellBrowser> g_pendingNavBrowser;

// Container of thread-affine elements, so use the optional wrapper.
// Released with `g_Wrappers.reset();` in Wh_ModUninit (`.clear()` would keep the buffer).
[[clang::no_destroy]] static std::optional<std::vector<ExplorerWrapper>> g_Wrappers;

with g_Wrappers.emplace(); in Wh_ModInit and -> for the accesses. Note the existing g_Wrappers.clear() in Wh_ModUninit also releases the browsers from an arbitrary thread rather than from their owning Explorer threads — dropping the cached browsers entirely (see item 5) sidesteps both problems.

7. The context-menu action runs synchronously inside the mouse-down handler.

InvokeFolderContextMenuVerb is called from SysListViewSubclass / DUISubclass while handling WM_LBUTTONDOWN/WM_MBUTTONDOWN, before DefSubclassProc has passed the button-down on. CreateViewObject + QueryContextMenu(..., CMF_NORMAL) activates every registered folder-background context-menu handler (cloud storage, AV, archivers, VCS shells) — COM activation and disk I/O that can take seconds, freezing the Explorer window meanwhile. InvokeCommand can then put up modal UI, still inside the unhandled mouse-down.

Fix: get the work out of the mouse-down. Allocate a private message with RegisterWindowMessage and PostMessage it to the window, then do the menu work when that message is handled — the click completes normally and the verb runs afterwards. Setting ci.fMask = CMIC_MASK_ASYNCOK also lets handlers that support it return immediately.

8. Cascading submenus are dropped before the recursion.

In EnumContextMenuMatch (and identically in DumpContextMenuRecursive) the wID guards run before the hSubMenu check:

if (mii.wID == 0) continue;
if (mii.wID < (UINT)idCmdFirst || mii.wID > 0x7FFF) continue;
if (mii.hSubMenu != NULL) { ... recurse ... }

A menu item that opens a submenu carries no meaningful command ID — inserted via InsertMenu(..., MF_POPUP, (UINT_PTR)hSubMenu, ...) it ends up with wID either 0 or the HMENU value, and both are rejected by the guards. So those cascades are never entered and their entries can never be matched, which undercuts the feature's main selling point (the README even advertises matching New to Folder). The diagnostic dump hides them too, so the "check the log" advice won't reveal what's missing.

The guards are only needed for leaf items, so just reorder:

if (!GetMenuItemInfoW(hMenu, i, TRUE, &mii)) continue;

if (mii.hSubMenu != NULL) {
    if (pcm2)
        pcm2->HandleMenuMsg(WM_INITMENUPOPUP, (WPARAM)mii.hSubMenu, MAKELPARAM(i, 0));
    if (EnumContextMenuMatch(mii.hSubMenu, pcm, pcm2, hwnd, matchText, idCmdFirst))
        return true;
    continue;
}

if (mii.wID == 0) continue;                                    // separator
if (mii.wID < (UINT)idCmdFirst || mii.wID > 0x7FFF) continue;  // leaf outside the range

9. Hardcoded Chinese fallback, and three redundant brand-specific actions.

if (InvokeFolderContextMenuVerb(path, hShellTab, L"Terminal")) return;
if (InvokeFolderContextMenuVerb(path, hShellTab, L"终端")) return;

Windhawk mods default to English and express other languages through the localization syntax; one hardcoded non-English string means Chinese users are covered and German/Japanese/French/… users are not. Beyond that, openInVSCode / openInTerminal / openInCursor are just openWithContextMenu with a fixed match string — the generic action already covers all three plus everything else, and dropping them keeps three brand names out of the settings UI. I'd remove the three brand options (or at minimum the 终端 special case) and point the README at "Open Context Menu Item".

Optional improvements

Minor polish — none of this affects users, so it's your call.

• The hand-rolled StringSetting class duplicates WindhawkUtils::StringSetting, which you already have via <windhawk_utils.h>. g_doubleClickAction = WindhawkUtils::StringSetting::make(L"doubleClickAction"); replaces Load() and the manual Wh_FreeStringSetting.
• Wh_GetStringSetting never returns NULL — it returns L"" when unset or on error. So the if (m_str) guard in StringSetting and all fourteen x.Get() ? x.Get() : L"" ternaries in CopySettings() are dead code.
• WindhawkUtils::SetFunctionHook((void*)CreateWindowExW, (void*)CreateWindowExW_hook, (void**)&CreateWindowExW_original) can use the type-safe overload without the casts: WindhawkUtils::SetFunctionHook(CreateWindowExW, CreateWindowExW_hook, &CreateWindowExW_original).
• SendKeyCombo releases out of order for 3-key combos: Release(vk2); if (vk3) Release(vk3); Release(vk1); — for Ctrl+Shift+N that's Shift up before N up. Strict reverse order (vk3, vk2, vk1) mirrors real input. (Same point Copilot raised.)
• DUISubclass compares wParam directly against WM_LBUTTONDOWN/WM_MBUTTONDOWN; for WM_PARENTNOTIFY the message is in the low word, so LOWORD(wParam) is the correct read even if the high word happens to be zero for mouse events today. This matters more now that the new fast path early-returns on a mismatch.
• NormalizeForMatch lowercases with towlower, but StrStrIW is already case-insensitive — and towlower in the default C locale won't fold non-ASCII anyway, so it does nothing for the non-English menus the README highlights. Dropping it (or using CharLowerBuffW) would be equivalent and clearer.
• Missing includes: <cwctype> for iswspace/towlower, and <optional> if you take the no_destroy suggestion.
• GetCurrentFolderPath(wchar_t* outPath, size_t outLen) never uses outLen — SHGetPathFromIDListW assumes MAX_PATH unconditionally. Either drop the parameter or switch to SHGetPathFromIDListEx.
• CMINVOKECOMMANDINFOEX with CMIC_MASK_UNICODE is the recommended form for a Unicode caller, and it lets you set lpDirectoryW, which some verbs want.
• The leaf-reading block (verb + display text) is duplicated verbatim between EnumContextMenuMatch and DumpContextMenuRecursive — a small helper returning both strings would keep them from drifting.
• The README says "Supports 14 different actions" while @description says 17 and the list has 17 (+ None).
• The README has no screenshot or GIF. A short GIF of double-clicking empty space and going up a level would make the mod page much easier to understand.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• Modifier+click corrupts the synthesized combo. The modifier+click actions fire while the user is physically holding the modifier, and the action implementations inject keys with SendInput. Shift+Click to Paste sends Ctrl+V with Shift still down, i.e. Ctrl+Shift+V (paste-as-plain-text in many contexts); Alt+Click to New Tab sends Ctrl+Alt+T. Worth synthesizing KEYEVENTF_KEYUP for any currently-held VK_CONTROL/VK_MENU/VK_SHIFT at the start of the combo (and restoring after), or driving those actions through the shell APIs instead of key injection.
• Substring matching can hit the wrong entry. "Code" (used by the built-in Open in VS Code) also matches "Encode…", "CodeBlocks", etc., and the first match in menu order wins. An exact-match-first pass, or matching the verb before the display text, would be more predictable.
• Virtual folders are unsupported. GetCurrentFolderPath relies on SHGetPathFromIDListW, which fails for This PC, Home/Quick Access, Libraries, Recycle Bin and search results. In those folders Copy Path, Duplicate Tab and all four context-menu actions silently do nothing. The IPersistIDList PIDL is already in hand — binding the context menu straight from the PIDL, instead of round-tripping through a path with SHParseDisplayName, would make the context-menu actions work there too.
• Duplicate Tab is timing-based. SendKeyCombo(Ctrl+T) followed by a fixed 500 ms timer, with the new browser picked up opportunistically in the FileCabinet_CreateViewWindow2 hook, will miss on a slow machine or a slow-to-populate folder. Nothing urgent to change, just noting it's inherently flaky.
• Enabling triple-click adds ~500 ms to every double-click. That's documented in the README, and there isn't a better option given how Windows reports clicks — just flagging that most users will probably want to leave triple-click off, which the defaults already do.

[LiHua81]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

Thanks for working through the previous round — the over-release of CWM_GETISHELLBROWSER, the submenu ordering, g_Wrappers pruning, the no_destroy wrapper, the async dispatch and the KillTimer-before-return fixes all look right. Three teardown problems remain, and one of them is new (introduced by the fix for the old item 1).

1. The subclass tracking fix covers the new-window path but dropped the already-open-window path — Wh_ModUninit now leaves those subclasses installed.

CreateWindowExW_hook records into g_subclassed, but InitEnumChildWindowsProc — the Wh_ModAfterInit enumeration that handles Explorer windows already open when the mod is enabled — still only sets wrapper.hListView and never touches g_subclassed:

if (lv) {
    if (WindhawkUtils::SetWindowSubclassFromAnyThread(lv, SysListViewSubclass, 0)) {
        Wh_Log(L"SysListView32 Subclassed %p", lv);
        wrapper.hListView = lv;          // <-- not added to g_subclassed
    }
} else if (dui) {
    if (WindhawkUtils::SetWindowSubclassFromAnyThread(hWnd, DUISubclass, 0)) {
        Wh_Log(L"DirectUIHWND Subclassed %p", hWnd);
        wrapper.hListView = hWnd;        // <-- not added to g_subclassed
    }
}

Since Wh_ModUninit now iterates g_subclassed instead of g_Wrappers, the windows this function subclassed are never unsubclassed. Repro: have an Explorer window open, enable the mod, disable it, click in that window → the subclass proc is invoked in the unmapped mod image. That's the same crash as before, just on the opposite set of windows.

Fix — record them at the point of subclassing, same as the hook does:

if (lv) {
    if (WindhawkUtils::SetWindowSubclassFromAnyThread(lv, SysListViewSubclass, 0)) {
        wrapper.hListView = lv;
        std::lock_guard<std::mutex> lk(g_subclassMutex);
        g_subclassed.push_back({ lv, true });
    }
} else if (dui) {
    if (WindhawkUtils::SetWindowSubclassFromAnyThread(hWnd, DUISubclass, 0)) {
        wrapper.hListView = hWnd;
        std::lock_guard<std::mutex> lk(g_subclassMutex);
        g_subclassed.push_back({ hWnd, false });
    }
}

2. Making the pending-click state thread_local turned the Wh_ModUninit timer cleanup into a no-op — the TIMERPROCs still point into the mod image.

g_midClickPendingHwnd / g_midClickTimerId, g_pendingDblClickHwnd / g_pendingDblClickTimerId and g_pendingNavHwnd are now thread_local (correct for the race), but Wh_ModUninit runs on the Windhawk loader thread, not on an Explorer UI thread. So:

void Wh_ModUninit() {
    InterlockedExchange(&g_initialized, 0);
    CancelPendingMidClick();   // reads this thread's TLS -> 0/NULL, kills nothing
    CancelPendingDblClick();   // same
    if (g_pendingNavHwnd && IsWindow(g_pendingNavHwnd))   // always NULL here
        KillTimer(g_pendingNavHwnd, 0x4D43);

None of the three timers (0x4D43, 0x4D44, 0x4D45) armed on Explorer threads is ever cancelled. They are SetTimer(..., NavigateNewTabProc / MidClickTimerProc / DblClickTimerProc), i.e. callbacks that live in the mod DLL, so if the user middle-clicks or double-clicks within GetDoubleClickTime() (~500 ms) of disabling/updating the mod, USER32 dispatches into unmapped memory → Explorer crash. The KillTimer-first reordering you made only helps a callback that actually runs before the unload.

Fix — don't put a mod-image TIMERPROC on an Explorer window at all. Use SetTimer(hWnd, id, ms, nullptr) and handle WM_TIMER in the subclass proc (killing the timer there); after Wh_ModUninit removes the subclass, a stray WM_TIMER just falls through to Explorer's own window proc and is ignored. Then also kill the timers for every tracked HWND in Wh_ModUninit, which you can now do because g_subclassed already holds them:

for (auto& e : toRemove) {
    if (e.hWnd && IsWindow(e.hWnd)) {
        KillTimer(e.hWnd, 0x4D44);
        KillTimer(e.hWnd, 0x4D45);
        WindhawkUtils::RemoveWindowSubclassFromAnyThread(
            e.hWnd, e.isListView ? SysListViewSubclass : DUISubclass);
    }
}

file-explorer-details-autofit-columns.wh.cpp is a close model: SetTimer(hwndTimer, AUTOFIT_TIMER_ID, delay, nullptr), WM_TIMER handled in the subclass proc with KillTimer first, KillTimer on WM_NCDESTROY, and KillTimer for every tracked window in Wh_ModUninit.

Note the duplicate-tab timer 0x4D43 is set on the ShellTabWindowClass window, which is not subclassed — pass the clicked (subclassed) window down into ExplorerWrapper and set that timer on it instead, so the same handling covers it.

Same root cause, smaller consequence: thread_local winrt::com_ptr<IShellBrowser> g_pendingNavBrowser on an Explorer UI thread is also unreachable from Wh_ModUninit, so a duplicate-tab in flight at unload time leaks an IShellBrowser reference until Explorer restarts. Clearing it from the WM_TIMER handler (which runs on the owning thread) covers the common case.

3. g_Wrappers teardown: a disengaged-optional dereference, and the COM releases happen on the wrong thread.

Two separate problems in the same place.

(a) FindShellTabAndDoAction dereferences the optional unconditionally:

std::lock_guard<std::mutex> lock(g_wrappersMutex);
for (ExplorerWrapper& w : *g_Wrappers) {   // UB once Wh_ModUninit has done g_Wrappers.reset()

Wh_ModUninit resets it under the same mutex, but the subclass proc's g_initialized check happens before it takes the lock, and the timer callbacks (item 2) can reach here with no check at all. Cheap fix: if (g_Wrappers) for (...) inside the lock.

(b) g_Wrappers.reset() releases every cached IShellBrowser from the uninit thread. Entries for tabs that were closed since the last FileCabinet_CreateViewWindow2Hook call are still in the vector (pruning only happens when a new view is created), and for those the mod holds the last reference — so reset() runs CShellBrowser's destructor on a thread that has nothing to do with the Explorer window's apartment.

The clean way out is to not cache the browser at all: FindShellTabAndDoAction already runs on the window's own thread, so a same-thread SendMessage(shellTab, CWM_GETISHELLBROWSER, 0, 0) gets a fresh (borrowed, never released) pointer — the same thing you already do in InitEnumChildWindowsProc, and what explorer-status-metadata.wh.cpp#L591 and add-virtual-folders-to-nav-top.wh.cpp#L2569 do. That deletes g_Wrappers, g_wrappersMutex, the [[clang::no_destroy]] std::optional<> dance and both problems above, and it also removes the stale-HWND lookup issue. See https://github.com/ramensoftware/windhawk/wiki/Global-objects-and-process-shutdown (section 5, containers of resource-owning or thread-affine elements) for the background on why the container form needed the attribute in the first place.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• The WM_NCDESTROY block in both subclass procs sits below CHECK_INIT_OR_DEFER, so the comment "even during teardown" doesn't hold — once g_initialized is 0 the macro returns first and the g_subclassed entry is never erased. Move the WM_NCDESTROY handling above the macro (it's harmless there, and Wh_ModUninit swaps the container anyway).
• CreateWindowExW_hook ignores the return value of SetWindowSubclassFromAnyThread and pushes into g_subclassed unconditionally, so a failed subclass is still recorded and a repeated DirectUIHWND creation under the same SHELLDLL_DefView pushes a duplicate entry. Push only on success, and skip if the HWND is already tracked.
• DumpContextMenuRecursive runs on every failed match regardless of whether logging is enabled, and it re-walks the whole tree, calling HandleMenuMsg(WM_INITMENUPOPUP, ...) on every submenu a second time — i.e. re-activating every shell extension that populates one. Worth reusing the expansion done by the match pass, or collecting the items during that pass and only formatting them at the end.
• SendMessage(shellTab, WM_USER + 7, 0, 0) — <shdeprecated.h> is already included, so CWM_GETISHELLBROWSER reads better than the raw WM_USER + 7.
• The mouse handlers use GetCursorPos + ScreenToClient instead of the coordinates the message already carries (GET_X_LPARAM(lParam) / GET_Y_LPARAM(lParam), <windowsx.h> is included). The message coordinates are the ones the click actually happened at; the cursor may have moved by the time the handler runs.
• openInVSCode / openInTerminal / openInCursor are openWithContextMenu with a hardcoded match string, so they add three brand names to seven dropdowns for nothing the generic action can't do. (Raised last round; still your call, but it would shrink the settings UI noticeably.)
• Still open from the previous round, all one-liners: the hand-rolled StringSetting duplicates WindhawkUtils::StringSetting; Wh_GetStringSetting never returns NULL, so the if (m_str) guard and the fourteen x.Get() ? x.Get() : L"" ternaries in CopySettings() are dead; WindhawkUtils::SetFunctionHook(CreateWindowExW, CreateWindowExW_hook, &CreateWindowExW_original) works without the void* casts; SendKeyCombo still releases vk2 before vk3 for 3-key combos (Copilot flagged this too); DUISubclass should read LOWORD(wParam) for WM_PARENTNOTIFY; towlower in NormalizeForMatch is redundant with StrStrIW and doesn't fold non-ASCII anyway; <cwctype> is missing for iswspace/towlower; GetCurrentFolderPath's outLen parameter is unused; CMINVOKECOMMANDINFOEX + CMIC_MASK_UNICODE is the recommended form; the leaf-reading block is still duplicated verbatim between EnumContextMenuMatch and DumpContextMenuRecursive.
• The README still says "Supports 14 different actions" while @description and the list say 17.
• The README still has no screenshot or GIF. A few seconds of double-clicking empty space and going up a level would make the mod page much easier to grasp.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• Modifier+click still corrupts the synthesized combo. Shift+Click to Paste injects Ctrl+V while Shift is physically held, i.e. Ctrl+Shift+V; Alt+Click to New Tab injects Ctrl+Alt+T. Moving dispatch to PostMessage helps only if the user has released the modifier by then, which is a race. Synthesizing KEYEVENTF_KEYUP for any held VK_CONTROL/VK_MENU/VK_SHIFT at the head of the combo (and restoring after) would make it deterministic.
• Substring matching can hit the wrong entry. "Code" (used by the built-in Open in VS Code) also matches "Encode…", "CodeBlocks", etc., and the first match in menu order wins. An exact-match-first pass, or matching the verb before the display text, would be more predictable.
• Virtual folders are unsupported. GetCurrentFolderPath relies on SHGetPathFromIDListW, which fails for This PC, Home/Quick Access, Libraries, Recycle Bin and search results, so Copy Path, Duplicate Tab and all four context-menu actions silently do nothing there. The IPersistIDList PIDL is already in hand — binding the context menu straight from it, instead of round-tripping through a path with SHParseDisplayName, would make the context-menu actions work in those folders too.
• Duplicate Tab is timing-based. Ctrl+T plus a fixed 500 ms timer, with the new browser picked up opportunistically in the FileCabinet_CreateViewWindow2 hook, will miss on a slow machine or a slow-to-populate folder. Inherently flaky; nothing obviously better available.
• Enabling triple-click adds ~500 ms to every double-click. Documented in the README and unavoidable given how Windows reports clicks — just noting most users will want to leave it off, which the defaults already do.

[LiHua81]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

Three issues worth fixing before merge; the rest is optional.

1. The subclass procs swallow every WM_TIMER, breaking Explorer's own timers.

In both SysListViewSubclass and DUISubclass the timer branch returns 0 unconditionally, so no WM_TIMER ever reaches DefSubclassProc:

if (uMsg == WM_TIMER) {
    switch (wParam) {
    case 0x4D43: NavigateNewTabProc(hWnd, uMsg, wParam, dwRefData); break;
    case 0x4D44: MidClickTimerProc(hWnd, uMsg, wParam, dwRefData); break;
    case 0x4D45: DblClickTimerProc(hWnd, uMsg, wParam, dwRefData); break;
    }
    return 0;   // <-- also eats the window's own timers
}

SysListView32 and SHELLDLL_DefView both use their own window timers — e.g. the delayed label edit (click-pause-click to rename), hot-tracking / hover selection, the incremental-search timeout, and marquee/drag auto-scroll. Blocking all of them is a user-visible regression for anyone who enables the mod. This is new in this PR: v2.3.0 used TIMERPROC callbacks, so WM_TIMER was never intercepted.

Only consume the IDs you own:

if (uMsg == WM_TIMER) {
    switch (wParam) {
    case 0x4D43: NavigateNewTabProc(hWnd, uMsg, wParam, dwRefData); return 0;
    case 0x4D44: MidClickTimerProc(hWnd, uMsg, wParam, dwRefData); return 0;
    case 0x4D45: DblClickTimerProc(hWnd, uMsg, wParam, dwRefData); return 0;
    }
    return DefSubclassProc(hWnd, uMsg, wParam, lParam);
}

Related: since you're setting timers on windows the mod doesn't own, hard-coded IDs (0x4D43–0x4D45) can also collide with an ID the control itself uses. They're unusual enough that a collision is unlikely, but it's worth being aware of — if you want to be safe, derive the IDs from something process-unique instead of fixed constants.

2. Injected key combos fire while the user is still physically holding the modifier.

ctrlClickAction / altClickAction / shiftClickAction are evaluated on WM_LBUTTONDOWN (lines ~1188–1201 and ~1433–1446), and any action implemented via SendInput (newTab, duplicateTab, closeTab, newFolder, paste, customHotkey) is then injected while the physical modifier key is still down. The target app sees the union of both.

Concrete case: Shift+Click → New Tab sends Ctrl+T while Shift is held, so File Explorer receives Ctrl+Shift+T — "reopen closed tab", not "new tab". Same for Alt+Click → New Folder: Ctrl+Shift+N becomes Ctrl+Shift+Alt+N. And with a custom hotkey the user configures (e.g. Shift+Click → Ctrl+V) the injected combo is silently polluted too.

Before injecting, release whichever of VK_CONTROL / VK_MENU / VK_SHIFT / VK_LWIN are physically down but not part of the combo, and re-press them afterwards if they're still down:

static void ReleaseHeldModifiers(std::vector<INPUT>& pre, std::vector<INPUT>& post,
                                 const std::vector<WORD>& combo) {
    for (WORD vk : {VK_CONTROL, VK_MENU, VK_SHIFT, VK_LWIN}) {
        if (!(GetKeyState(vk) & 0x8000)) continue;
        if (std::find(combo.begin(), combo.end(), vk) != combo.end()) continue;
        pre.push_back({INPUT_KEYBOARD, {.ki = {.wVk = vk, .dwFlags = KEYEVENTF_KEYUP}}});
        post.push_back({INPUT_KEYBOARD, {.ki = {.wVk = vk}}});
    }
}

Alternatively, defer the injected-key actions until the modifier is released (e.g. act on WM_LBUTTONUP plus a modifier-released check) — but the release/restore approach keeps the current instant behavior.

3. Only the first tab of an already-open Explorer window is hooked at enable time.

InitEnumWindowsProc (line 1564) grabs a single ShellTabWindowClass:

HWND shellTab = FindWindowEx(hWnd, NULL, L"ShellTabWindowClass", NULL);
if (shellTab != NULL)
    EnumChildWindows(shellTab, InitEnumChildWindowsProc, (LPARAM)shellTab);

On Windows 11 each tab is its own ShellTabWindowClass child of CabinetWClass, so when the mod is enabled with a multi-tab window open, only one tab gets subclassed. The others stay dead until the user navigates in them (which recreates the view and trips the CreateWindowExW hook) — from the user's point of view the mod just doesn't work in those tabs. Iterate all of them:

for (HWND shellTab = FindWindowEx(hWnd, NULL, L"ShellTabWindowClass", NULL);
     shellTab;
     shellTab = FindWindowEx(hWnd, shellTab, L"ShellTabWindowClass", NULL)) {
    EnumChildWindows(shellTab, InitEnumChildWindowsProc, (LPARAM)shellTab);
}

Optional improvements

Minor polish — none of this affects users in normal operation, so it's your call.

• Wh_ModUninit's pending-nav cleanup is now a no-op. g_pendingNavPath / g_pendingNavBrowser / g_pendingNavHwnd became thread_local in this PR (lines 747–749), but Wh_ModUninit runs on an arbitrary Windhawk thread, so lines 1617–1619 clear that thread's (always-empty) copies. The Explorer UI thread's com_ptr<IShellBrowser> keeps its reference, which is only released if/when that thread exits — and if the mod DLL is already unloaded by then, it may never be. It's a single interface reference during a ~500 ms window, so the practical impact is small, but the comment claims a cleanup that doesn't happen. Either move the release to the owning thread (you already have a posted-message channel — PostDoAction-style — and the tracked HWND list), or drop the misleading lines. See https://github.com/ramensoftware/windhawk/wiki/Global-objects-and-process-shutdown for the general pattern (thread-affine objects must be released on their owning thread).

• GetUIAutomation()'s pointer is shared across apartments. The function-local static IUIAutomation* (line 1053) is process-wide, but each Explorer window runs on its own thread with its own STA. So the instance created in window A's apartment is called directly from window B's thread, which isn't valid COM. Making it thread_local (still intentionally leaked) keeps the current "no Release() at shutdown" property while staying inside one apartment. Also note the current form leaks one IUIAutomation per enable/disable cycle, not just per process.

• g_subclassed can accumulate duplicate entries. CreateWindowExW_hook pushes defView every time a DirectUIHWND is created under it (line 1499), and InitEnumChildWindowsProc may have pushed the same HWND already. SetWindowSubclassFromAnyThread is idempotent, so the extra entries are harmless, but a quick std::find_if guard before push_back keeps the vector honest.

• Use the message's own coordinates instead of GetCursorPos. In the hit tests (lines ~1169, ~1207, ~1233) lParam already carries the client-relative point; GET_X_LPARAM(lParam) / GET_Y_LPARAM(lParam) (you already include <windowsx.h>) is both cheaper and correct if the message is processed after the cursor moved. In DUISubclass the WM_PARENTNOTIFY lParam is also the point, in the parent's client coordinates.

• Drop the hand-rolled StringSetting (line 348) in favour of WindhawkUtils::StringSetting. It's the same RAII shape and supports reassignment: g_doubleClickAction = WindhawkUtils::StringSetting::make(L"doubleClickAction");. Also, Wh_GetStringSetting never returns NULL (it returns L"" on error / unset), so all the ... ? ... : L"" ternaries in CopySettings and the if (s) in OpenWithContextMenu can go.

• Use the type-safe hook helper. Line 1599 casts everything to void*; WindhawkUtils::SetFunctionHook(CreateWindowExW, CreateWindowExW_hook, &CreateWindowExW_original); compiles the signature check for you.

• towlower in NormalizeForMatch is redundant — StrStrIW already does a locale-aware case-insensitive compare, and towlower in the default "C" locale only folds ASCII anyway, so it doesn't buy anything for accented Latin text. Keeping just the whitespace/& stripping would be equivalent and clearer.

• PostDoAction's heap block leaks if the message is never dispatched (window destroyed, or mod unloaded — CHECK_INIT_OR_DEFER runs before the g_msgDoAction branch, so during teardown the string is forwarded to DefSubclassProc and never freed). Tiny and unload-only, but easy to avoid by passing a small fixed action enum in wParam instead of a heap-allocated string — the action set is a closed list anyway.

• SendKeyCombo releases out of order (already noted by the Copilot bot): Release(vk2); if (vk3) Release(vk3); Release(vk1); releases the middle modifier before the main key. Harmless for the combos in use, but strict reverse order (vk3, vk2, vk1) mirrors real input.

• g_pendingNavHwnd is assigned and cleared but never read — dead state, can be removed.

• WM_USER + 7 would read better as a named constant. Other mods define it explicitly, e.g. mods/explorer-status-metadata.wh.cpp#L104 (#define CWM_GETISHELLBROWSER (WM_USER + 7)). The class-name check before sending it is correct, so this is purely readability.

• volatile LONG g_initialized + InterlockedExchange can just be std::atomic<bool>.

• README says "Supports 14 different actions" while @description says 17 and the dropdown lists 18 entries.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• IShellView::GetItemObject(SVGIO_BACKGROUND, ...) would be a better source for the background menu. InvokeFolderContextMenuVerb currently goes path → SHParseDisplayName → BindToObject → CreateViewObject, which means (a) it silently does nothing for any non-filesystem location, because GetCurrentFolderPath depends on SHGetPathFromIDListW (Libraries, This PC, network locations, search results), and (b) the menu it builds isn't quite the one Explorer shows — the defview-contributed items (View, Sort by, Refresh, Paste, Undo) are missing. You already have the IShellView in GetCurrentFolderPath; asking it for the background context menu directly gives you the real menu and removes the path round-trip entirely.

• The openInVSCode / openInTerminal / openInCursor actions are just openWithContextMenu with hard-coded English needles (L"Code", L"Terminal", L"Cursor"). Two consequences worth documenting or reconsidering: on a non-English Windows they won't match anything (the README's "Non-English menus work" claim only holds for the generic action, where the user types their own text), and L"Code" is a substring that will also match entries like "Encode"/"Decode" if a shell extension registers one. Since the generic action plus the contextMenuMatch setting already covers all three, dropping them would shrink the dropdown by three entries and remove the mismatch risk — or at least mention the English-only limitation in their README lines.

• Building the menu blocks the Explorer UI thread. QueryContextMenu loads every registered background shell extension, and EnumContextMenuMatch then calls HandleMenuMsg(WM_INITMENUPOPUP, ...) on every cascading submenu to populate it — so a click can stall the UI for a noticeable moment on a machine with many shell extensions. Posting the work off the mouse handler (which this PR does) helps responsiveness of the click itself but not the stall. There's no clean alternative here since shell verb invocation wants a UI thread, so this is just an FYI; one cheap mitigation would be to only descend into submenus if no top-level item matched.

• GetCommandString(..., GCS_VERBA, ...) + MultiByteToWideChar(CP_ACP, ...) works, but GCS_VERBW straight into the wchar_t buffer avoids the ANSI round-trip (and its loss for verbs with non-ANSI characters). The CMINVOKECOMMANDINFO/MAKEINTRESOURCEA side has to stay ANSI, but the verb query doesn't.

• Triple-click and delayed double-click use SetTimer with GetDoubleClickTime() but no position check. Two clicks 400 ms apart at opposite ends of the view will count as a double-click. Explorer's own detection also requires the second click to land within SM_CXDOUBLECLK/SM_CYDOUBLECLK of the first; adding that check to the DUISubclass path (the SysListView32 path gets WM_LBUTTONDBLCLK from the system, so it's already correct) would match native behavior.

[LiHua81]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

Nice cleanup overall — switching the CWM_GETISHELLBROWSER result from take_ownership_from_abi to copy_from is the right call (that message returns a borrowed pointer, so the old code was over-releasing), and moving the action dispatch out of the mouse-down handler via RegisterWindowMessage + PostMessage is exactly the right shape. A few things below.

1. Triple Click Action never fires unless Double Click Action is also set. The pending-double-click timer (0x4D45) — which is the only thing the triple-click branch keys off — is armed inside the double-click path, and that path bails out early when the double-click action is none:

• SysListViewSubclass, line 1220: if (wcscmp(s.doubleClick.c_str(), L"none") == 0) return DefSubclassProc(...) returns before the tripleOn block that calls SetTimer(hWnd, 0x4D45, ...).
• DUISubclass, line 1431: the SetTimer(hWnd, 0x4D45, ...) sits inside if (dblOn) { if (tripleOn) { ... } }.

Since the third click is only recognized by if (tripleOn && g_pendingDblClickHwnd == hWnd && g_pendingDblClickTimerId != 0), a user who sets Triple Click = Go Up and leaves Double Click = None (a perfectly natural config — the point of triple-click is often to not have double-click do anything) gets no behavior at all, with nothing in the log to explain why. Arm the timer whenever tripleOn, independent of dblOn, and let DblClickTimerProc no-op when the stored action is empty — it already guards on if (!g_pendingDblClickAction.empty()), so storing L"" when the double-click action is none is enough:

} else if (uMsg == WM_LBUTTONDBLCLK) {
    bool dblOn    = (wcscmp(s.doubleClick.c_str(), L"none") != 0);
    bool tripleOn = (wcscmp(s.tripleClick.c_str(), L"none") != 0);
    if (!dblOn && !tripleOn)
        return DefSubclassProc(hWnd, uMsg, wParam, lParam);
    /* ...hit test... */
    if (tripleOn) {
        CancelPendingDblClick();
        g_pendingDblClickHwnd    = hWnd;
        g_pendingDblClickAction  = dblOn ? s.doubleClick : L"";
        g_pendingDblClickCombo   = s.doubleClickCombo;
        g_pendingDblClickTimerId = SetTimer(hWnd, 0x4D45, GetDoubleClickTime(), nullptr);
    } else { /* instant double-click */ }
}

2. The new "release held modifiers, then restore them" logic in SendKeyCombo (lines 448-483) can leave a modifier key stuck down. The motivation is right, but there are three concrete failure modes:

• Right-hand modifiers aren't actually released. GetKeyState(VK_SHIFT) reports the combined left+right state, but SendInput with ki.wVk = VK_SHIFT and wScan = 0 maps to the left Shift scan code. Hold Right Shift and Shift+Click: the injected keyup releases Left Shift (already up, no-op), the combo still runs with Shift down, and then the restore injects a Left Shift down that nothing ever releases → Shift stuck on until the user taps Left Shift. Same for Ctrl and Alt.
• The restore races the user. It's a second, separate SendInput after the combo, and the whole action now runs from a posted message (g_msgDoAction), so there's real latency. If the user lifts the modifier in that window, the injected key-down has no matching key-up → stuck modifier again.
• VK_LWIN shouldn't be in the list at all. Win isn't one of the mod's triggers (only Ctrl/Alt/Shift are), and re-pressing VK_LWIN followed by the user's physical Win-up is seen by the shell as a bare Win tap, which pops the Start menu.

Use the side-specific VKs so both sides are handled, and put release + combo + restore in a single SendInput batch so nothing can interleave (this also fixes the non-reverse release order Copilot flagged for 3-key combos):

static void SendKeyCombo(WORD vk1, WORD vk2, WORD vk3 = 0) {
    static constexpr WORD kSideMods[] = {VK_LCONTROL, VK_RCONTROL, VK_LMENU,
                                         VK_RMENU, VK_LSHIFT, VK_RSHIFT};
    std::vector<INPUT> in;
    auto Key = [&](WORD vk, DWORD flags) {
        in.push_back(INPUT{INPUT_KEYBOARD, {.ki = {.wVk = vk, .dwFlags = flags}}});
    };

    std::vector<WORD> held;
    for (WORD vk : kSideMods)
        if (GetKeyState(vk) & 0x8000) held.push_back(vk);

    for (WORD vk : held) Key(vk, KEYEVENTF_KEYUP);
    Key(vk1, 0); Key(vk2, 0); if (vk3) Key(vk3, 0);
    if (vk3) Key(vk3, KEYEVENTF_KEYUP);
    Key(vk2, KEYEVENTF_KEYUP); Key(vk1, KEYEVENTF_KEYUP);
    for (WORD vk : held) Key(vk, 0);

    SendInput((UINT)in.size(), in.data(), sizeof(INPUT));  // one atomic batch
}

3. All four context-menu actions silently do nothing in virtual folders. InvokeFolderContextMenuVerb (line 714) takes a path, and GetCurrentFolderPath gets it from SHGetPathFromIDListW, which returns FALSE for any non-filesystem PIDL — This PC, Home/Quick Access, Libraries, Network, Recycle Bin, search results. In those views OpenInVSCode / OpenInTerminal / OpenInCursor / OpenWithContextMenu return at if (!GetCurrentFolderPath(...)) return; without even a log line, so the user just sees nothing happen.

The path round-trip is also unnecessary work: the mod already holds the IShellView, and IShellView::GetItemObject(SVGIO_BACKGROUND, IID_IContextMenu, ...) returns exactly the "right-click empty space" menu for the current view. That drops SHParseDisplayName + SHGetDesktopFolder + BindToObject + CreateViewObject entirely, and works for virtual folders:

IShellView* psv = nullptr;
if (FAILED(hBrowser->QueryActiveShellView(&psv)) || !psv) return false;
IContextMenu* pcm = nullptr;
HRESULT hr = psv->GetItemObject(SVGIO_BACKGROUND, IID_IContextMenu, (void**)&pcm);
psv->Release();
if (FAILED(hr) || !pcm) return false;
// ...QueryContextMenu / EnumContextMenuMatch as today...

At minimum, log when the folder path can't be resolved so the failure isn't silent.

4. The three hardcoded openInVSCode / openInTerminal / openInCursor actions duplicate openWithContextMenu and match too loosely. They're literally the generic action with a fixed needle (L"Code", L"Terminal", L"Cursor" — lines 993-1013). Because StrContainsNorm is a normalized substring test over both the display text and the verb, L"Code" matches any entry containing "code" (a "Decode…"/"QR Code" shell extension, "Open with VS Code Insiders" instead of stable, or an unrelated localized string), and L"Terminal" picks whichever of "Open in Terminal" / "Windows Terminal Preview" / "Open in Terminal as administrator" happens to come first in menu order — and the user has no way to correct either.

Since openWithContextMenu + contextMenuMatch covers all three cases and lets the user disambiguate, I'd drop them and put the suggested match strings in the README's Context Menu Match table instead. That also removes three product-specific entries from each of the seven action dropdowns.

Optional improvements

Minor polish — none of this affects users, so it's your call.

• DumpContextMenuRecursive re-walks the whole menu on every failed match. It re-expands every submenu via HandleMenuMsg(WM_INITMENUPOPUP, ...) (which runs third-party shell extensions' menu-building code) and calls GetCommandString on every leaf a second time — EnumContextMenuMatch already did all of that in the pass that just failed. Wh_Log itself is cheap when logging is off, but this walk isn't. Collect text/verb into a std::vector<std::wstring> during the single EnumContextMenuMatch pass and log it at the end only if nothing matched.

• NormalizeForMatch case-folds with towlower, which under the default "C" CRT locale only folds ASCII. The README promises case-insensitive matching, but that won't hold for German/Russian/Greek/Turkish menu text. Build the string first, then fold it with CharLowerBuffW(out.data(), (DWORD)out.size()).

• g_subclassed accumulates duplicate entries. CreateWindowExW_hook (line 1511) pushes the parent SHELLDLL_DefView every time a DirectUIHWND is created under it, without checking whether that HWND is already tracked, and without checking SetWindowSubclassFromAnyThread's return value (unlike InitEnumChildWindowsProc, which does check). Each duplicate costs an extra cross-thread SendMessage in Wh_ModUninit. A std::unordered_map<HWND, bool>, or a contains-check before push_back, removes both problems.

• The local StringSetting class can be replaced by WindhawkUtils::StringSetting, which supports re-assignment: g_doubleClickAction = WindhawkUtils::StringSetting::make(L"doubleClickAction");. Relatedly, Wh_GetStringSetting never returns NULL (it returns L"" when unset or on error), so the Get() ? Get() : L"" ternaries in CopySettings() and the if (s) in OpenWithContextMenu are dead branches.

• Includes: <algorithm> was added, but std::erase_if for std::vector comes from <vector>; meanwhile iswspace/towlower are used without <cwctype> and rely on a transitive include.

• PostDoAction's heap block leaks if the window is destroyed (or the mod unloaded) before the posted message is dispatched. The mod compares action strings with wcscmp in ~20 places anyway — converting the action to a small enum class Action would remove the allocation entirely and make wParam self-contained.

• Mouse coordinates: the handlers call GetCursorPos + ScreenToClient when lParam already carries the click's client coordinates (GET_X_LPARAM(lParam) / GET_Y_LPARAM(lParam)). By the time a message is handled the cursor may have moved, so the hit test can disagree with the actual click point.

• Two objects survive mod unload. The IUIAutomation in GetUIAutomation() (line 1069) is deliberately leaked to avoid a Release at DLL_PROCESS_DETACH — right reasoning for process exit, but it also means a fresh instance is created and abandoned on every enable/disable/update cycle, accumulating for the life of Explorer. Similarly, thread_local winrt::com_ptr<IShellBrowser> g_pendingNavBrowser can only be cleared by Wh_ModUninit on its own thread, so a duplicate-tab in flight on another Explorer thread leaves an AddRef'd browser behind (narrow ~500 ms window, low impact). The recommended shape for both is an explicit release on the owning thread in Wh_ModUninit rather than a permanent leak — see https://github.com/ramensoftware/windhawk/wiki/Global-objects-and-process-shutdown.

• README count is stale: it still says "Supports 14 different actions" while @description says 17 and there are 17 non-none options.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• contextMenuMatch is a single setting shared by all seven triggers. Every trigger has its own ...CustomHotkey field, so the asymmetry is noticeable: a user can't set Ctrl+Click = Git Bash and Alt+Click = PowerShell 7. A per-trigger ...ContextMenuMatch field would match the existing pattern.

• Modifier+click still performs the ListView's default behavior. The Ctrl/Alt/Shift branches fall through to DefSubclassProc, so Ctrl+Click on empty space begins a marquee selection at the same time the action (e.g. Go Up) navigates away. Returning 0 for a handled modifier click would suppress that, at the cost of not giving the view focus — worth deciding deliberately either way.

• InvokeCommand runs the verb's handler synchronously on the Explorer UI thread. Some handlers show UI or do slow work, which will briefly freeze that Explorer window. Deferring the dispatch via g_msgDoAction already got this out of the mouse-down handler, which is the important part; there's no clean way to go further without a worker thread and its own apartment, so this is just an FYI.

• Expanding every submenu during matching (HandleMenuMsg(WM_INITMENUPOPUP, ...)) populates "New", "Send to", etc., which enumerates ShellNew handlers and runs third-party extension code. That's the same work a real right-click does, so it's acceptable — but it does mean an action whose match string appears late in the menu (or not at all) is noticeably slower than one that matches near the top.

[LiHua81]

/ai-review

[LiHua81]

/ai-reveiw

[LiHua81]

/ai-reveiw

[LiHua81]

/ai-review

[windhawk-reviewer]

Submission review

Note: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

---

The new triple-click / modifier+click / context-menu features are a nice addition, but there are a few real problems in the dispatch code.

1. SendParsedHotkey doesn't release the physically-held modifier, so modifier+click + Custom Hotkey sends the wrong combo

SendKeyCombo was just taught to release held side-modifiers before injecting (lines 485-507), but SendParsedHotkey (lines 554-591) wasn't. Custom Hotkey is dispatched synchronously from the subclass proc while the user is still holding the modifier:

if (ctrlOn && ctrlDown) {
    if (!TryCustomHotkey(s.ctrlClick.c_str(), s.ctrlClickCombo))   // fires with Ctrl down

So Alt+Click with custom hotkey F4 actually injects Alt+F4 and closes the Explorer window; Shift+Click with Ctrl+N becomes Ctrl+Shift+N; Ctrl+Click with V becomes Ctrl+V. Apply the same release/restore batch to SendParsedHotkey — ideally factor the "release held kSideMods, inject, restore" wrapper out of SendKeyCombo and have both go through it.

2. Missing braces / duplicated lines in the dispatch blocks

Several if (!TryCustomHotkey(...)) blocks are missing braces, so PostDoAction runs unconditionally, and two of them have a duplicated SetCtxMenuMatch line. Lines 1242-1249:

        if (tripleOn && g_pendingDblClickHwnd == hWnd && g_pendingDblClickTimerId != 0) {
            CancelPendingDblClick();
            if (!TryCustomHotkey(s.tripleClick.c_str(), s.tripleClickCombo))
            SetCtxMenuMatch(s.tripleClick.c_str(), g_tripleClickCtxMatch);
                SetCtxMenuMatch(s.tripleClick.c_str(), g_tripleClickCtxMatch);
                PostDoAction(hWnd, s.tripleClick.c_str());
            return DefSubclassProc(hWnd, uMsg, wParam, lParam);
        }

Same shape at lines 1256-1268, 1296-1298, 1316-1320, 1327-1329, 1412-1416, 1468-1475, 1502-1504, 1519-1522. Today the consequence is "only" that a customHotkey action both injects the keys and posts a no-op PostDoAction(hWnd, L"customHotkey") (which walks the parent chain, does a SendMessage(WM_USER+7) and then falls through DoAction doing nothing) — but it is clearly not what the code says it does, and it directly causes item 3 below. The intended shape is:

if (!TryCustomHotkey(s.tripleClick.c_str(), s.tripleClickCombo)) {
    SetCtxMenuMatch(s.tripleClick.c_str(), g_tripleClickCtxMatch);
    PostDoAction(hWnd, s.tripleClick.c_str());
}

3. Several per-trigger *ContextMenuMatch settings silently have no effect

SetCtxMenuMatch is a side channel that has to be called at every dispatch site, and three sites don't call it:

• DUISubclass lines 1513-1518 — the Ctrl+Click and Alt+Click branches never call SetCtxMenuMatch (only the Shift branch does). Since DirectUIHWND/UIItemsView is the view Windows 10/11 Explorer actually uses, ctrlClickContextMenuMatch and altClickContextMenuMatch are dead for most users — they silently fall back to the global contextMenuMatch.
• MidClickTimerProc (lines 867-878) — so middleClickContextMenuMatch is ignored whenever Double Middle Click is also configured (the delayed path).
• DblClickTimerProc (lines 888-901) — so doubleClickContextMenuMatch is ignored whenever Triple Click is enabled.

The robust fix is to stop using a thread-local side channel and carry the match text with the action: resolve it once at dispatch time and pack both strings into the block PostDoAction already allocates (and store it alongside g_pendingDblClickAction / the pending middle-click state for the timer paths). That deletes g_pendingCtxMenuMatch, SetCtxMenuMatch and all the "did I remember to call it here?" cases at once.

4. One IUIAutomation instance is shared across all Explorer window threads

GetUIAutomation() (lines 1115-1123) caches a single IUIAutomation* in a function-local static, created by whichever thread calls it first. Every Explorer browser window (CabinetWClass) runs on its own thread, so DUISubclass on every other Explorer window then calls ElementFromPoint/get_CurrentClassName on an object that belongs to a different apartment, with no marshalling. That's a COM apartment violation and a plausible source of hangs/crashes in Explorer with several windows open.

Make it per-thread. A raw thread_local IUIAutomation* is trivially destructible, so it's safe at process shutdown as-is — explorer-nav-dragover-fix.wh.cpp#L589 does exactly this and documents the same reasoning:

thread_local IUIAutomation* g_threadUiAutomation = nullptr;

(Pre-existing in 2.3.0, but the mod is much more multi-window-active now.)

5. openInVSCode / openInTerminal match hardcoded English text

OpenInVSCode / OpenInTerminal (lines 1052-1060) call the matcher with the literals L"Code" and L"Terminal". On a non-English Windows the menu text is localized and the verbs are GUIDs (as your own README table shows for both entries), so these two actions just silently do nothing. They're also exactly openWithContextMenu with a preset string. Simplest fix: drop both options and let users configure openWithContextMenu — the README already documents how to find the right text. If you want to keep them as convenience presets, at least state the English-only limitation in the README.

6. A failed match walks and re-populates the whole context menu twice

EnumContextMenuMatch already calls pcm2->HandleMenuMsg(WM_INITMENUPOPUP, ...) on every cascading submenu it descends into. When nothing matches, DumpContextMenuRecursive (lines 682-735, called at lines 759-760 and 807-809) walks the same menu again and calls HandleMenuMsg(WM_INITMENUPOPUP) on every submenu a second time — re-running every third-party shell extension's popup-init, on the Explorer UI thread, on every click. This runs regardless of whether logging is enabled (there's no API to check), so a mistyped match string means every click pays for it.

Collect the per-item dump strings during the single EnumContextMenuMatch pass (into a small std::vector<std::wstring>) and only Wh_Log them if the pass returned no match, instead of doing a second walk.

Optional improvements

Minor polish — none of this affects users in normal operation, so it's your call.

• SetCtxMenuMatch reads settings without the lock. Lines 470-474 call perTriggerMatch.Get() while StringSetting::Load (line 364) can concurrently Wh_FreeStringSetting the old pointer from Wh_ModSettingsChanged on another thread — a use-after-free on a settings change. Everything else (CopySettings, OpenWithContextMenu) correctly takes g_settingsMutex. If you adopt the fix in item 3, resolve the match text inside CopySettings and the problem disappears.
• Posted action strings can leak on unload. PostDoAction HeapAllocs the action string; if the message is dispatched after g_initialized is cleared, CHECK_INIT_OR_DEFER returns before the HeapFree (and after the subclass is removed the message reaches Explorer's own wndproc, which won't free it either). Only leaks on unload, so low impact.
• The custom StringSetting class (lines 361-373) duplicates WindhawkUtils::StringSetting, which is move-assignable — g_doubleClickAction = WindhawkUtils::StringSetting(L"doubleClickAction"); in LoadSettings does the same job.
• Wh_GetStringSetting never returns NULL (it returns L"" when unset or on error), so the Get() ? Get() : L"" ternaries in CopySettings, the if (s) in OpenWithContextMenu, and the null check in SetCtxMenuMatch are all dead code.
• Use the type-safe hook form: WindhawkUtils::SetFunctionHook((void*)CreateWindowExW, (void*)CreateWindowExW_hook, (void**)&CreateWindowExW_original) (line 1678) throws away the template's type checking. See classic-taskbar-properties.wh.cpp#L3329: WindhawkUtils::SetFunctionHook(CreateWindowExW, CreateWindowExW_hook, &CreateWindowExW_original);
• Timer IDs 0x4D43/0x4D44/0x4D45 are set on Explorer's own windows. SetTimer with an existing ID replaces that timer, and the subclass swallows those WM_TIMERs, so a collision with a comctl32/Explorer timer on SysListView32 or SHELLDLL_DefView would silently break Explorer behavior. Unlikely at those values, but a dedicated message-only window owned by the mod would be collision-free.
• Prefer GCS_VERBW with a wchar_t buffer over GCS_VERBA + MultiByteToWideChar(CP_ACP, ...) (lines 654-658, 715-719) — it avoids the ANSI round-trip, and GCS_VERBW is what modern handlers implement.
• NormalizeForMatch uses towlower, which in the default C locale only lowercases ASCII. The README promises case-insensitive matching for non-English menus; for scripts that have case (Cyrillic, Greek, accented Latin) it won't hold. CharLowerBuffW on the assembled string would.
• Missing include: <cwctype> for iswspace/towlower (currently relying on a transitive include).
• Doc mismatch: @description says "17 actions", the README says "Supports 16 different actions" (the list has 17 including None).
• Duplicate entries in g_subclassed: CreateWindowExW_hook pushes defView once per DirectUIHWND child created under it, and Wh_ModAfterInit's enumeration can push the same window again. Harmless (the second RemoveWindowSubclassFromAnyThread no-ops), but a std::unordered_map<HWND, bool> would be cleaner.
• GetClassName return value unchecked in InitEnumWindowsProc / InitEnumChildWindowsProc (lines 1611, 1639) — on failure className is uninitialized and wcscmp reads garbage. The rest of the file checks it.
• g_pendingNavHwnd is assigned and cleared but never read — dead state.
• extern thread_local std::wstring g_pendingCtxMenuMatch; at line 466 gives the variable external linkage in a single-TU mod. Just move the definition above its first use and mark it static. (Moot if you adopt item 3.)
• README has no screenshot/GIF. A short GIF of, say, double-click-to-go-up and Ctrl+Click would make the mod page much easier to evaluate.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

• InvokeCommand blocks the Explorer UI thread. Moving the dispatch to a posted message got it out of WM_LBUTTONDOWN, but the invoke still runs synchronously on the window's own thread, so the folder view is frozen while the handler works (e.g. VS Code cold-starting). Setting ci.fMask |= CMIC_MASK_ASYNCOK lets handlers that support it do their work off the UI thread.
• Substring matching is greedy. "code" also matches "Encode"/"Decode with …", and the first match in menu order wins. An option for exact/whole-word matching on the normalized string would make the setting more predictable for short inputs.
• SendInput targets the foreground window. Now that actions are posted asynchronously, a focus change between the click and the dispatch sends the injected keys somewhere else. Very unlikely in practice, but the hotkey-based actions (New Tab, Close Tab, New Folder, Paste) are the ones affected.
• Matching order across submenus: EnumContextMenuMatch descends into a submenu before checking any later top-level item, so an entry buried in "New" can win over a later top-level entry that also matches. Worth a sentence in the README's "Multiple matches" tip.