Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Windows 10 title bar borders #36429

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

pingzing
Copy link

@pingzing pingzing commented Dec 18, 2024

Summary of the Pull Request

The top border of windows in Windows 10 will no longer always be black, instead of the appropriate color. Specifically, it fixes the following windows:

  • Settings Window
  • OOBE Window
  • HOSTS editor window
  • Advanced Paste window
  • Environment Variables editor window
  • File Locksmith window
  • Peek window
  • Registry Preview window

PR Checklist

Detailed Description of the Pull Request / Additional comments

This adds a bit of extra code to make any window that's using WinUI and ExtendsContentIntoTitleBar also call DwmExtendFrameIntoClientArea() on that window. This does nothing on Windows 11, but on Windows 10, resolves the "top 1px border is black" bug, with one caveat.

That caveat is visible in my changes to the Peek module: I changed it from using AppWindow.TitleBar.ExtendsContentIntoTitleBar to just using the ExtendsContentIntoTitleBar property directly on the MUX.Window. This is because, for some reason, using the AppWindow.TitleBar's version of the property makes DwmExtendFrameIntoClientArea() suddenly do nothing. No clue why.
This is not ideal, because it's my understanding that AppWindow.TitleBar is The Future(tm), and the old ExtendsContentIntoTitleBar is probably going to be deprecated soon.

Other thoughts:

  • ForceTopBorder1PixelInset() is kind of a stupid name. Open to suggestions.
  • Should we do an OS check inside ForceTopBorder1PixelInset() so it's a no-op on W11?

Validation Steps Performed

Opened each of:

  • Settings Window
  • OOBE Window
  • HOSTS editor window
  • Advanced Paste window
  • Environment Variables editor window
  • File Locksmith window
  • Peek window
  • Registry Preview window

...on Windows 10 and verified that the top borders all showed correctly.

I also validated (just for the Settings Window and HOSTS editor window) that the appearance on Windows 11 was unchanged.

@@ -82,6 +82,7 @@ double GetHeight(int maxCustomActionCount) =>
};

WindowHelpers.BringToForeground(this.GetWindowHandle());
WindowHelpers.ForceTopBorder1PixelInset(this.GetWindowHandle());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs for DwmExtendFrameIntoClientArea() say that:

This function must be called whenever Desktop Window Manager (DWM) composition is toggled. Handle the WM_DWMCOMPOSITIONCHANGED message for composition change notification.

...however, the docs for WM_DWMCOMPOSITIONCHANGED go on to note that:

As of Windows 8, DWM composition is always enabled, so this message is not sent regardless of video mode changes.

...so I don't think we need to handle this in Window.Activated--just one call at creation time seems to be enough.

@@ -241,6 +241,10 @@ private void OnLaunchedFromRunner(string[] cmdArgs)
// https://github.com/microsoft/microsoft-ui-xaml/issues/7595 - Activate doesn't bring window to the foreground
// Need to call SetForegroundWindow to actually gain focus.
WindowHelpers.BringToForeground(settingsWindow.GetWindowHandle());

// https://github.com/microsoft/microsoft-ui-xaml/issues/8948 - A window's top border incorrectly
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this note be inline at call sites, or on the function itself?

@pingzing pingzing marked this pull request as ready for review December 19, 2024 22:52
@crutkas crutkas added the Needs-Review This Pull Request awaits the review of a maintainer. label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Review This Pull Request awaits the review of a maintainer.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants