From 447118ab708276f6fc664b6b8b0c85137481d9b5 Mon Sep 17 00:00:00 2001 From: leileizhang Date: Wed, 27 Aug 2025 19:16:55 +0800 Subject: [PATCH] fix: Context menu registry entries are not cleaned up when disabled via GPO (#41411) ## Summary of the Pull Request This PR fixes an issue where context menu runtime registration wasn't properly cleaned up when GPO (Group Policy Object) policies disabled the module. The problem occurred because the module constructor didn't consider GPO policies when determining its initial enabled state. ## PR Checklist - [x] Closes: #41387 - [ ] **Communication:** I've discussed this with core contributors already. If the 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](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed This pull request refactors how context menu registration and unregistration are handled across several PowerToys modules. The main improvement is the introduction of a new `UpdateRegistration` helper method in each module, which centralizes and simplifies the logic for registering or unregistering context menus when the module is enabled or disabled. This reduces code duplication and ensures consistent behavior. **Context menu registration logic refactor:** * Added a private `UpdateRegistration` method to each of the following modules to handle context menu registration and unregistration based on the enabled state: - `FileLocksmithModule` in `PowerToysModule.cpp` - `NewModule` in `powertoys_module.cpp` - `ImageResizerModule` in `dllmain.cpp` - `PowerRenameModule` in `dllmain.cpp` * Replaced direct calls to registration/unregistration functions in `enable`, `disable`, and `init_settings` methods with calls to the new `UpdateRegistration` method in all affected modules, ensuring consistent and centralized handling [[1]](diffhunk://#diff-256ed936dafec1bf6ff17849b4797dd276f5b07bebe2e483bc1580c8f06e92d9L91-R122) [[2]](diffhunk://#diff-256ed936dafec1bf6ff17849b4797dd276f5b07bebe2e483bc1580c8f06e92d9R155) [[3]](diffhunk://#diff-4a3942d548f3daec02a833983ed9b2b69f75e2cd1b74a8ce1b874f3fd33fde55L101-R125) [[4]](diffhunk://#diff-4a3942d548f3daec02a833983ed9b2b69f75e2cd1b74a8ce1b874f3fd33fde55L153-R177) [[5]](diffhunk://#diff-0c0a89e812ff4625d165417da14f1c3f203e5ac7907555ae4fde122f3dddcf7aL115-L130) [[6]](diffhunk://#diff-34581ec47c37b0d2e1d9b59696225c47342930694e732db06cbdf653ceb2c2d7L205-R234) [[7]](diffhunk://#diff-34581ec47c37b0d2e1d9b59696225c47342930694e732db06cbdf653ceb2c2d7R334). These changes improve maintainability and reduce the risk of inconsistent registration behavior across modules. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../FileLocksmithExt/PowerToysModule.cpp | 30 ++++++++++++---- .../powertoys_module.cpp | 36 ++++++++++++------- src/modules/imageresizer/dll/dllmain.cpp | 31 +++++++++++----- src/modules/powerrename/dll/dllmain.cpp | 29 +++++++++++---- 4 files changed, 91 insertions(+), 35 deletions(-) diff --git a/src/modules/FileLocksmith/FileLocksmithExt/PowerToysModule.cpp b/src/modules/FileLocksmith/FileLocksmithExt/PowerToysModule.cpp index d0ee80b872..5a46049e2f 100644 --- a/src/modules/FileLocksmith/FileLocksmithExt/PowerToysModule.cpp +++ b/src/modules/FileLocksmith/FileLocksmithExt/PowerToysModule.cpp @@ -19,6 +19,26 @@ class FileLocksmithModule : public PowertoyModuleIface { +private: + // Update registration based on enabled state + void UpdateRegistration(bool enabled) + { + if (enabled) + { +#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) + FileLocksmithRuntimeRegistration::EnsureRegistered(); + Logger::info(L"File Locksmith context menu registered"); +#endif + } + else + { +#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) + FileLocksmithRuntimeRegistration::Unregister(); + Logger::info(L"File Locksmith context menu unregistered"); +#endif + } + } + public: FileLocksmithModule() { @@ -88,21 +108,16 @@ public: package::RegisterSparsePackage(path, packageUri); } } -#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) - FileLocksmithRuntimeRegistration::EnsureRegistered(); -#endif m_enabled = true; + UpdateRegistration(m_enabled); } virtual void disable() override { Logger::info(L"File Locksmith disabled"); -#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) - FileLocksmithRuntimeRegistration::Unregister(); - Logger::info(L"File Locksmith context menu unregistered (Win10)"); -#endif m_enabled = false; + UpdateRegistration(m_enabled); } virtual bool is_enabled() override @@ -135,6 +150,7 @@ private: { m_enabled = FileLocksmithSettingsInstance().GetEnabled(); m_extended_only = FileLocksmithSettingsInstance().GetShowInExtendedContextMenu(); + UpdateRegistration(m_enabled); Trace::EnableFileLocksmith(m_enabled); } diff --git a/src/modules/NewPlus/NewShellExtensionContextMenu/powertoys_module.cpp b/src/modules/NewPlus/NewShellExtensionContextMenu/powertoys_module.cpp index ad94431953..b63b755d86 100644 --- a/src/modules/NewPlus/NewShellExtensionContextMenu/powertoys_module.cpp +++ b/src/modules/NewPlus/NewShellExtensionContextMenu/powertoys_module.cpp @@ -21,6 +21,26 @@ // Note: Settings are managed via Settings and UI Settings class NewModule : public PowertoyModuleIface { +private: + // Update registration based on enabled state + void UpdateRegistration(bool enabled) + { + if (enabled) + { +#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) + NewPlusRuntimeRegistration::EnsureRegisteredWin10(); + Logger::info(L"New+ context menu registered"); +#endif + } + else + { +#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) + NewPlusRuntimeRegistration::Unregister(); + Logger::info(L"New+ context menu unregistered"); +#endif + } + } + public: NewModule() { @@ -98,14 +118,9 @@ public: { newplus::utilities::register_msix_package(); } - else - { -#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) - NewPlusRuntimeRegistration::EnsureRegisteredWin10(); -#endif - } powertoy_new_enabled = true; + UpdateRegistration(powertoy_new_enabled); } virtual void disable() override @@ -150,19 +165,14 @@ private: { Trace::EventToggleOnOff(false); } - if (!package::IsWin11OrGreater()) - { -#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) - NewPlusRuntimeRegistration::Unregister(); - Logger::info(L"New+ context menu unregistered (Win10)"); -#endif - } powertoy_new_enabled = false; + UpdateRegistration(powertoy_new_enabled); } void init_settings() { powertoy_new_enabled = NewSettingsInstance().GetEnabled(); + UpdateRegistration(powertoy_new_enabled); } }; diff --git a/src/modules/imageresizer/dll/dllmain.cpp b/src/modules/imageresizer/dll/dllmain.cpp index 697550df9d..c616e77e42 100644 --- a/src/modules/imageresizer/dll/dllmain.cpp +++ b/src/modules/imageresizer/dll/dllmain.cpp @@ -43,11 +43,32 @@ private: //contains the non localized key of the powertoy std::wstring app_key; + // Update registration based on enabled state + void UpdateRegistration(bool enabled) + { + if (enabled) + { +#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) + ImageResizerRuntimeRegistration::EnsureRegistered(); + Logger::info(L"ImageResizer context menu registered"); +#endif + } + else + { +#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) + ImageResizerRuntimeRegistration::Unregister(); + Logger::info(L"ImageResizer context menu unregistered"); +#endif + } + } + + public: // Constructor ImageResizerModule() { m_enabled = CSettingsInstance().GetEnabled(); + UpdateRegistration(m_enabled); app_name = GET_RESOURCE_STRING(IDS_IMAGERESIZER); app_key = ImageResizerConstants::ModuleKey; LoggerHelpers::init_logger(app_key, L"ModuleInterface", LogSettings::imageResizerLoggerName); @@ -112,10 +133,7 @@ public: package::RegisterSparsePackage(path, packageUri); } } -#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) - ImageResizerRuntimeRegistration::EnsureRegistered(); -#endif - + UpdateRegistration(m_enabled); Trace::EnableImageResizer(m_enabled); } @@ -123,11 +141,8 @@ public: virtual void disable() { m_enabled = false; + UpdateRegistration(m_enabled); Trace::EnableImageResizer(m_enabled); -#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) - ImageResizerRuntimeRegistration::Unregister(); - Logger::info(L"ImageResizer context menu unregistered (Win10)"); -#endif } // Returns if the powertoys is enabled diff --git a/src/modules/powerrename/dll/dllmain.cpp b/src/modules/powerrename/dll/dllmain.cpp index a4f906fe66..698797f932 100644 --- a/src/modules/powerrename/dll/dllmain.cpp +++ b/src/modules/powerrename/dll/dllmain.cpp @@ -168,6 +168,25 @@ private: //contains the non localized key of the powertoy std::wstring app_key; + // Update registration based on enabled state + void UpdateRegistration(bool enabled) + { + if (enabled) + { +#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) + PowerRenameRuntimeRegistration::EnsureRegistered(); + Logger::info(L"PowerRename context menu registered"); +#endif + } + else + { +#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) + PowerRenameRuntimeRegistration::Unregister(); + Logger::info(L"PowerRename context menu unregistered"); +#endif + } + } + public: // Return the localized display name of the powertoy virtual PCWSTR get_name() override @@ -202,9 +221,7 @@ public: package::RegisterSparsePackage(path, packageUri); } } -#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) - PowerRenameRuntimeRegistration::EnsureRegistered(); -#endif + UpdateRegistration(m_enabled); } // Disable the powertoy @@ -212,10 +229,7 @@ public: { m_enabled = false; Logger::info(L"PowerRename disabled"); -#if defined(ENABLE_REGISTRATION) || defined(NDEBUG) - PowerRenameRuntimeRegistration::Unregister(); - Logger::info(L"PowerRename context menu unregistered (Win10)"); -#endif + UpdateRegistration(m_enabled); } // Returns if the powertoy is enabled @@ -315,6 +329,7 @@ public: void init_settings() { m_enabled = CSettingsInstance().GetEnabled(); + UpdateRegistration(m_enabled); Trace::EnablePowerRename(m_enabled); }