Skip to content

Create classic-taskbar-properties.wh.cpp#4429

Merged
m417z merged 5 commits into
ramensoftware:mainfrom
babamohammed2022:patch-4
Jun 15, 2026
Merged

Create classic-taskbar-properties.wh.cpp#4429
m417z merged 5 commits into
ramensoftware:mainfrom
babamohammed2022:patch-4

Conversation

@babamohammed2022

Copy link
Copy Markdown
Contributor

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): Deepseek
    • 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.

@m417z

m417z commented Jun 15, 2026

Copy link
Copy Markdown
Member

In addition to the notes below, It'd be nice to have a screenshot.

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. ShellExecuteExW_hook matches far too broadly — it hijacks unrelated launches. The interception fires on wcsstr(path, L"taskbar") after lower-casing:

if (wcsstr(path, L"ms-settings:taskbar") || wcsstr(path, L"taskbar")) {

The second clause matches any file, folder, or URL whose path contains the substring "taskbar" anywhere — C:\Users\me\Taskbar screenshots\, TaskbarX.exe, a downloaded taskbar-tweaks.zip, etc. For all of those the mod swallows the call, shows its own dialog, and returns TRUE (success) without performing the requested action — so the real shell operation silently fails. Match the exact URI instead (e.g. _wcsnicmp(path, L"ms-settings:taskbar", 19) == 0, or an exact compare). For reference, settings-to-control-panel.wh.cpp matches ms-settings: URIs via an exact lookup table and also hooks both ShellExecuteExW and ShellExecuteW (the shell doesn't always route through the Ex variant) — both points worth adopting.

2. The SHAppBarMessage hook locks the taskbar edge and serves no working feature. Hook_SHAppBarMessage overwrites the caller's pData->uEdge with g_desiredEdge on every ABM_QUERYPOS/ABM_SETPOS:

DWORD edge = InterlockedCompareExchange(&g_desiredEdge, 0, 0);
if (edge <= 3) pData->uEdge = edge;

But g_desiredEdge is only ever assigned once, in Wh_ModInit, to the current edge, and nothing in the UI ever changes it — IDC_COMBO_LOCATION is populated and shown but never read in ApplySettings, so the "Taskbar location" dropdown is a dead control. The net effect is that, while the mod is loaded, this hook forces the taskbar back to its stored edge on every reposition request — i.e. it can prevent the user from moving the taskbar by dragging, an unintended side effect of a feature that doesn't actually exist. It also mutates a caller-owned struct, which hooks shouldn't do. Since there's no working position-change feature, please remove the SHAppBarMessage hook and g_desiredEdge entirely (and either wire up IDC_COMBO_LOCATION properly or drop it). This looks like leftover scaffolding from an abandoned "change position" attempt.

Optional improvements

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

  • Dead window class. BuildAndShowDialog registers L"TaskbarPropertiesDlg" via RegisterClassExW, but the dialog template uses class 0 (the standard dialog class), so the registered class is never actually used — and it's never UnregisterClassed. It's harmless (its lpfnWndProc is DefDlgProcW, a stable user32 address), but it's dead code; just remove the registration block.
  • Use WindhawkUtils::SetFunctionHook (with #include <windhawk_utils.h>) instead of raw Wh_SetFunctionHook with void* casts — it's type-safe and the house style.
  • wcscpy_safe re-implements a bounded copy by hand; StringCchCopyW (<strsafe.h>) does the same. Trivial.
  • Hardcoded Italian. InitLocalization() switches to Italian when (GetUserDefaultUILanguage() & 0xFF) == LANG_ITALIAN. The English default is good; baking in one specific second language keyed on the UI language is unusual but fine.
  • Fixed new BYTE[10000] / new BYTE[12000] template buffers use magic sizes with no bounds check. The content is bounded today, but a computed size (or a guard) would be safer.

Functionality notes

Non-critical observations about the feature behavior itself.

  • The interception may not trigger on all builds. It only fires when the shell launches ms-settings:taskbar through shell32!ShellExecuteExW. Modern Explorer often launches ms-settings: URIs via ShellExecuteW or a WinRT launcher (Windows.System.Launcher/IApplicationActivationManager), which this hook won't see — so the classic dialog may quietly not appear when the user clicks "Taskbar settings." Worth verifying on your target builds and, at minimum, also hooking ShellExecuteW as settings-to-control-panel.wh.cpp does.
  • Many settings need an Explorer restart / logout to take effect (as the README notes), so unlike the real Settings UI the dialog will often appear to "do nothing" immediately after Apply. That's inherent to writing these particular registry values, just worth keeping in mind for the UX.

@babamohammed2022

Copy link
Copy Markdown
Contributor Author

In addition to the notes below, It'd be nice to have a screenshot.

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. ShellExecuteExW_hook matches far too broadly — it hijacks unrelated launches. The interception fires on wcsstr(path, L"taskbar") after lower-casing:

if (wcsstr(path, L"ms-settings:taskbar") || wcsstr(path, L"taskbar")) {

The second clause matches any file, folder, or URL whose path contains the substring "taskbar" anywhere — C:\Users\me\Taskbar screenshots\, TaskbarX.exe, a downloaded taskbar-tweaks.zip, etc. For all of those the mod swallows the call, shows its own dialog, and returns TRUE (success) without performing the requested action — so the real shell operation silently fails. Match the exact URI instead (e.g. _wcsnicmp(path, L"ms-settings:taskbar", 19) == 0, or an exact compare). For reference, settings-to-control-panel.wh.cpp matches ms-settings: URIs via an exact lookup table and also hooks both ShellExecuteExW and ShellExecuteW (the shell doesn't always route through the Ex variant) — both points worth adopting.

2. The SHAppBarMessage hook locks the taskbar edge and serves no working feature. Hook_SHAppBarMessage overwrites the caller's pData->uEdge with g_desiredEdge on every ABM_QUERYPOS/ABM_SETPOS:

DWORD edge = InterlockedCompareExchange(&g_desiredEdge, 0, 0);
if (edge <= 3) pData->uEdge = edge;

But g_desiredEdge is only ever assigned once, in Wh_ModInit, to the current edge, and nothing in the UI ever changes it — IDC_COMBO_LOCATION is populated and shown but never read in ApplySettings, so the "Taskbar location" dropdown is a dead control. The net effect is that, while the mod is loaded, this hook forces the taskbar back to its stored edge on every reposition request — i.e. it can prevent the user from moving the taskbar by dragging, an unintended side effect of a feature that doesn't actually exist. It also mutates a caller-owned struct, which hooks shouldn't do. Since there's no working position-change feature, please remove the SHAppBarMessage hook and g_desiredEdge entirely (and either wire up IDC_COMBO_LOCATION properly or drop it). This looks like leftover scaffolding from an abandoned "change position" attempt.

Optional improvements

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

  • Dead window class. BuildAndShowDialog registers L"TaskbarPropertiesDlg" via RegisterClassExW, but the dialog template uses class 0 (the standard dialog class), so the registered class is never actually used — and it's never UnregisterClassed. It's harmless (its lpfnWndProc is DefDlgProcW, a stable user32 address), but it's dead code; just remove the registration block.
  • Use WindhawkUtils::SetFunctionHook (with #include <windhawk_utils.h>) instead of raw Wh_SetFunctionHook with void* casts — it's type-safe and the house style.
  • wcscpy_safe re-implements a bounded copy by hand; StringCchCopyW (<strsafe.h>) does the same. Trivial.
  • Hardcoded Italian. InitLocalization() switches to Italian when (GetUserDefaultUILanguage() & 0xFF) == LANG_ITALIAN. The English default is good; baking in one specific second language keyed on the UI language is unusual but fine.
  • Fixed new BYTE[10000] / new BYTE[12000] template buffers use magic sizes with no bounds check. The content is bounded today, but a computed size (or a guard) would be safer.

Functionality notes

Non-critical observations about the feature behavior itself.

  • The interception may not trigger on all builds. It only fires when the shell launches ms-settings:taskbar through shell32!ShellExecuteExW. Modern Explorer often launches ms-settings: URIs via ShellExecuteW or a WinRT launcher (Windows.System.Launcher/IApplicationActivationManager), which this hook won't see — so the classic dialog may quietly not appear when the user clicks "Taskbar settings." Worth verifying on your target builds and, at minimum, also hooking ShellExecuteW as settings-to-control-panel.wh.cpp does.
  • Many settings need an Explorer restart / logout to take effect (as the README notes), so unlike the real Settings UI the dialog will often appear to "do nothing" immediately after Apply. That's inherent to writing these particular registry values, just worth keeping in mind for the UX.

Thank you for the review.
Screenshot of the window:
Screenshot
image
Screenshot of the window from Windows 7 for comparison:
669e2c9b530edc210d31f586_146717 image0
I've tried to fix the code and address all issues by also integrating some parts of the "Settings to Control Panel" mod.

@m417z

m417z commented Jun 15, 2026

Copy link
Copy Markdown
Member

I was suggesting to add a screenshot to the mod's readme.

@babamohammed2022

Copy link
Copy Markdown
Contributor Author

I was suggesting to add a screenshot to the mod's readme.

I've added the screenshot in the readme too. If there are any other issues to address, please let me know

@m417z m417z merged commit 29bacda into ramensoftware:main Jun 15, 2026
3 checks passed
@Anixx

Anixx commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Something is wrong with colors and fonts:

изображение

The font is greater than system default.

@babamohammed2022

Copy link
Copy Markdown
Contributor Author

Something is wrong with colors and fonts:
изображение

The font is greater than system default.

Thank you for reporting this issue. I've made a pull request so that we can discuss about it there #4640. Please try the new version and tell me if the issue persists

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.

3 participants