[Run] Fix dark mode detection code, plus refactor

[daverayment]

Summary of the Pull Request

Fixes an issue with Run's dark mode detection, which contained an int cast of a registry key value without checking its type, and lacked a graceful fallback if the cast or any surrounding code (such as the registry value retrieval call) fails.

Also includes refactor of ThemeExtensions and ThemeManager, plus additions to support unit testing the theme code.

PR Checklist

• Closes: Strange "problem" occured during startup. Cannot cast 'System.String' to 'System.Int32' #37275
• Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
• Tests: Added/updated and all pass
• Localization: All end user facing strings can be localized
• Dev docs: Added/updated
• New binaries: Added on the required places
  • JSON for signing for new binaries
  • WXS for installer for new binaries and localization folder
  • YML for CI pipeline for new test projects
  • YML for signed pipeline
• Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

IsSystemDarkMode()

Prior to this fix, the code for detecting Dark mode looked like this:

private static bool IsSystemDarkMode()
{
    const string registryKey = @"HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Themes\Personalize";
    const string registryValue = "AppsUseLightTheme";

    // Retrieve the registry value, which is a DWORD (0 or 1)
    object registryValueObj = Registry.GetValue(registryKey, registryValue, null);
    if (registryValueObj != null)
    {
        // 0 = Dark mode, 1 = Light mode
        bool isLightMode = Convert.ToBoolean((int)registryValueObj, CultureInfo.InvariantCulture);
        return !isLightMode; // Invert because 0 = Dark
    }
    else
    {
        // Default to Light theme if the registry key is missing
        return false; // Default to dark mode assumption
    }
}

The following line is flawed:

bool isLightMode = Convert.ToBoolean((int)registryValueObj, CultureInfo.InvariantCulture);

Specifically, the int cast on registryValueObj can fail if the object is not convertible to an int.

The boolean conversion is unnecessary, too, as the value can be compared to 0, which represents the Dark theme.

The method is called by GetCurrentTheme to determine what app theme is active, so there is actually no need to return a bool; we can simply return the theme directly. With this in mind, we can change the method to:

    internal Theme GetAppsTheme()
    {
        try
        {
            // A 0 for this registry value represents Dark mode. If the value could not be
            // retrieved or is the wrong type, we default to Light mode.
            var regValue = _registryService.GetValue(PersonalizeKey, "AppsUseLightTheme", 1);
            return regValue is int intValue && intValue == 0 ? Theme.Dark : Theme.Light;
        }
        catch
        {
            return Theme.Light;
        }
    }

The method is renamed GetAppsTheme, which reflects its updated purpose. (Also the method was never about checking the System theme, but the Apps theme.) This now uses a pattern-matching is instead of Convert to better handle nulls and incompatible types. A defensive try/catch has been added because Registry.GetValue() may theoretically throw. The method is internal and registry access is done through a new service class because of the new unit testing additions, which are discussed later. Finally, the const registry key string is now declared earlier in the file, together with its counterpart for GetHighContrastBaseType.

GetHighContrastBaseType()

Before this PR, the method looked like this:

    public static Theme GetHighContrastBaseType()
    {
        const string registryKey = @"HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Themes";
        const string registryValue = "CurrentTheme";

        string themePath = (string)Registry.GetValue(registryKey, registryValue, string.Empty);
        if (string.IsNullOrEmpty(themePath))
        {
            return Theme.Light; // Default to light theme if missing
        }

        string theme = themePath.Split('\\').Last().Split('.').First().ToLowerInvariant();

        return theme switch
        {
            "hc1" => Theme.HighContrastOne,
            "hc2" => Theme.HighContrastTwo,
            "hcwhite" => Theme.HighContrastWhite,
            "hcblack" => Theme.HighContrastBlack,
            _ => Theme.Light,
        };
    }

There is a complicating factor here in that Theme.Light is being used as a return value to represent the case when no matching High Contrast theme was found. This makes GetCurrentTheme difficult to decipher, as Theme.Light is used as both a guard/default value and as an actual valid value for the apps theme. To fix this, I changed the method to return null if no match was found.

I also replaced

themePath.Split('\\').Last().Split('.').First().ToLowerInvariant();

with

Path.GetFileNameWithoutExtension(themePath)

because the values do represent actual file paths.

The switch code was replaced with a case-insensitive culture-invariant lookup into a Dictionary, and the method was renamed GetHighContrastTheme. The BaseType suffix from before didn't seem to reflect the return type.

GetCurrentTheme()

