Skip to content

Add mod to fix checkbox background in conflict dialog#4717

Merged
m417z merged 2 commits into
ramensoftware:mainfrom
Anixx:patch-597257
Jul 10, 2026
Merged

Add mod to fix checkbox background in conflict dialog#4717
m417z merged 2 commits into
ramensoftware:mainfrom
Anixx:patch-597257

Conversation

@Anixx

@Anixx Anixx commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

This mod fixes the black background issue on the 'Apply to all conflicts' checkbox in the file conflict dialog when using the Windows Classic theme.

Changelog

If this pull request updates an existing mod, describe the changes below:

  • Changelog item 1...
  • Changelog item 2...

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.

This mod fixes the black background issue on the 'Apply to all conflicts' checkbox in the file conflict dialog when using the Windows Classic theme.
@m417z

m417z commented Jul 8, 2026

Copy link
Copy Markdown
Member

Submission review

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

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


1. Don't swap the window procedure directly — use WindhawkUtils::SetWindowSubclassFromAnyThread. The mod replaces CtrlNotifySink's GWLP_WNDPROC (line 132) and restores it manually in Wh_ModUninit (line 163). This breaks the subclass chain: if another mod (or the shell) subclasses the same window after you, restoring the saved OrigParentProc un-chains their proc too, and if you unload while a subclass sits on top of yours, you'll set the window proc back to a stale value. Windhawk provides WindhawkUtils::SetWindowSubclassFromAnyThread / RemoveWindowSubclassFromAnyThread (add #include <windhawk_utils.h>) precisely so mods coexist safely — see Development-tips. It also removes the manual prop bookkeeping and the WM_NCDESTROY handling entirely (the wrapper removes the subclass on WM_NCDESTROY for you).

One caveat when you switch: RemoveWindowSubclassFromAnyThread is implemented via SendMessage to the window's thread, so calling it inside Wh_ModUninit while holding g_windowsMutex would deadlock (the window's thread may be blocked on the same mutex). Copy the handles out, release the lock, then remove:

std::vector<HWND> hwnds;
{
    std::lock_guard<std::mutex> lock(g_windowsMutex);
    hwnds.swap(g_subclassedWindows);
}
for (HWND hWnd : hwnds)
    WindhawkUtils::RemoveWindowSubclassFromAnyThread(hWnd, ParentSubclassProc);

2. WM_NCDESTROY swallows the original window procedure. In the current manual subclass, the WM_NCDESTROY branch calls RemovePropW(hWnd, L"OrigParentProc") (line 96) before the code below reads it (line 99). So origProc comes back NULL and the message is dispatched to DefWindowProcW instead of the real CtrlNotifySink proc — the original window never gets its own WM_NCDESTROY, which can leak DirectUI-internal state for that sink. If you keep manual subclassing (human note: no reason to do that), read the proc first, then remove:

WNDPROC origProc = (WNDPROC)GetPropW(hWnd, L"OrigParentProc");
if (uMsg == WM_NCDESTROY) {
    { std::lock_guard<std::mutex> lock(g_windowsMutex); /* erase from vector */ }
    RemovePropW(hWnd, L"OrigParentProc");
}
return origProc ? CallWindowProcW(origProc, hWnd, uMsg, wParam, lParam)
                : DefWindowProcW(hWnd, uMsg, wParam, lParam);

Switching to the Windhawk subclass utility (item 1) makes this moot.

Optional improvements

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

  • Type-safe hook: prefer WindhawkUtils::SetFunctionHook(CreateWindowExW, CreateWindowExW_Hook, &pOriginalCreateWindowExW) over raw Wh_SetFunctionHook with void* casts (line 144) — it checks the prototype at compile time.
  • Missing include: std::find (line 92) comes from <algorithm>; include it directly instead of relying on a transitive include.
  • README nits: "recent cumulative uptates" → "updates" (line 19), and the "After" image still uses the ![before] alt text (line 27).

Functionality notes

Non-critical observations about behavior itself.

  • The fix is applied even when a visual theme is active. Nothing gates on the classic theme — the hierarchy match (ButtonCtrlNotifySinkDirectUIHWND#32770) holds regardless of theme, so when a themed user hits the conflict dialog, the subclass forces WM_CTLCOLORSTATIC to return a COLOR_BTNFACE brush and bk-color. On the default (light) themed dialog that paints a gray box behind the checkbox text — a regression for anyone not running Classic. Consider gating the fix on classic theme being active, e.g. skip the override when IsThemeActive() returns TRUE (uxtheme), so it only touches the case it's meant to fix.
  • Dialogs already open when the mod is enabled won't be fixed until they're re-opened, since the mod only reacts to CreateWindowExW. Conflict dialogs are transient, so this is low-impact, but worth noting.

@Anixx

Anixx commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Fixed issues.

@m417z m417z merged commit 53e4ca9 into ramensoftware:main Jul 10, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants