Skip to content

Bump to 1.7.0: cover common display languages in chevron detection - #5260

Open
wygodad wants to merge 8 commits into
ramensoftware:mainfrom
wygodad:patch-3
Open

Bump to 1.7.0: cover common display languages in chevron detection #5260
wygodad wants to merge 8 commits into
ramensoftware:mainfrom
wygodad:patch-3

Conversation

@wygodad

@wygodad wygodad commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

This updates the already-published Tray hover expand mod to 1.7.0.

The main change is how the chevron is identified. Until now it was matched by its
localized name, with a positional guess as a fallback, so users whose display
language was not in the keyword list fell through to that guess, which can select
a different tray button and invoke it. It is now identified by its class name
together with its AutomationId, which is the same in every display language, and
when it cannot be identified the mod does nothing at all instead of guessing.

The rest of the changes come from user reports about auto-collapse, the chevron
tooltip and tray icon context menus.

Changelog

  • Identify the chevron by its class name plus AutomationId, independent of the
    display language, and accept it only when exactly one element matches
  • Restrict detection to tray elements, so no taskbar button can be matched by a
    keyword and invoked
  • Do nothing instead of guessing the chevron by position when it cannot be
    identified; the guess is now an opt-in setting, off by default
  • Cover the most common Windows display languages in the name fallback
  • Add a "Hover delay" setting, so the flyout does not open when the cursor only
    brushes past the chevron
  • Keep tray icon context menus usable: detect clicks that fall between polling
    ticks, and never collapse while a context menu is open
  • Suppress the chevron tooltip whenever the cursor is on it, covering both
    "Show hidden icons" and "Hide", and with the taskbar on either edge
  • Hide only tooltips owned by the taskbar process
  • Do not treat an unrendered chevron as a hover target, which could turn the
    corner of the screen into a hotspot and make the flyout cycle
  • Drive re-finding from the cursor instead of a fixed timer, with backoff on
    repeated failures, so an auto-hiding taskbar works immediately on reveal and a
    taskbar with no hidden icons costs almost nothing
  • Scope the fullscreen check to the monitor the chevron is on
  • Set per-monitor-v2 DPI awareness on the worker thread
  • Document that the mod is Windows 11 only, and how to restore the keyword
    defaults

Fixes #5257
Fixes #4684
Fixes #4732
Fixes #4747
Fixes #4524

Note on #4524: two of its three points are fixed here, along with the context
menu problem raised in a follow-up comment. The remaining one, the flyout
occasionally taking a second to disappear, I was unable to reproduce; the
reporter has been asked to say so if it persists.

Mod authorship

If this pull request introduces a new mod, please complete the section below.

This mod was created by:

    • The submitter, without AI assistance
    • The submitter, with AI assistance
    • Claude
    • ChatGPT
    • Gemini
    • Another AI (please specify):
    • Other (please specify):

Please select the options that best apply. Your selection does not affect the acceptance criteria, but it helps reviewers understand the context of the code and provide relevant feedback.

…pdate keywords)

Fixes ramensoftware#5257

Users on locales outside the default keyword list fell back to the
positional guess, which can select the wrong tray button (reported for
Dutch in ramensoftware#5257: the fallback alternated between the chevron and the
Quick Settings button).
- Expand the default chevron name keywords to cover the most common
  Windows display languages
- Use CharLowerBuffW instead of towlower for case-insensitive matching,
  so non-ASCII scripts (Cyrillic, Greek) fold correctly
- Update the readme guidance for locales still missing from the list
@windhawk-reviewer windhawk-reviewer Bot added the waiting-for-author The author's turn: request an AI review, or respond to one that was posted. label Aug 28, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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.

@wygodad

wygodad commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 28, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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 diagnosis in issue 5257 is right and the fix direction is reasonable, but the keyword list is applied over a much wider set of elements than just the tray, and widening it from 4 to 24 substrings makes that a real problem. The findings below are mostly about the detection logic the PR touches.

1. Name matching is applied to every button on the taskbar, not just tray buttons

FindOverflowButton (line 292) does ElementFromHandle(Shell_TrayWnd)FindAll(TreeScope_Subtree, ControlType == Button). On Windows 11 that subtree contains Start, Search, Task view, Widgets, every app button in the task list, the tray icons, the clock and the notification-center button — numbered-taskbar uses exactly this query to enumerate app buttons, and has to filter them out by AutomationId/ClassName.

The name loop (lines 320-327) accepts the first element in tree order whose name contains any keyword, with no AutomationId or ClassName check — and task list buttons come before the tray in that order. Taskbar buttons are named after the window/app they represent, so:

  • A browser window on Stack Overflow matches the overflow keyword.
  • A window titled "…hidden icons…" matches hidden icons; rozwiń ("expand") is an equally generic Polish word.

The consequence isn't just a missed chevron: the matched element becomes the hover target, so hovering that taskbar button makes the mod Invoke() it — activating/minimizing an unrelated window. This is latent in 1.6.0 (overflow was already there) but adding 20+ common words across many languages makes a collision much more likely.

Restrict the candidate set before matching by name — at minimum require the same AutomationId the fallback already uses:

BSTR aid = nullptr;
e->get_CurrentAutomationId(&aid);
bool isTrayIcon = (aid && s.trayIconAutomationId == aid);
if (aid) SysFreeString(aid);
if (!isTrayIcon) { e->Release(); continue; }   // then name-match / leftmost

Better still, scope the search root itself: FindFirst the notification-area container (e.g. by UIA_ClassNamePropertyId) and run FindAll from there, the way numbered-taskbar narrows to Taskbar.TaskList before enumerating.

2. Match the chevron by its language-independent class name instead of by localized text

The readme says the chevron has no language-independent identifier, but it does have one that several merged mods already rely on: the XAML type name SystemTray.ChevronIconView — see taskbar-notification-icon-spacing.wh.cpp#L556, taskbar-blob-shape.wh.cpp#L1059 and taskbar-vertical.wh.cpp#L2040. UIA surfaces those dotted XAML type names through UIA_ClassNamePropertyId / get_CurrentClassNamenumbered-taskbar compares against Taskbar.SystemTrayIcon, SystemTray.NormalButton, SystemTray.OmniButtonCenter that way.

Worth verifying with Accessibility Insights on your build, but if it holds it makes the whole language problem go away: match on the class name first, fall back to the keyword list, and only then to position. That is a real fix for 5257 rather than a list that will always be missing someone's locale (and whose entries are build-specific — gizli simgeleri won't match a build that renders "Gizli simgeler", κρυφών εικονιδίων won't match a nominative-case rewording, etc.).

3. The positional fallback — the part that actually misfired in 5257 — is unchanged

Per the issue thread, the reported symptom was the fallback landing on Quick Settings. Any locale still missing from the list (and any future build where the wording changes) hits the same path, so it's worth hardening while you're here:

  • Reject degenerate rectangles. UIA returns an empty {0,0,0,0} rect for offscreen/non-rendered elements; with r.left == 0 such an element always wins the r.left < leftmostX comparison at line 338 and becomes the "leftmost tray button". numbered-taskbar guards this explicitly: bounds.right > bounds.left && bounds.bottom > bounds.top. Also skip elements where get_CurrentIsOffscreen is true.
  • Wh_Log the element that was picked (name + AutomationId + class name + rect) when the fallback is used. The next report like 5257 then takes one log instead of a video.

4. HideChevronTooltip hides windows belonging to other processes

FindWindowExW(nullptr, h, s.tooltipClass.c_str(), nullptr) (line 394) enumerates all top-level windows of that class on the desktop, across every process. Xaml_WindowedPopupClass is the generic WinUI/XAML popup host used by Settings, Terminal, Photos and any WinUI 3 app, and the geometry filter is loose — r.bottom <= chevron.bottom accepts anything above the taskbar, so any XAML popup that happens to overlap the chevron's narrow x-range gets ShowWindow(SW_HIDE). Filter by the taskbar's process, e.g.:

