fix: replace MB_SYSTEMMODAL with MB_SETFOREGROUND in MessageDialog

[majiayu000]

Fixes #3684

Problem

MessageDialog on Windows doesn't show the parent window's icon in the dialog title bar. The MB_SYSTEMMODAL flag creates a system-level modal that doesn't associate with the owner window, so the icon is not inherited.

Fix

Replace MB_SYSTEMMODAL with MB_SETFOREGROUND in dialog.go. MB_SETFOREGROUND keeps the dialog prominent in the foreground while allowing it to inherit the owner window's icon. The parent HWND is already correctly passed via getHandleForDialog(), so application-modal behavior is preserved.

Test Plan

• Verified cross-compilation with GOOS=windows go build
• Existing calculateMessageDialogFlags() tests unaffected (that function doesn't touch this flag)
• Manual verification on Windows needed: build a Wails app with a custom icon, trigger MessageDialog, verify the dialog title bar shows the app icon

Summary by CodeRabbit

• Bug Fixes

  • Fixed MessageDialog appearance and focus behavior on Windows so dialogs reliably show and receive focus.
  • Improved error reporting when displaying dialogs so failures are surfaced more clearly.

• Documentation

  • Updated changelog with the Windows dialog fix.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0cb72ddf-55d0-4f43-9722-6c76d43641b9

📥 Commits

Reviewing files that changed from the base of the PR and between c349a3e and 07f87dd.

📒 Files selected for processing (1)

• v2/internal/frontend/desktop/windows/dialog.go

🚧 Files skipped from review as they are similar to previous changes (1)

• v2/internal/frontend/desktop/windows/dialog.go

---

📝 Walkthrough

Walkthrough

This change replaces MB_SYSTEMMODAL with MB_SETFOREGROUND in the Windows MessageDialog call and adds error handling; a changelog entry documents the fix. (No exported signatures changed.)

Changes

Cohort / File(s)	Summary
Windows Dialog Fix v2/internal/frontend/desktop/windows/dialog.go	Replaced MB_SYSTEMMODAL with MB_SETFOREGROUND when calling MessageBox, added error propagation for the call.
Changelog Update website/src/pages/changelog.mdx	Added a "Fixed" changelog entry describing the Windows MessageDialog icon display fix.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Windows

Suggested reviewers

• leaanthony

Poem

🐰 I nudged a flag and tuned the light,
MB_SETFOREGROUND brought icons right,
No more lost crowns in the dialog's glow,
Now parent windows proudly show —
Hooray! 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The PR description provides problem statement, fix explanation, and test plan. However, the description does not fill out the required template checklist items (Type of change, How Has This Been Tested section with platform checkboxes, Test Configuration, and full Checklist completion).	Complete the template by checking relevant Type of change box, documenting Windows testing details, and confirming all checklist items have been verified.
Linked Issues check	❓ Inconclusive	The PR addresses the core requirement from issue #3684 (fixing icon display in MessageDialog on Windows) by replacing MB_SYSTEMMODAL with MB_SETFOREGROUND and adds error handling. However, it does not address the MessageBoxIndirectW approach previously suggested in the issue.	Provide evidence that the proposed fix actually resolves the icon display issue with before/after screenshots, and clarify why the MessageBoxIndirectW approach was not pursued.
Out of Scope Changes check	❓ Inconclusive	The changelog entry documents the MB_SYSTEMMODAL replacement fix. However, the reviewer noted this involves a behavioral change (system-modal to app-modal) that should be explicitly documented, and the commit message mentions error handling for MessageBox return values.	Clarify whether the error handling addition for MessageBox return values is in scope for this fix, and document the modal behavior change in the changelog.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the primary change in the PR: replacing MB_SYSTEMMODAL with MB_SETFOREGROUND in MessageDialog on Windows.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.16.4)

v2/internal/frontend/desktop/windows/dialog.go

┌──────────────┐
│ Opengrep CLI │
└──────────────┘

�[32m✔�[39m �[1mOpengrep OSS�[0m
�[32m✔�[39m Basic security coverage for first-party code vulnerabilities.

�[1m Loading rules from local config...�[0m

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[leaanthony]

Thanks for digging into this — it's been open for a while and the root cause analysis in your issue comment is solid.

I have a few questions before this can move out of draft:

Does MB_SETFOREGROUND actually fix the icon?

MB_SETFOREGROUND is a focus/z-order flag — it asks Windows to bring the dialog to the foreground when it appears. I'm not aware of it having any effect on icon inheritance from the parent window. The parent HWND is already being passed correctly, so if MB_SYSTEMMODAL was the only thing preventing icon inheritance, simply removing it (without replacing it with anything) should also work. Can you share a before/after screenshot on Windows confirming the app icon appears in the dialog title bar with this change?

Is the modal behaviour change intentional?

MB_SYSTEMMODAL creates a system-level modal that blocks all top-level windows across the system, not just the calling app. Swapping it for MB_SETFOREGROUND changes that to app-modal behaviour, which is arguably more correct — but it's a meaningful behavioural change that should be called out explicitly in the changelog entry.

What happened to the MessageBoxIndirectW approach?

In your comment on #3684 you outlined a more complete fix: use MessageBoxIndirectW with a MSGBOXPARAMSW struct, set hIcon via GetClassLongPtrW(hwnd, GCLP_HICON), and use MB_USERICON. That directly sets the icon handle on the dialog and would be a guaranteed fix for the root cause. Did you try that and find it unnecessary, or was the flag swap sufficient on its own?

Happy to merge something simple if it works — just need Windows verification first. Could you mark ready for review once you've confirmed the icon renders correctly?

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@v2/internal/frontend/desktop/windows/dialog.go`:
- Line 194: The call to windows.MessageBox currently discards its error return;
modify the call in dialog.go to capture both returns (e.g., button, err :=
windows.MessageBox(...)), check if err != nil and return or wrap that error from
the enclosing function (the function that calls getHandleForDialog and
MessageBox), otherwise continue returning the button result; update any callers
or return types if necessary so failures from windows.MessageBox are propagated
instead of masked.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 230657c7-3beb-4e0d-9f61-2322fc72e6ba

📥 Commits

Reviewing files that changed from the base of the PR and between 63725f1 and c349a3e.

📒 Files selected for processing (2)

• v2/internal/frontend/desktop/windows/dialog.go
• website/src/pages/changelog.mdx

[majiayu000]

Closing this PR — after reviewing the feedback, I agree that MB_SETFOREGROUND is a focus/z-order flag and likely doesn't address the icon inheritance issue. The proper fix would require MessageBoxIndirectW with explicit hIcon handling, which is a different approach. I don't have a Windows environment to verify the current change, so it's best not to leave this open unvalidated. Thanks for the thorough review!