The change to return null from GetHighContrastTheme and the actual theme value from GetAppsTheme means we can change GetCurrentTheme from:

    public static Theme GetCurrentTheme()
    {
        // Check for high-contrast mode
        Theme highContrastTheme = GetHighContrastBaseType();
        if (highContrastTheme != Theme.Light)
        {
            return highContrastTheme;
        }

        // Check if the system is using dark or light mode
        return IsSystemDarkMode() ? Theme.Dark : Theme.Light;
    }

to

    public Theme GetCurrentTheme() => GetHighContrastTheme() ?? GetAppsTheme();

This more accurately represents what the method is doing - it looks for an active High Contrast theme, and if no match is found, it returns the preferred Dark/Light theme for apps.

ThemeManager Updates

UpdateTheme could be simplified by taking into account the fact that GetCurrentTheme already does a check for the High Contrast theme. It can be changed from:

        public void UpdateTheme()
        {
            ManagedCommon.Theme newTheme = _settings.Theme;
            ManagedCommon.Theme theme = ThemeExtensions.GetHighContrastBaseType();
            if (theme != ManagedCommon.Theme.Light)
            {
                newTheme = theme;
            }
            else if (_settings.Theme == ManagedCommon.Theme.System)
            {
                newTheme = ThemeExtensions.GetCurrentTheme();
            }

            _mainWindow.Dispatcher.Invoke(() =>
            {
                SetSystemTheme(newTheme);
            });
        }

to:

        public void UpdateTheme()
        {
            Theme newTheme = _settings.Theme == Theme.System ?
                _themeHelper.GetCurrentTheme() :
                _settings.Theme;

            _mainWindow.Dispatcher.Invoke(() =>
            {
                SetSystemTheme(newTheme);
            });
        }

Some minor refactoring to ThemeManager.cs, including:

• Removed ManagedCommon prefix from many places. There was already a using for this namespace, making them redundant.
• Changed a couple of instances of pattern matching against Theme values to be straight equality comparisons, which is more idiomatic. For example:

Color = theme is ManagedCommon.Theme.Dark ? (Color)ColorConverter.ConvertFromString("#202020") : (Color)ColorConverter.ConvertFromString("#fafafa")

was changed to:

Color = (Color)ColorConverter.ConvertFromString(theme == Theme.Dark ? "#202020" : "#fafafa")

• Made CurrentTheme an auto property, as it doesn't need a backing field.
• Removed test code and comment (ResourceDictionary test = new ResourceDictionary...).
• Removed the Theme.Light and Theme.Dark mappings in the switch, as this is the else block after they are specifically checked for in the if, so they will never match.
• Changed the High Contrast switch's default value to Theme.Light so issues are more visible should a new HC mode be added but not correctly matched.
• Added a reference to ThemeHelper, as it's no longer a static class because of the updates for unit testing. Speaking of unit tests...

Unit Tests

There were no tests for the theme-related methods, so this PR:

• Adds a new IRegistryService interface and default RegistryService class which replicates the existing Win32 Registry access.
• Adds a RegistryServiceFactory which creates a new RegistryService instance for use by ThemeHelper.
• Adds a constructor to ThemeHelper which takes an IRegistryService instance or uses the factory to create the default.
• Mocks the Registry for the unit tests, which are new and can be found in Wox.Test.ThemeHelperTest.cs.

Finallly

• Renamed ThemeExtensions.cs to ThemeHelper.cs. The file does not contain extension methods.
• Added XML comments to all public and internal methods updated in this PR.

Validation Steps Performed

New unit tests which exercise ThemeHelper methods directly.

Manual tests

The application responds correctly when:

• The theme is changed between Dark and Light modes in Windows' Personalisation settings.
• A High Contrast theme is selected through Accessibility > Contrast themes.
• The user overrides the system or app settings through the PowerToys Settings app.

[crutkas]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[crutkas]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[mantaionut]

There is an issue with this change. In HC mode we load different background colors.
So if in Launcher settings you set either dark or light mode instead of System then in HC mode the colors will look differently.
You can test this by running in any of the 4 HC mode in Windows 11 and then in launcher settings change the App Theme.

[daverayment]

Thank you for the review, @mantaionut

I can see that I need to immediately return the Contrast theme if one is active and matches one of the 4 defaults. I will update the code and add to the unit tests to cover the various cases.

However, even if I do that, I am not seeing the Light background applied if I change the setting to "Light" from "Windows default". I've checked both my new local build and an installed version of 0.88. It looks like there has been a change as part of the move to .NET 9, because 0.86 still works. I will raise this as a separate issue.

[daverayment]

Here's the separate Contrast theme issue: #37398