DWORD taskbarPid = 0;
GetWindowThreadProcessId(FindWindowW(L"Shell_TrayWnd", nullptr), &taskbarPid);
...
DWORD pid = 0;
GetWindowThreadProcessId(h, &pid);
if (pid != taskbarPid) continue;

(The setting is off by default, so this is not the common path, but hiding another app's popup is not recoverable from the user's side.)

5. Mixed-DPI: UIA rectangles and GetCursorPos may not be in the same coordinate space

get_CurrentBoundingRectangle returns physical screen coordinates, while GetCursorPos / GetWindowRect are subject to DPI virtualization unless the calling thread is per-monitor-v2 aware. Unless you've verified windhawk.exe is already PMv2 aware, the hover test at line 510 can be off on multi-monitor setups with different scaling per monitor. It's a cheap one-liner at the top of WorkerThread, and other tool mods do the same — bt-battery-monitor.wh.cpp#L1676, audioswap.wh.cpp#L1186:

SetThreadDpiAwarenessContext(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2);
Optional improvements

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

  • #include <algorithm> is now unused (line 166). This PR removed the only std::transform; nothing else in the file uses <algorithm>.
  • Lowercase the keywords once, at load time. NameMatches calls ToLower(k) for every keyword on every candidate (line 246) — that's now 24 allocations per element per scan. Store a pre-lowered copy in Settings when LoadSettings builds the list.
  • Settings-generation check reads the counter after the snapshot (lines 465-467). LoadSettings writes g_settings and then bumps the generation; the worker snapshots first and only then reads the generation into settingsGen, so an update landing between the two lines is recorded as "already applied" and silently dropped until the next change. Read the generation first:
    int gen = g_settingsGeneration;
    if (gen != settingsGen) { s = GetSettingsSnapshot(); settingsGen = gen; ... }
  • The comment on the unload wait isn't accurate (lines 676-677). The worker also blocks inside cross-process UIA calls (FindAll, get_CurrentBoundingRectangle, Invoke) into explorer.exe, so WaitForSingleObject(g_thread, INFINITE) can take considerably longer than the poll interval if the shell is busy. The wait itself is fine (UIA has its own timeouts) — just worth not claiming otherwise in the comment.
  • Tool-mod boilerplate has drifted from the wiki snippet. The CreateProcessInternalW_t typedef dropped its parameter names and commandLine was re-wrapped. Functionally identical, but the maintainer prefers this block byte-for-byte identical to the wiki version so it can be skimmed at a glance — cf. explorer-folder-hover-menu.wh.cpp.
  • The PR body's Changelog section still has the template placeholders ("Changelog item 1..."); this PR updates an existing mod, so it'd be good to fill it in.

Functionality notes

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

  • Existing users won't get the new defaults. Windhawk persists a mod's settings once they've been saved, so anyone who already opened the settings page (like the reporter in 5257, who changed suppressInFullscreen) keeps the old four-keyword list and sees no change in 1.7.0. Worth a line in the readme/changelog telling those users to reset the "Chevron name keywords" setting or add their language manually.
  • CharLowerBuffW caveats. It's a clear improvement over towlower, but it's a per-character non-linguistic mapping: it always maps Ii even under Turkish/Azeri, and it doesn't do full Unicode case folding (Greek final sigma, etc.). None of the current keywords are affected, but LCMapStringEx(..., LCMAP_LOWERCASE | LCMAP_LINGUISTIC_CASING, ...) is the linguistically correct form if you ever need it.
  • Polling every 50 ms is fine here. There's no cross-process hover event for a foreign window, so a poll loop is the reasonable approach, it runs in a dedicated process, and each tick is only GetCursorPos + a couple of cheap Win32 calls. Noting it only so it doesn't get raised as a concern later.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 28, 2026
Detection is now restricted to tray elements and no longer guesses.

- Restrict candidates to tray elements (class name prefixed SystemTray.,
  or the tray AutomationId) before any name or position test, so a task
  list button named after its window title can no longer be matched and
  invoked
- Identify the chevron by class name plus AutomationId, which is
  language-independent, and accept it only when unambiguous
- Keep name matching as a fallback, drop the generic "overflow" and
  "rozwin" keywords
- Turn the positional guess into an opt-in setting, off by default: when
  the chevron cannot be identified the mod now does nothing and logs
  every tray candidate instead of invoking an unidentified button
- Reject offscreen elements and degenerate rectangles
- Hide only tooltips owned by the taskbar process
- Set per-monitor-v2 DPI awareness on the worker thread
- Read the settings generation before the snapshot, pre-lowercase the
  keywords, drop the unused <algorithm> include, correct the unload
  comment, restore the tool-mod boilerplate to the wiki version
@wygodad

wygodad commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, this was a useful review. Everything is addressed, with one finding corrected by measurement.

Finding 2 does not hold for UIA on this build. I enumerated the entire UIA tree under Shell_TrayWnd (103 elements, all control types, depth 12) on 25H2 build 26200.9278: there is no SystemTray.ChevronIconView anywhere, and no SystemTray.SystemTrayFrame container either. The mods you cite inject into explorer.exe and walk the XAML visual tree, where that type does exist; UIA exposes the automation peer's class name instead. What UIA reports:

Element ClassName AutomationId
Chevron SystemTray.NormalButton SystemTrayIcon
Notification icons SystemTray.NormalButton NotifyItemIcon
Network, battery SystemTray.AccentButton SystemTrayIcon
Volume SystemTray.OmniButtonCenter SystemTrayIcon
Clock SystemTray.OmniButtonLeft SystemTrayIcon
Notification centre SystemTray.OmniButtonRight SystemTrayIcon
Show desktop SystemTray.ShowDesktopButton SystemTrayIcon

So the goal of your finding is met, just through the pair rather than a single class: the chevron is the only tray element carrying SystemTray.NormalButton together with SystemTrayIcon. It is accepted only when exactly one element matches, and both values are settings.

  1. Fixed at the root. Candidates are restricted to tray elements before any name or position test, so no task list button can be matched or invoked. Since the notification-area container is not exposed through UIA, the class-name prefix is the scoping mechanism.

  2. Went further than suggested. Offscreen elements and degenerate rectangles are rejected, but the positional guess itself is now opt-in and off by default. Geometry validates a candidate's shape, not its identity, so when the chevron is absent the leftmost valid element is still the wrong button. The mod now does nothing and logs every tray candidate (class, AutomationId, position, name) instead.

4, 5. Tooltip hiding is filtered by the taskbar's process id, and the worker thread sets per-monitor-v2 DPI awareness.

All optional items done: unused include removed, keywords pre-lowercased at load, settings generation read before the snapshot, unload comment corrected, boilerplate restored to the wiki version. The readme now tells existing users to reset the keyword setting to pick up new defaults.

Verified on 25H2 26200.9278 (Polish): the mod log shows only open/collapse lines, confirming identification goes through the class name and never reaches the name fallback.

@wygodad

wygodad commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 28, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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 previous round's findings are all addressed, and the measurement-backed pushback on the SystemTray.ChevronIconView suggestion is correct — UIA exposes the automation peer's class name, not the XAML type, so class + AutomationId is the right pair here. The tool-mod boilerplate is now byte-for-byte identical to the wiki snippet. What's left is in the worker loop rather than in the detection rewrite.

1. The periodic rectangle refresh doesn't apply the guard that discovery does

FindOverflowButton rejects offscreen elements and degenerate rectangles (lines 385-392), but the refresh path that runs every RECT_REFRESH_MS only checks the HRESULT:

if (SUCCEEDED(pBtn->get_CurrentBoundingRectangle(&r))) {
    cachedRect = r;
    haveRect = true;
}

IUIAutomationElement::get_CurrentBoundingRectangle returns S_OK with an empty rectangle when the element is not currently displaying a UI — it does not fail. So once the chevron stops being rendered while the element itself stays alive (auto-hiding taskbar retracted, taskbar temporarily hidden, the last hidden icon removed), cachedRect becomes {0,0,0,0} and haveRect stays true. PtInRectPad then treats x ∈ [-pad, pad], y ∈ [-pad, pad] as the chevron, i.e. the top-left corner of the primary monitor becomes a hover hotspot, and moving the cursor there fires DoExpand(pBtn) on a still-valid element — the flyout opens out of nowhere. suppressInFullscreen covers the fullscreen case but not the auto-hide one.

Apply the same guard as in discovery, and treat a bad rect like a stale element:

RECT r;
BOOL offscreen = FALSE;
if (SUCCEEDED(pBtn->get_CurrentBoundingRectangle(&r)) &&
    r.right > r.left && r.bottom > r.top &&
    SUCCEEDED(pBtn->get_CurrentIsOffscreen(&offscreen)) && !offscreen) {
    cachedRect = r;
    haveRect = true;
} else {
    pBtn->Release(); pBtn = nullptr;
    haveRect = false;
    overBtnPrev = false;   // also stale once the button is gone
    nextRefind = 0;
    WaitForSingleObject(g_stopEvent, s.pollInterval);
    continue;
}

2. ACTION_COOLDOWN_MS covers the open but not the collapse, so auto-collapse can re-open the flyout

cooling is derived from lastOpenAt, which is only set in the expand branch (line 683). The collapse branch doesn't set it:

} else if (now - leftAt >= (ULONGLONG)s.grace) {
    DoCollapse(pBtn);
    leftAt = 0;
    flyoutBelievedOpen = false;

Since the chevron exposes no ExpandCollapse pattern, DoCollapse is a second Invoke(), i.e. a toggle. The flyout does not disappear instantly, so on the next ticks GetVisibleFlyout still returns it, cooling is false, leftAt is re-armed, and after grace the mod invokes the chevron again — which re-opens the flyout it just closed. With the default grace of 200 ms this is usually masked by the flyout hiding in time, but grace is user-settable down to 0 (if (s.grace < 0) s.grace = 0;), and at small values it turns into a visible open/close flicker loop.

Treat a collapse as an action for cooldown purposes — rename lastOpenAt to lastActionAt (which is what cooling already means) and set it in both branches:

DoCollapse(pBtn);
lastActionAt = now;
leftAt = 0;
flyoutBelievedOpen = false;

This also gives the enterEdge test a 300 ms guard after a collapse, which is desirable anyway: without it, a cursor that is still near the padded hit area right after the collapse can immediately re-trigger the open.

3. The trayIconAutomationId setting's name and description no longer match what it does

- trayIconAutomationId: SystemTrayIcon
  $name: Tray icon AutomationId (fallback)
  $description: Used only when the chevron is not matched by name. The leftmost tray button with this AutomationId is then assumed to be the chevron.

After this PR the value is used in three places, only one of which is the fallback: the tray-element filter (automationId == s.trayIconAutomationId, line 380), the primary class-name + AutomationId identification (line 414), and the positional guess (line 440) — which is itself now gated behind positionalFallback, not behind "not matched by name". A user who follows the description and edits this field to fix the leftmost-guess would silently disable the primary detection path, which is exactly the failure mode this release is meant to remove. Drop the "(fallback)" suffix and describe it as the AutomationId paired with "Chevron class name", the way the chevronClass description already does.

Optional improvements

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

  • Fetch the candidate properties with a cache request. FindAll + get_CurrentClassName / get_CurrentAutomationId / get_CurrentIsOffscreen / get_CurrentBoundingRectangle / get_CurrentName is one cross-process RPC into explorer.exe per property per button, so a walk over a Windows 11 taskbar is roughly 100-200 round trips. That's a one-off when the chevron is found, but it repeats every REFIND_INTERVAL_MS (3 s) for as long as it isn't — which is the steady state for anyone with no hidden icons. IUIAutomation::CreateCacheRequest + AddProperty(...) + pRoot->FindAllBuildCache(TreeScope_Subtree, pCond, pCache, &pArr) collapses that into a single call, and the get_Cached* accessors replace the get_Current* ones. The default AutomationElementMode_Full keeps the returned elements usable for GetCurrentPatternAs, so DoExpand/DoCollapse are unaffected. explorer-folder-hover-menu.wh.cpp and mutealert.wh.cpp both use cache requests.
  • The "not identified" candidate dump repeats every 3 s. It's the right diagnostic, but re-emitting the whole table on every retry buries the rest of the log and makes it harder to read the one thing you asked users to attach. Logging it once per failure streak (e.g. a bool loggedCandidates reset whenever a chevron is found) would keep it useful.
  • CUIAutomation8 would make the unload comment provably true. The comment on WaitForSingleObject(g_thread, INFINITE) relies on UIA's built-in timeouts; you can set them explicitly by creating CUIAutomation8 instead of CUIAutomation, querying IUIAutomation2 and calling put_ConnectionTimeout / put_TransactionTimeout. Cheap, and it turns an assumption into a guarantee.
  • GetVisibleFlyout isn't process-filtered. HideChevronTooltip now checks the taskbar PID, but FindWindowW(s.flyoutClass.c_str(), nullptr) still matches the first window of that class anywhere on the desktop. TopLevelWindowForOverflowXamlIsland is explorer-only in practice, but the class is user-settable, so the same check would be consistent — and it protects a user who points the setting at a more generic class on a build where auto-collapse doesn't work.
  • HideChevronTooltip runs a full desktop window enumeration every tick. While the flyout is open and the cursor is on the chevron it does FindWindowW + a FindWindowExW walk of every top-level window of the class, 20×/s at the default poll interval. Resolving taskbarPid once (it's already implied by the cached chevron element) and/or only running the sweep on the tick the flyout becomes visible would cut most of that.
  • Say how to "reset" the keyword list. The readme tells pre-1.7.0 users to reset "Chevron name keywords", but Windhawk's settings UI has no per-setting reset. What actually works is what LoadSettings already implements — delete every entry and save, and if (!keywords.empty()) falls back to the built-in list. Worth spelling out, otherwise the advice isn't actionable.
  • TRAY_CLASS_PREFIX could be a constexpr std::wstring_view with className.starts_with(TRAY_CLASS_PREFIX) (C++23 is available), instead of a global std::wstring plus a three-argument compare. Same behaviour, no static initialization, and the intent reads directly.

Functionality notes

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

  • There's no hover dwell time. enterEdge fires on the first tick the cursor is inside the padded rect, and Invoke makes the flyout foreground — so brushing past the chevron on the way to the clock or the notification centre opens it and takes keyboard focus away from whatever the user was typing in. An optional "Hover delay (ms)" (require the cursor to stay inside the rect for N ms before expanding, N = 0 keeping today's behaviour) would make accidental triggers cost nothing.
  • IsForegroundFullscreen uses the foreground window's monitor, not the chevron's. A fullscreen game on monitor 2 suppresses the flyout on monitor 1, where the taskbar is fully visible and nothing would be covered. Comparing MonitorFromWindow(hwnd, ...) against MonitorFromRect(&cachedRect, ...) and only suppressing when they match would scope it to the case the setting describes.
  • The positional guess assumes an LTR layout. On RTL display languages (Arabic, Hebrew) the tray sits on the left and the chevron is the rightmost tray button, so positionalFallback picks the wrong end. Those two languages are also absent from the keyword list, so RTL users land on exactly the path that caused issue 5257. Since the guess is opt-in and logged, this is only worth handling if you want it to cover them — checking the taskbar's WS_EX_LAYOUTRTL/GetProcessDefaultLayout, or just picking the tray button furthest from the notification-centre button, would do it.
  • ChevronButton may be a second valid AutomationId. numbered-taskbar.wh.cpp#L180 lists ChevronButton among the tray AutomationIds it excludes, which suggests some builds expose the chevron with that id rather than SystemTrayIcon. On such a build your class+id pair matches nothing and detection drops to the keyword list. Worth checking whether accepting ChevronButton as an additional unambiguous signal is safe on your build — if it is, it's free coverage.
  • The readme no longer states which Windows version is supported. The 1.6.0 text said "on Windows 11"; the rewrite dropped it. The class names and AutomationIds this release keys on are Windows 11 shell types, so on Windows 10 nothing is identified and the mod does nothing (which is the correct, safe outcome — just undocumented now). A one-line "Windows 11 only" in the Notes would save a few reports.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 28, 2026
- Apply the discovery guard to the periodic rectangle refresh: an element
  that is alive but not rendered returns S_OK with an empty rectangle, which
  turned the top-left corner of the screen into a hover hotspot and made the
  flyout cycle open and closed under a stationary cursor
- Arm the action cooldown on collapse as well, so a collapse cannot be
  followed by an immediate re-open at small collapse delays
- Rename the chevron AutomationId setting and describe it as part of the
  primary identification rather than a fallback
- Fetch candidate properties through a cache request instead of one
  cross-process call per property
- Log the candidate table once per failure streak
- Filter the flyout lookup by the taskbar process and resolve that process
  id once instead of per tick
- Add an opt-in hover delay before the flyout opens
- Scope the fullscreen check to the monitor the chevron is on
- Accept ChevronButton as a second chevron AutomationId
- Document that the mod is Windows 11 only and how to restore the keyword
  defaults
@wygodad

wygodad commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

All three required findings are fixed, and most of the optional items as well.

  1. The refresh path now applies the same guard as discovery and treats an empty rectangle like a stale element. Thanks for this one: get_CurrentBoundingRectangle returning S_OK with an empty rect is also a plausible cause of [Tray hover expand] Unintentional cyclical expand / collapse #4684, where a user with an auto-hiding taskbar sees the flyout cycle open and closed. A rectangle that alternates between valid and empty under a stationary cursor produces exactly that loop.
  2. lastOpenAt is now lastActionAt and the collapse arms it too.
  3. The chevron AutomationId setting is renamed and described as part of the primary identification, since it is no longer a fallback-only value.

Optional items done: cache request via FindAllBuildCache with get_Cached* accessors, the candidate dump logged once per failure streak, GetVisibleFlyout filtered by the taskbar process, the taskbar process id resolved once instead of per tick, TRAY_CLASS_PREFIX as a wstring_view with starts_with, and the readme now says how to restore the keyword defaults (clear the list and save).

On the tooltip process filter from the previous round: I verified the chevron tooltip is owned by explorer.exe, the same process as Shell_TrayWnd, so that filter is correct and hiding still works.

From the functionality notes: added an opt-in "Hover delay" setting, which also covers a standing request in #4524; scoped the fullscreen check to the chevron's monitor; accepted ChevronButton as a second AutomationId, which is free coverage given the pair is only used when exactly one element matches; and documented that this is Windows 11 only.

Deliberately skipped: RTL handling in the positional guess, which is opt-in and logged, and explicit CUIAutomation8 timeouts, since UIA applies its own.

Verified on 25H2 build 26200.9278 at 140% scaling.

@wygodad

wygodad commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 28, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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 previous round's three items are all correctly fixed — the rect refresh now applies the discovery guard and drops the element on an empty rect, lastActionAt is armed on collapse too, and the AutomationId setting is described as part of the primary identification. The tool-mod boilerplate diffs byte-for-byte clean against the wiki snippet. Both remaining findings are in the worker loop's state machine.

1. Clicking the chevron to close the flyout makes the mod re-open it immediately

dwellFired is only set inside the enterEdge && !flyoutVisible branch (line 781), so it stays false for any hover stay during which the flyout was already open. That is a very common state: the flyout renders directly above the chevron, so the natural gesture is cursor → chevron (mod opens it) → up into the icons (overBtn goes false, which resets insideSince and dwellFired at lines 739-741) → back down onto the chevron → click to close.

On that last step enterEdge is true on every tick (overBtn && !dwellFired && !cooling && now - insideSince >= hoverDelay) and is only held back by !flyoutVisible. The moment the user's click toggles the flyout closed, the next tick (≤ pollInterval) sees flyoutVisible == false with enterEdge still true and calls DoExpand — the flyout the user just dismissed pops straight back up. cooling doesn't help, because the mod itself took no action, and clickedInFlyout doesn't either, since the click is on the chevron, not inside the flyout window. Pressing Esc while the cursor rests on the chevron has the same result, and with hoverDelay > 0 it also happens on the very first click (the user clicks before the dwell elapses, so dwellFired was never set).

The cleanest fix is to treat "the flyout was open during this stay" as having served the stay, so a re-open requires leaving and re-entering the hit area:

if (overBtn && flyoutVisible) {
    dwellFired = true;   // this stay has already seen an open flyout
}
if (enterEdge && !flyoutVisible) {
    dwellFired = true;
    ...
}

(Placed after flyoutVisible is computed at line 764. Arming it from the mouse-down edge you already track — anyBtnDown && !anyBtnDownPrev && overBtndwellFired = true; lastActionAt = now; — works too, but it can miss a short click at a large pollInterval, so the state-based form above is more robust.)

2. The fixed 3 s re-find timer is both too slow and too eager

REFIND_INTERVAL_MS (line 531) is the only thing that drives re-acquisition, and now that the rect guard correctly drops the element whenever the chevron isn't rendered (lines 708-725), that timer decides how responsive the mod is. It's wrong in both directions:

  • Auto-hide taskbars: up to 3 s of the mod doing nothing. When the taskbar retracts, the chevron reports an empty rect, pBtn is released and the next FindOverflowButton finds no on-screen tray candidates, so nextRefind is pushed 3 s out. The user then moves to the bottom edge, the taskbar slides in, and hovering the chevron does nothing until the timer fires — 1.5 s on average, 3 s worst case, on every reveal. That's the mod's whole feature intermittently not working in a common configuration.
  • No hidden icons: a full UIA subtree walk of Shell_TrayWnd every 3 s, forever. When nothing is hidden the chevron doesn't exist, so the "not found" path is the permanent steady state for those users, and the mod keeps re-walking the taskbar subtree for the life of the session.

One change fixes both: drive re-finding from the cursor instead of from a timer. The cursor is already read every tick, so gate it on the cursor being over the taskbar (or, cheaper, on the last known chevron rect / the taskbar's monitor edge) — retry immediately while it is, and don't walk at all while it isn't:

// Only worth re-finding when the cursor is somewhere the chevron could be.
POINT pt; GetCursorPos(&pt);
HWND hTaskbar = FindWindowW(L"Shell_TrayWnd", nullptr);
RECT tb;
bool nearTaskbar = hTaskbar && GetWindowRect(hTaskbar, &tb) &&
                   PtInRectPad(tb, pt, TASKBAR_REVEAL_PAD);
if (!pBtn && nearTaskbar && now >= nextRefind) { ... }

With nextRefind then only throttling the hot path (a few hundred ms is plenty), an auto-hide user gets the chevron back as soon as the taskbar is up, and a user with no hidden icons stops paying for the walk entirely. If you'd rather not add the cursor test, at minimum make the retry adaptive — short (e.g. 250 ms) while the cursor is in the bottom strip of the screen, long (or backing off) otherwise.

Optional improvements

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

  • The __IUIAutomation_FWD_DEFINED__ #error guard (lines 204-206) is dead weight. If <uiautomation.h> didn't provide the interface the file wouldn't compile anyway, and the check has no fallback to offer. Dropping it removes three lines of noise.
  • CHEVRON_AUTOMATION_ID_ALT is hardcoded while the primary AutomationId is a setting (line 262 vs. trayIconAutomationId). If a future build renames the id the user can override one value but not the other, and they can't remove ChevronButton if it ever starts matching something else. Either make the setting a list, or add a short comment saying the alt id is intentionally fixed.
  • The ambiguity log repeats every 3 s. Wh_Log(L"%d tray elements share the chevron signature…") (line 471) fires on every re-find attempt, which is the same problem you just fixed for the candidate dump — it could reuse the loggedCandidates streak flag.
  • The rect-refresh bail-out doesn't reset the flyout-tracking state (lines 716-724). It clears haveRect, overBtnPrev, insideSince and dwellFired, but leaves leftAt and flyoutBelievedOpen set, so if the chevron goes stale while the flyout is open, the first tick after it comes back can collapse immediately instead of waiting out grace. Clearing leftAt there would match the rest of the reset.
  • The settings-change path doesn't reset the hover-dwell state either (lines 669-678). overBtnPrev and insideSince survive, so if the cursor happens to be on the chevron when settings are saved, the next open skips the configured hoverDelay. One extra line alongside the existing resets.

Functionality notes

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

  • The candidate dump logs an empty table on every auto-hide cycle. With the guard in place, a hidden taskbar yields zero on-screen tray candidates, so the failure path emits Chevron not identified among 0 tray candidates: each time the streak restarts. Skipping the dump when cands.empty() (nothing was visible, so there's nothing to diagnose) would keep the log to the case it was written for. Fixing item 2 above reduces how often this fires, but doesn't remove it.
  • Running a UIA client at all has a standing cost in explorer. Once the mod's IUIAutomation is alive, UiaClientsAreListening() is true in the provider process and the XAML taskbar builds automation peers it would otherwise skip. There's no way around it for this design — UIA is the only cross-process handle on the chevron — so this is purely an FYI, not something to change.
  • PtOverWindow uses the flyout's full window rect. XAML island popups usually carry a transparent shadow/margin border, so the "cursor is over the flyout" area is a few pixels larger than what the user sees. It only makes auto-collapse marginally stickier, which is the harmless direction, but it's worth knowing if someone reports the flyout not closing right at its edge.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 28, 2026
- Treat a hover stay that has seen an open flyout as served, so clicking
  the chevron to dismiss the flyout no longer re-opens it on the next
  tick. Moving up into the icons and back down onto the chevron used to
  leave the enter edge armed, because the flag that spends it was only
  set in the branch that opens the flyout
- Drive re-finding from the cursor instead of a fixed timer. The
  rectangle guard drops the element on every auto-hide retract, so a
  fixed interval left the mod idle for up to three seconds after each
  reveal; conversely, with no hidden icons the chevron never exists and
  the timer walked the taskbar subtree for the life of the session.
  Retry every 250 ms while the cursor is at the taskbar, back off while
  it is elsewhere
- Clear the collapse timer when the element is dropped, so the flyout is
  not collapsed on the first tick after the chevron comes back
- Reset the hover-dwell state when settings change, so a cursor resting
  on the chevron does not skip the configured hover delay
- Log the ambiguous-signature line once per failure streak, and skip the
  candidate dump when nothing was rendered
- Drop the dead UI Automation header guard, note why the alternate
  chevron AutomationId is deliberately not a setting
@wygodad

wygodad commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

Both findings confirmed in the code and fixed.

  1. Reproduced exactly as described: leaving the hit area clears the flag that spends the enter edge, and the branch that sets it again only runs when the flyout is closed, so a stay entered from the icons kept the edge armed indefinitely. A stay that has seen an open flyout now counts as served, so re-opening requires leaving and re-entering.

  2. Re-finding is now driven by the cursor. It retries every 250 ms while the cursor is within a padded taskbar rectangle and backs off otherwise, so an auto-hide user gets the chevron back as soon as the taskbar is up, and a user with no hidden icons stops paying for the subtree walk entirely. This matters for [Tray hover expand] Unintentional cyclical expand / collapse #4684, which is an auto-hiding taskbar setup.

All optional items done as well: the collapse timer is cleared when the element is dropped, the hover-dwell state is reset on a settings change, the ambiguity line is gated by the same streak flag as the candidate dump, the dump is skipped when nothing was rendered (a retracted taskbar produces no candidates and needs no diagnosis), the dead header guard is gone, and the alternate AutomationId now carries a comment explaining why it is fixed rather than configurable.

On the functionality notes: PtOverWindow using the flyout's full window rect is intentional, since the shadow margin only makes auto-collapse slightly stickier, which is the harmless direction.

The reporter in #4684 has since confirmed the root cause was a missing keyword for Czech and that the 1.7.0 approach addresses it, so the PR now closes that issue too. Verified on 25H2 build 26200.9278 at 140% scaling, including the click-to-dismiss gesture from finding 1.

@wygodad

wygodad commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added the waiting-for-ai-review An AI review was requested and is being prepared. label Aug 29, 2026
@windhawk-reviewer

Copy link
Copy Markdown

This pull request has already had 3 AI reviews in the last 24 hours, which is the limit, so no review was posted this time.

Comment /ai-review again after 2026-08-29 20:05 UTC (in 1 hour) to get another one.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 29, 2026
@wygodad

wygodad commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 29, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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 previous round's two items are correctly fixed — a stay that has seen an open flyout is now served, so the click-to-dismiss gesture no longer re-opens, and re-finding is cursor-driven. The tool-mod boilerplate still diffs byte-for-byte clean against the wiki snippet, and the settings block and the code are in sync in both directions. All three findings below are in the new code: the context-menu guard, the broadened tooltip suppression, and the cost of the new re-find cadence.

1. IsPopupMenuOpen only inspects the first #32768 window, so the guard can silently report "no menu"

static bool IsPopupMenuOpen() {
    HWND h = FindWindowW(L"#32768", nullptr);
    return h && IsWindowVisible(h);
}

FindWindowW returns only the first top-level window of that class and stops. Menu windows are created lazily per thread and then kept around hidden — after any process on the desktop has shown a menu once, its #32768 window stays alive and retains a Z-order position. So the one FindWindowW happens to return may be a hidden leftover, in which case the function returns false even though a tray icon's context menu is on screen, and auto-collapse dismisses it — the exact symptom this guard was added for (issues 4732 / 4524). The mod already gets this right one function up, in GetVisibleFlyout (line 556), which enumerates rather than sampling the first hit:

static bool IsPopupMenuOpen() {
    HWND h = nullptr;
    while ((h = FindWindowExW(nullptr, h, L"#32768", nullptr))) {
        if (IsWindowVisible(h)) return true;
    }
    return false;
}

Note also that this test is desktop-wide and unfiltered — a menu open in any unrelated application suspends auto-collapse for as long as it is up. That is the safe direction and probably fine, but it is worth knowing; the tray-icon menus you actually care about belong to the icon's owner process, so filtering by PID is not an option, but you could require the menu to intersect the flyout or the taskbar's monitor.

2. Tooltip suppression now runs on plain hover, and Xaml_WindowedPopupClass is not tooltip-exclusive — right-clicking the chevron can hide the taskbar context menu

HideChevronTooltip used to be called only under overBtn && flyoutVisible; it is now called whenever the cursor is on the chevron (line 840), and the geometry test accepts any taskbar-owned tooltipClass window that overlaps the chevron horizontally and sits within one chevron-height band above or below it (lines 620-629).

Xaml_WindowedPopupClass is explorer's generic WinUI popup host, not a tooltip class — taskbar-multi-tray treats windows of that class as "native control center or context menu". Right-clicking the chevron is an ordinary thing to do (it brings up "Taskbar settings"), and the resulting popup is explorer-owned, horizontally over the chevron, and its bottom edge sits right against the chevron's top edge — so overlapsX && besideChevron matches and the mod calls ShowWindow(h, SW_HIDE) on it. With "Hide the chevron tooltip" enabled the menu would flash and vanish, with no way for the user to bring it back. The same reasoning applies to any other XAML popup that happens to be anchored at the chevron.

Since the class alone cannot discriminate, add a property that can. A tooltip is a single short line; a menu is not, so a height cap is the cheapest filter:

LONG band = chevron.bottom - chevron.top;
if (r.bottom - r.top > 2 * band) continue;   // menus/flyouts, not a tooltip

An interaction-based test works too (a tooltip never becomes the foreground window and never takes capture, a menu does), or simply skip the whole pass while a mouse button is down. Whichever you pick, please verify what the Win11 taskbar context menu reports for its class on your build before deciding this cannot happen.

3. The cursor-driven re-find walks the taskbar's UIA subtree 4×/second, indefinitely, for every user who has no hidden icons

if (nearTaskbar) {
    pBtn = FindOverflowButton(pAuto, s, !loggedCandidates, &didLog);
    nextRefind = now + REFIND_NEAR_MS;   // 250 ms

nearTaskbar is the whole Shell_TrayWnd rectangle plus 32 px — on a bottom taskbar that is an ~80 px band across the full width of the screen, i.e. wherever the pointer is while the user works with taskbar buttons. When there are no hidden tray icons the chevron does not exist at all, so pBtn stays nullptr and FindOverflowButton runs every 250 ms for as long as the pointer is in that band, forever. Each run is a cross-process FindAll(TreeScope_Subtree, ControlType == Button) over the taskbar, which forces explorer to build/serve XAML automation peers for the whole subtree on its UI thread — the most latency-sensitive place to add 4 Hz of work, and precisely while the user is interacting with the taskbar.

The previous round's finding was that a fixed 3 s timer left auto-hide users idle after a reveal; the fix over-corrected into an unbounded fast retry. Bounding it keeps both properties: count consecutive failed lookups and back off (250 ms → 500 → 1000 → …, capped at a few seconds), resetting the counter when the chevron is found and on the transition of the cursor into the band, so an auto-hide reveal still re-acquires on the first or second try while an absent chevron costs almost nothing.

Complementary and cheap: on the fast path, query with CreateAndCondition(ControlType == Button, ClassName == chevronClass) and FindFirstBuildCache, which returns one element instead of the full button list, and fall back to the current full walk (which you need for the name fallback and the candidate dump) only on a slow cadence.

Optional improvements

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

  • WindhawkUtils::StringSetting instead of the manual get/free pairs. LoadSettings (lines 940-963) is correct as written, but the RAII wrapper drops the four Wh_FreeStringSetting calls: auto fc = WindhawkUtils::StringSetting::make(L"flyoutClass"); if (*fc) s.flyoutClass = fc;, and StringSetting::make(L"keywords[%d]", i) in the loop. Requires #include <windhawk_utils.h>.
  • Gate the periodic rectangle refresh on cursor proximity too. Once the chevron is found, get_CurrentBoundingRectangle + get_CurrentIsOffscreen run every 750 ms forever (lines 760-773) — two cross-process UIA calls a second, in perpetuity, even when the pointer is on the other side of the screen where the result cannot matter. Refreshing only when the cursor is in the taskbar band (or lazily, on the first tick after it enters) would make the idle cost zero.
  • The name-fallback log line isn't streak-gated. Wh_Log(L"Chevron matched by name, not by class name") (line 484) fires on every acquisition, unlike the candidate dump and the ambiguity line. On a build where the class match never works and the taskbar auto-hides, that is one line per reveal; gating it on logCandidates would make it consistent with its neighbours.
  • pad is not clamped while pollInterval, hoverDelay and grace are (lines 934-936). A large value makes a large part of the screen a hover hotspot and a negative one silently shrinks the hit area below the button; a [0, 64]-ish clamp would match the treatment of the other numeric settings.
  • GetAsyncKeyState is only called inside the haveRect block, so its "pressed since the last call" window is unbounded whenever the chevron is unavailable. The first tick after the chevron is re-acquired can therefore see a press bit left over from an arbitrarily old click and, if the flyout happens to be open under the cursor, latch clickedInFlyout. Calling it unconditionally each tick (or clearing the state along with overBtnPrev when the element is dropped) keeps the window bounded by pollInterval.

Functionality notes

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

  • The 0x0001 bit of GetAsyncKeyState is documented as unreliable: "another application can call GetAsyncKeyState and receive the 'recently pressed' bit instead of your application" (docs). So the new click detection is best-effort — on a machine where something else polls the same keys, a click can still be missed at large polling intervals. It degrades to the previous behaviour rather than misfiring, and IsPopupMenuOpen backstops the important case (once finding 1 is fixed), so this is FYI rather than a defect.
  • clickedInFlyout stays latched until the flyout closes. The only reset is if (!flyoutVisible && !cooling) (line 883). If a click inside the flyout leaves the flyout open — left-clicking an icon whose app just raises a window without dismissing the flyout, for instance — auto-collapse never re-arms for that session, and the flyout stays open until the user clicks it away manually. Re-arming after the cursor has been outside both the flyout and any menu for, say, a few multiples of the collapse delay would recover the behaviour without reintroducing the bug this guard fixes.
  • besideChevron requires a non-negative gap, so a tooltip that overlaps the chevron by even one pixel matches neither gapAbove >= 0 nor gapBelow >= 0 and is not hidden. If any build renders the tooltip flush against or slightly over the button, tooltip hiding quietly does nothing there; allowing a small negative gap would cover it.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 29, 2026
- Enumerate every #32768 window instead of testing only the first one:
  menu windows are kept alive hidden after use, so the first in Z-order is
  often a leftover and the guard could report no menu while one was on
  screen, which is exactly the case it was added for
- Back off after consecutive failed chevron lookups, resetting on success
  and when the cursor enters the taskbar band: the previous round's
  cursor-driven re-find retried every 250 ms indefinitely for anyone with
  no hidden icons, forcing explorer to build automation peers on its UI
  thread while the user works with the taskbar
- Refresh the cached rectangle only while the cursor is at the taskbar,
  since elsewhere the result cannot change any decision
- Sample the mouse buttons every tick rather than only when the chevron is
  available, so the "pressed since the previous call" bit cannot report an
  arbitrarily old click as fresh
- Clamp the hit area padding, log the name fallback once per streak

Not adopted: the suggested height cap on tooltip candidates, meant to
protect the chevron's context menu. Verified on this build that the
chevron has no context menu, and the cap made tooltip hiding miss.
@wygodad

wygodad commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 29, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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.


Round four's menu-detection and re-find-backoff items are correctly fixed — IsPopupMenuOpen now enumerates instead of sampling the first hit, and the lookup backs off geometrically with a 4 s cap. The height cap on tooltip candidates was declined with a measurement, which is fair; an alternative guard that doesn't have its downside is in the optional section rather than here. The tool-mod boilerplate still diffs byte-for-byte clean against the wiki snippet, and the settings block and the code remain in sync in both directions. Both findings below are about calls the worker makes into other processes.

1. ShowWindow on explorer's tooltip window is a blocking cross-thread call, and the unload wait is INFINITE

if (overlapsX && (overlapsFlyout || besideChevron)) {
    ShowWindow(h, SW_HIDE);
}

h belongs to explorer.exe, not to the mod's process. Showing or hiding a window owned by another thread's input queue is synchronous: the call marshals into the owning thread and blocks until that thread processes it. ShowWindowAsync exists for exactly this case — its whole documented purpose is "sets the show state of a window created by a different thread" without waiting for the operation to complete — and SWP_ASYNCWINDOWPOS is documented in the same terms ("this prevents the calling thread from blocking its execution while other threads process the request").

So while explorer's UI thread is busy — which is not rare, and is exactly when a stray tooltip is most likely to be sitting on screen — the worker stalls inside ShowWindow for as long as explorer takes. That has two consequences:

  • The poll loop stops for that whole time, so hover detection, auto-collapse and the stop event are all delayed.
  • WhTool_ModUninit waits WaitForSingleObject(g_thread, INFINITE), and the comment there justifies the missing timeout with "the cross-process UIA calls the worker can be inside have their own timeouts". That reasoning covers the UIA calls but not this one — ShowWindow has no timeout, so a hung explorer makes the unload wait unbounded and the mod's process hangs on disable/update.

One-word fix:

ShowWindowAsync(h, SW_HIDE);

(or SetWindowPos(h, nullptr, 0, 0, 0, 0, SWP_HIDEWINDOW | SWP_NOMOVE | SWP_NOSIZE | SWP_NOZORDER | SWP_NOACTIVATE | SWP_ASYNCWINDOWPOS), as in taskbar-volume-control.wh.cpp#L1311-L1313; windows-animations.wh.cpp#L1584-L1587 switches the flag on explicitly when the target window is foreign). Hiding a tooltip a frame later is indistinguishable to the user. Worth adjusting the unload comment too, since it currently asserts a bound the code no longer has.

2. Sampling GetAsyncKeyState on every tick takes the shared "recently pressed" bit away from every other application

SHORT kl = GetAsyncKeyState(VK_LBUTTON);
SHORT kr = GetAsyncKeyState(VK_RBUTTON);
SHORT km = GetAsyncKeyState(VK_MBUTTON);

Moving these out of the haveRect block does fix the stale-bit problem from last round, but it also makes the mod poll the three mouse buttons unconditionally, 20×/s by default, for the entire session. The 0x0001 bit is not per-process state: it is consumed by whoever reads it first, which is what the documentation warns about from the other side — "another application can call GetAsyncKeyState and receive the 'recently pressed' bit instead of your application". A continuous poller at this rate is that other application for everything else on the desktop: any program that uses the low bit for a mouse button will essentially never see it while this mod runs. Nothing should rely on that bit, so the blast radius is small, but it is a system-wide side effect of a mod whose feature is confined to one taskbar button.

The bit has exactly one consumer here — clickedInFlyout, which is only read inside if (s.autoClose && flyoutVisible && ...). Gating the sampling on that keeps the property you added it for (while the flyout is open it is still sampled every tick, so staleness stays bounded by pollInterval) and reduces the interference to the seconds a flyout is actually open:

// `flyoutBelievedOpen` still holds last tick's value here.
bool anyBtnDown = false, pressedSinceTick = false;
if (s.autoClose && flyoutBelievedOpen) {
    SHORT kl = GetAsyncKeyState(VK_LBUTTON);
    ...
}

Note this also stops the polling entirely for users who turn auto-collapse off, where the result is never used at all. If you want to be strict about the one sample that can still be stale — the first tick after the flyout becomes visible — discard pressedSinceTick on that transition; it can't latch anyway, since the cursor is on the chevron and the flyout doesn't cover it.

Optional improvements

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

  • The name-fallback log gate doesn't actually take effect. Wh_Log(L"Chevron matched by name, not by class name") is now wrapped in if (logCandidates), but logCandidates is !loggedCandidates, and loggedCandidates is reset to false on every successful find (loggedCandidates = pBtn ? false : ...) and is only ever set from the not-identified dump. So on a build that always matches by name, the line still fires on every acquisition — the one-per-streak behaviour you were after needs a separate flag that survives a successful find. Same for the positional-guess line, which isn't gated at all.
  • The band-entry reset can defeat the new backoff. if (nearTaskbar && !nearTaskbarPrev) { refindFailures = 0; nextRefind = 0; } makes every crossing of the 32 px boundary a free immediate walk, with no floor — a cursor moving in and out of that band repeatedly gets one full FindAllBuildCache per crossing, which for a user with no hidden icons is the case the backoff was added for. Keeping a lastWalkAt and requiring now - lastWalkAt >= REFIND_NEAR_MS regardless of the reset bounds it at 4 Hz worst case while keeping the fast re-acquire after an auto-hide reveal.
  • leftAt isn't cleared on the settings-change path. The rect-drop path clears it (with the "don't collapse on the first tick back" comment), but the settings block clears haveRect, overBtnPrev, insideSince, dwellFired and nextRefind and leaves leftAt — so if settings are saved while the flyout is open, the first tick after the chevron is re-acquired can collapse immediately instead of waiting out grace. Same one-line fix as the other path.
  • TASKBAR_REVEAL_PAD (32) is smaller than the maximum pad (64). With a large hit-area padding the cursor can be inside cachedRect ± pad while nearTaskbar is false, so neither the re-find nor the rectangle refresh runs at a position the mod does treat as "on the chevron". PtInRectPad(tb, pt, max(TASKBAR_REVEAL_PAD, s.pad)) keeps the two thresholds consistent.
  • A tooltip guard that isn't height-based. Since the height cap made hiding miss, an alternative that doesn't depend on the popup's size: a tooltip appears from hover alone, a menu only ever follows a click. You already track pressedSinceTick/anyBtnDown — recording the time of the last press and skipping the sweep for ~1 s afterwards would leave normal tooltip hiding untouched while making it impossible for the mod to hide any popup that a click produced, on any build, whatever tooltipClass is set to. Worth considering given the setting is user-editable and the class is explorer's generic popup host.
  • Duplicated comment. Lines 879-880 have both the old and the new first line of the tooltip comment (// Suppress the chevron's tooltip for as long as the cursor is on it: followed by // Suppress the chevron's tooltip while the cursor is on it, both).
  • WindhawkUtils::StringSetting instead of the four manual get/free pairs in LoadSettingsauto fc = WindhawkUtils::StringSetting::make(L"flyoutClass"); if (*fc) s.flyoutClass = fc;, and StringSetting::make(L"keywords[%d]", i) in the loop. Requires #include <windhawk_utils.h>. Same behaviour, four fewer free calls to keep paired.

Functionality notes

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

  • clickedInFlyout still stays latched until the flyout closes (if (!flyoutVisible && !cooling) clickedInFlyout = false;). A click inside the flyout that leaves the flyout open — middle-click, a click on empty space in it, an icon that only toggles something — disables auto-collapse for the rest of that flyout session. Light dismiss means the flyout does eventually close on its own, so it self-heals, but auto-collapse silently doesn't work for that opening. Re-arming once the cursor has been outside both the flyout and any menu for a few multiples of the collapse delay would recover it without reintroducing the bug the guard fixes.
  • IsPopupMenuOpen is desktop-wide. Any visible #32768 window anywhere — a menu in an unrelated application, or one left visible off-screen — suspends auto-collapse for as long as it is up. That is the safe direction, and PID filtering isn't an option since tray menus belong to the icon's owner process, but requiring the menu to intersect the flyout or at least the taskbar's monitor would narrow it if it ever comes up in a report.
  • The rectangle refresh is now gated on nearTaskbar, so a cached rectangle can be up to RECT_REFRESH_MS stale on the first tick after the cursor returns to the taskbar. In practice the gap is almost always longer than 750 ms so the refresh fires on that first tick, but if a user returns to the taskbar within that window while it is auto-hidden, the mod can act on the old rectangle once. Harmless as far as I can tell — noting it because it is the same class of thing the rect guard was added for.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 29, 2026
- Hide the tooltip with ShowWindowAsync: the window belongs to explorer, so
  the synchronous form marshals into its UI thread and blocks until that
  thread handles it, which stalls the poll loop behind a busy shell and
  leaves the unload wait without a bound
- Sample the mouse buttons only while auto-collapse is watching an open
  flyout: the "pressed since the previous call" bit is desktop-wide state
  that the first caller consumes, so polling it for the whole session took
  it away from every other application, while it is only ever read to decide
  whether a click landed inside an open flyout
- Give the "identified, but not by class name" lines their own flag, which
  survives a successful find, so a build that always matches by name logs
  that once rather than on every acquisition
- Floor the lookup cadence so repeatedly crossing the taskbar band cannot
  earn one full walk per crossing and bypass the backoff
- Clear the collapse timer on the settings-change path too, so saving
  settings while the flyout is open cannot collapse it on the next tick
- Widen the taskbar band to at least the hit-area padding, so there is no
  position treated as on the chevron but not as at the taskbar
- Remove a duplicated comment line

Not adopted: skipping the tooltip sweep for a second after any click. It
would require polling the mouse state continuously, which is exactly what
the second finding asks to stop doing.
@wygodad

wygodad commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 29, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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.


Both round-five items are fixed as described — the tooltip is hidden with ShowWindowAsync, so the unload comment's "the wait is bounded" claim holds again, and the mouse sampling is now gated on s.autoClose && flyoutBelievedOpen. All five optional items landed too (the weak-match log flag, the lastWalkAt floor, leftAt on the settings path, revealPad, the duplicated comment). The tool-mod boilerplate still diffs byte-for-byte clean against the wiki snippet, the settings block and the code are in sync in both directions, and the lifecycle (single CreateThread, stop event, join in WhTool_ModUninit, no extra module reference) is correct. One finding, and it is a consequence of the new gate.

flyoutBelievedOpen is never cleared when the chevron is dropped, so the new mouse-sampling gate can latch on for the rest of the session

The gate added this round reads a flag that is only ever recomputed inside the if (haveRect) block:

bool anyBtnDown = false;
bool pressedSinceTick = false;
if (s.autoClose && flyoutBelievedOpen) {   // evaluated every tick, unconditionally
    SHORT kl = GetAsyncKeyState(VK_LBUTTON);
    ...
}
...
if (haveRect) {
    ...
    flyoutBelievedOpen = flyoutVisible || cooling;   // the only assignment

Both paths that drop the element reset a batch of state and leave flyoutBelievedOpen alone — the rect guard:

} else {
    pBtn->Release(); pBtn = nullptr;
    haveRect = false;
    overBtnPrev = false;
    insideSince = 0;
    dwellFired = false;
    leftAt = 0;
    nextRefind = 0;
    WaitForSingleObject(g_stopEvent, s.pollInterval);
    continue;
}

and the settings-change block, which clears haveRect, overBtnPrev, insideSince, dwellFired, leftAt, nextRefind and both log flags.

So if the element is dropped on a tick where the flyout was open, flyoutBelievedOpen stays true with nothing left to correct it, and GetAsyncKeyState(VK_LBUTTON/RBUTTON/MBUTTON) is polled at pollInterval — 20×/s by default — until the chevron is re-acquired. If it never is, that is for the life of the session. That reinstates exactly the desktop-wide side effect the gate was added to remove: the 0x0001 "recently pressed" bit is consumed by whoever reads it first, so while the mod is in this state no other application can observe it.

The window is narrow — flyout visibility is re-checked every tick while flyoutBelievedOpen is set, so it normally goes false as soon as the flyout closes — but it is reachable: the drop requires nearTaskbar, and if the re-find then fails (the last hidden icon was removed through the flyout, so the chevron no longer exists) the backoff walks out to 4 s and stops entirely once the cursor leaves the band, leaving the flag latched with no path back.

One line in each reset, next to the existing ones:

haveRect = false;
overBtnPrev = false;
insideSince = 0;
dwellFired = false;
leftAt = 0;
flyoutBelievedOpen = false;   // nothing recomputes this without haveRect
clickedInFlyout = false;      // same block; harmless but also stale here
nextRefind = 0;

clickedInFlyout has the same gap and self-heals via if (!flyoutVisible && !cooling) once the chevron is back, so it is only worth clearing for consistency — flyoutBelievedOpen is the one that has no recovery path.

Optional improvements

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

  • The ambiguous-signature line is still gated on the wrong flag. Wh_Log(L"%d tray elements share the chevron signature, falling back to name") uses logCandidates, but logCandidates is !loggedCandidates and loggedCandidates is reset to false on every successful find (loggedCandidates = pBtn ? false : ...). It is also never set by this branch — only the not-identified dump sets *logged. So on a build where several tray elements share the signature and the name fallback then succeeds, the line prints on every acquisition, which is the same problem loggedWeakMatch was introduced to solve for the other two lines. It belongs on that flag: if (classMatches > 1 && logWeakMatch) { *logWeakMatch = true; Wh_Log(...); }.
  • anyBtnDownPrev = anyBtnDown; is skipped by the rect-drop continue. The assignment sits at the bottom of the loop, past the continue in the rect guard, so on that tick the freshly sampled value is discarded and anyBtnDownPrev keeps a two-ticks-old value — enough to produce a spurious anyBtnDown && !anyBtnDownPrev edge on the next tick. Assigning it right after the sampling block instead of at the bottom removes the hole and also makes the else { anyBtnDownPrev = false; } branch unnecessary.
  • The positional fallback ignores the alternate AutomationId. if (cands[i].automationId != s.trayIconAutomationId) continue; excludes candidates that were admitted as tray elements via CHEVRON_AUTOMATION_ID_ALT, so on a build that uses ChevronButton the opt-in guess has nothing to pick from. Matching the isTrayElement test (!= s.trayIconAutomationId && != CHEVRON_AUTOMATION_ID_ALT) makes the two consistent.
  • WindhawkUtils::StringSetting instead of the four manual get/free pairs in LoadSettingsauto fc = WindhawkUtils::StringSetting::make(L"flyoutClass"); if (*fc) s.flyoutClass = fc;, and StringSetting::make(L"keywords[%d]", i) in the loop. Requires #include <windhawk_utils.h>. Repeating this from last round only because it was left in place without a note; same behaviour either way.

Functionality notes

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

  • On a build where flyoutClass is wrong, "Hide the chevron tooltip" can hide the flyout itself. The sweep runs whenever s.hideTooltip && overBtn, and when GetVisibleFlyout returns nothing, flyout is nullptr, so h == flyout never excludes anything and only besideChevron applies. The overflow flyout is an explorer-owned popup sitting directly adjacent to the chevron and horizontally overlapping it — i.e. it satisfies the test — so if a future build's flyout is a Xaml_WindowedPopupClass window (the exact situation the README already tells users to fix via the "Flyout window class" setting), enabling tooltip hiding would hide the flyout on every tick and the mod would look completely broken. Since the size-based guard was measured and rejected, a README line under the tooltip note — "if you had to change 'Flyout window class', change it before enabling this" — would cover it without touching the matching logic.
  • Fullscreen suppression spends the hover attempt. dwellFired = true; is set before IsFullscreenOverChevron is consulted, so a stay that was suppressed is marked served: if the user leaves fullscreen while the cursor is still resting on the chevron, nothing opens until the cursor leaves the hit area and comes back. Moving the flag into the branch that actually invokes would fix that at the cost of one IsFullscreenOverChevron call per tick while the cursor sits on the chevron during fullscreen — probably not worth it, noting it as a deliberate trade-off rather than a bug.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 29, 2026
- Clear flyoutBelievedOpen and clickedInFlyout on both paths that drop the
  element. Only the haveRect block assigns them, while the mouse sampling
  gate added last round reads flyoutBelievedOpen every tick, so dropping the
  chevron while the flyout was open latched the flag with no way back and
  resumed polling the shared key state for the rest of the session
- Put the ambiguous-signature line on the flag that survives a successful
  find, like the other two weak-match lines
- Record the previous mouse state right after sampling instead of at the end
  of the loop, which a `continue` can skip, leaving a two-ticks-old value
  that could produce a press edge that never happened
- Let the positional guess consider the alternate chevron AutomationId, so
  it matches the test that admitted those candidates in the first place
@wygodad

wygodad commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 30, 2026
@windhawk-reviewer

Copy link
Copy Markdown

This pull request has already had 3 AI reviews in the last 24 hours, which is the limit, so no review was posted this time.

Comment /ai-review again after 2026-08-30 20:11 UTC (in 10 hours) to get another one.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-author The author's turn: request an AI review, or respond to one that was posted.

Projects

None yet

1 participant