[FancyZones]Modern apps snapping fix, refactor and tests (#29499)

* popup windows check * use minimize maximize buttons check * update test utils * added tests * define types for easier testing * changed checks order * remove option check * upd test * remove FZ popup option * max min buttons -> caption * calculator test * updated excluded tests * add asserts to child window test * update window creation * splash screen refactor * remove hotfix part * replace style checking functions * remove no longer necessary check * tool window check fix * fix mouse snapping check * added check and test for non-root window * spelling * Revert "remove FZ popup option" This reverts commit 26732ad683. * skip child window tests in CI * remove the option * remove the constant * updated tests
2026-04-10 13:35:31 +02:00 · 2023-11-21 21:41:55 +01:00
parent 1a8007ca0a
commit f9cbff6976
21 changed files with 335 additions and 185 deletions
--- a/src/modules/fancyzones/FancyZonesTests/UnitTests/UnitTests.vcxproj
+++ b/src/modules/fancyzones/FancyZonesTests/UnitTests/UnitTests.vcxproj
@@ -56,6 +56,7 @@
    <ClCompile Include="Util.Spec.cpp" />
    <ClCompile Include="Util.cpp" />
    <ClCompile Include="WindowKeyboardSnap.Spec.cpp" />
+    <ClCompile Include="WindowProcessingTests.Spec.cpp" />
    <ClCompile Include="WorkArea.Spec.cpp" />
    <ClCompile Include="WorkAreaIdTests.Spec.cpp" />
    <ClCompile Include="Zone.Spec.cpp" />
--- a/src/modules/fancyzones/FancyZonesTests/UnitTests/UnitTests.vcxproj.filters
+++ b/src/modules/fancyzones/FancyZonesTests/UnitTests/UnitTests.vcxproj.filters
@@ -66,6 +66,9 @@
    <ClCompile Include="WindowKeyboardSnap.Spec.cpp">
      <Filter>Source Files</Filter>
    </ClCompile>
+    <ClCompile Include="WindowProcessingTests.Spec.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
  </ItemGroup>
  <ItemGroup>
    <ClInclude Include="pch.h">
--- a/src/modules/fancyzones/FancyZonesTests/UnitTests/Util.cpp
+++ b/src/modules/fancyzones/FancyZonesTests/UnitTests/Util.cpp
@@ -8,8 +8,7 @@ namespace Mocks
    class HwndCreator
    {
    public:
-        HwndCreator(const std::wstring& title = L"");
-
+        HwndCreator(const std::wstring& title, const std::wstring& className, DWORD exStyle, DWORD style, HWND parentWindow);
        ~HwndCreator();

        HWND operator()(HINSTANCE hInst);
@@ -20,23 +19,30 @@ namespace Mocks
        inline HINSTANCE getHInstance() const { return m_hInst; }
        inline const std::wstring& getTitle() const { return m_windowTitle; }
        inline const std::wstring& getWindowClassName() const { return m_windowClassName; }
+        inline DWORD getExStyle() const noexcept { return m_exStyle; }
+        inline DWORD getStyle() const noexcept { return m_style; }
+        inline HWND getParentWindow() const noexcept { return m_parentWindow; }

    private:
        std::wstring m_windowTitle;
        std::wstring m_windowClassName;
+        DWORD m_exStyle{ 0 };
+        DWORD m_style{ 0 };
+        HWND m_parentWindow{ NULL };

        std::mutex m_mutex;
        std::condition_variable m_conditionVar;
        bool m_conditionFlag;
        HANDLE m_thread;

-        HINSTANCE m_hInst;
+        HINSTANCE m_hInst{};
        HWND m_hWnd;
    };

-    HWND WindowCreate(HINSTANCE hInst)
+    HWND WindowCreate(HINSTANCE hInst, const std::wstring& title /*= L""*/, const std::wstring& className /*= L""*/, 
+        DWORD exStyle, DWORD style, HWND parentWindow)
    {
-        return HwndCreator()(hInst);
+        return HwndCreator(title, className, exStyle, style, parentWindow)(hInst);
    }
 }

@@ -85,8 +91,21 @@ DWORD WINAPI ThreadProc(LPVOID lpParam)

    if (RegisterDLLWindowClass(creator->getWindowClassName().c_str(), creator) != 0)
    {
-        auto hWnd = CreateWindowEx(0, creator->getWindowClassName().c_str(), creator->getTitle().c_str(), WS_EX_APPWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, 10, 10, nullptr, nullptr, creator->getHInstance(), NULL);
-        SetWindowPos(hWnd, HWND_TOPMOST, 10, 10, 100, 100, SWP_SHOWWINDOW);
+        auto hWnd = CreateWindowEx(creator->getExStyle(), 
+            creator->getWindowClassName().c_str(), 
+            creator->getTitle().c_str(), 
+            creator->getStyle(), 
+            CW_USEDEFAULT, CW_USEDEFAULT, 
+            CW_USEDEFAULT, CW_USEDEFAULT, 
+            creator->getParentWindow(),
+            nullptr, 
+            creator->getHInstance(), 
+            NULL);
+
+        ShowWindow(hWnd, SW_SHOW);
+        // wait after ShowWindow to make sure that it's finished and shown
+        std::this_thread::sleep_for(std::chrono::milliseconds(100)); 
+
        creator->setHwnd(hWnd);
        creator->setCondition(true);

@@ -95,8 +114,6 @@ DWORD WINAPI ThreadProc(LPVOID lpParam)
            TranslateMessage(&messages);
            DispatchMessage(&messages);
        }
-
-        creator->setHwnd(hWnd);
    }
    else
    {
@@ -108,8 +125,16 @@ DWORD WINAPI ThreadProc(LPVOID lpParam)

 namespace Mocks
 {
-    HwndCreator::HwndCreator(const std::wstring& title) :
-        m_windowTitle(title), m_windowClassName(std::to_wstring(++s_classId)), m_conditionFlag(false), m_thread(nullptr), m_hInst(HINSTANCE{}), m_hWnd(nullptr)
+    HwndCreator::HwndCreator(const std::wstring& title, const std::wstring& className, DWORD exStyle, DWORD style, HWND parentWindow) :
+        m_windowTitle(title), 
+        m_windowClassName(className.empty() ? std::to_wstring(++s_classId) : className), 
+        m_exStyle(exStyle),
+        m_style(style),
+        m_parentWindow(parentWindow),
+        m_conditionFlag(false), 
+        m_thread(nullptr), 
+        m_hInst(HINSTANCE{}), 
+        m_hWnd(nullptr)
    {
    }

--- a/src/modules/fancyzones/FancyZonesTests/UnitTests/Util.h
+++ b/src/modules/fancyzones/FancyZonesTests/UnitTests/Util.h
@@ -123,7 +123,8 @@ namespace Mocks
        return reinterpret_cast<HINSTANCE>(++s_nextInstance);
    }

-    HWND WindowCreate(HINSTANCE hInst);
+    HWND WindowCreate(HINSTANCE hInst, const std::wstring& title = L"", const std::wstring& className = L""
+        , DWORD exStyle = 0, DWORD style = 0, HWND parentWindow = nullptr);
 }

 namespace Helpers
--- a/src/modules/fancyzones/FancyZonesTests/UnitTests/WindowProcessingTests.Spec.cpp
+++ b/src/modules/fancyzones/FancyZonesTests/UnitTests/WindowProcessingTests.Spec.cpp
@@ -0,0 +1,194 @@
+#include "pch.h"
+
+#include <FancyZonesLib/FancyZonesWindowProcessing.h>
+#include <FancyZonesLib/Settings.h>
+#include <FancyZonesLib/WindowUtils.h>
+
+#include "Util.h"
+
+#include <CppUnitTestLogger.h>
+
+using namespace Microsoft::VisualStudio::CppUnitTestFramework;
+
+namespace Microsoft
+{
+    namespace VisualStudio
+    {
+        namespace CppUnitTestFramework
+        {
+            template<>
+            std::wstring ToString<FancyZonesWindowProcessing::ProcessabilityType>(const FancyZonesWindowProcessing::ProcessabilityType& type)
+            {
+                return std::to_wstring((int)type);
+            }
+
+        }
+    }
+}
+
+namespace FancyZonesUnitTests
+{
+    TEST_CLASS (WindowProcessingUnitTests)
+    {
+        HINSTANCE hInst{};
+
+        TEST_METHOD_CLEANUP(CleanUp)
+        {
+            FancyZonesSettings::instance().SetSettings(Settings{});
+        }
+
+        TEST_METHOD (MinimizedWindow)
+        {
+            HWND window = Mocks::WindowCreate(hInst);
+            ShowWindow(window, SW_MINIMIZE);
+            std::this_thread::sleep_for(std::chrono::milliseconds(100)); // let ShowWindow finish
+            Assert::IsTrue(IsIconic(window));
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::Minimized, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (ToolWindow)
+        {
+            HWND window = Mocks::WindowCreate(hInst, L"", L"", WS_EX_TOOLWINDOW);
+            
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::ToolWindow, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (InvisibleWindow)
+        {
+            HWND window = Mocks::WindowCreate(hInst);
+            ShowWindow(window, SW_HIDE);
+            std::this_thread::sleep_for(std::chrono::milliseconds(100)); // let ShowWindow finish
+            
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::NotVisible, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD(NonRootWindow)
+        {
+            HWND rootWindow = Mocks::WindowCreate(hInst, L"RootWindow", L"", 0, WS_TILEDWINDOW | WS_CLIPCHILDREN);
+            Assert::IsTrue(FancyZonesWindowUtils::IsRoot(rootWindow));
+
+            HWND window = CreateWindow(WC_COMBOBOX, TEXT(""), CBS_DROPDOWN | CBS_HASSTRINGS | WS_CHILD | WS_OVERLAPPED | WS_VISIBLE, 0, 0, 10, 10, rootWindow, NULL, hInst, NULL);
+            Assert::IsFalse(FancyZonesWindowUtils::IsRoot(window));
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::NonRootWindow, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (Popup_App)
+        {
+            HWND window = Mocks::WindowCreate(hInst, L"", L"", 0, WS_TILEDWINDOW | WS_POPUP);
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::Processable, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsTrue(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (Popup_Menu)
+        {
+            HWND window = Mocks::WindowCreate(hInst, L"", L"", 0, WS_POPUP | WS_TILED | WS_CLIPCHILDREN | WS_CLIPSIBLINGS);
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::NonProcessablePopupWindow, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (Popup_MenuEdge)
+        {
+            HWND window = Mocks::WindowCreate(hInst, L"", L"", 0, WS_POPUP | WS_TILED | WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_THICKFRAME | WS_SIZEBOX);
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::NonProcessablePopupWindow, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (Popup_Calculator)
+        {
+            HWND window = Mocks::WindowCreate(hInst, L"", L"", 0, WS_BORDER | WS_CLIPSIBLINGS | WS_DLGFRAME | WS_GROUP | WS_POPUP | WS_POPUPWINDOW | WS_SIZEBOX | WS_TABSTOP | WS_TILEDWINDOW);
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::Processable, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsTrue(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (Popup_CalculatorTopmost)
+        {
+            HWND window = Mocks::WindowCreate(hInst, L"", L"", 0, WS_BORDER | WS_CAPTION | WS_CLIPSIBLINGS | WS_DLGFRAME | WS_OVERLAPPED | WS_POPUP | WS_POPUPWINDOW | WS_SIZEBOX | WS_SYSMENU | WS_THICKFRAME);
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::Processable, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsTrue(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (ChildWindow_OptionDisabled)
+        {
+            FancyZonesSettings::instance().SetSettings(Settings{ .allowSnapChildWindows = false });
+            HWND parentWindow = Mocks::WindowCreate(hInst, L"", L"", 0, WS_TILEDWINDOW);
+            if (!IsWindowVisible(parentWindow))
+            {
+                // skip the test if the parent window isn't visible.
+                // test can run locally, but will fail in CI because of the configuration
+                return;
+            }
+
+            HWND window = Mocks::WindowCreate(hInst, L"", L"", 0, 0, parentWindow);
+            Assert::IsTrue(IsWindowVisible(window), L"Child window not visible");
+            Assert::IsTrue(FancyZonesWindowUtils::HasVisibleOwner(window), L"Child window doesn't have visible owner");
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::ChildWindow, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (ChildWindow_OptionEnabled)
+        {
+            FancyZonesSettings::instance().SetSettings(Settings{ .allowSnapChildWindows = true });
+            HWND parentWindow = Mocks::WindowCreate(hInst, L"", L"", 0, WS_TILEDWINDOW);
+            if (!IsWindowVisible(parentWindow))
+            {
+                // skip the test if the parent window isn't visible.
+                // test can run locally, but will fail in CI because of the configuration
+                return;
+            }
+
+            HWND window = Mocks::WindowCreate(hInst, L"", L"", 0, 0, parentWindow);
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::Processable, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsTrue(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (ExcludedApp_ByDefault)
+        {
+            // set class from the excluded list
+            HWND window = Mocks::WindowCreate(hInst, L"", L"SysListView32");
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::Excluded, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (ExcludedApp_ByDefault_SplashScreen)
+        {
+            HWND window = Mocks::WindowCreate(hInst, L"", L"MsoSplash");
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::Excluded, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (ExcludedApp_ByUser)
+        {
+            // case sensitive, should be uppercase
+            FancyZonesSettings::instance().SetSettings(Settings{ .excludedAppsArray = { L"TEST_EXCLUDED" } });
+
+            // exclude by window title
+            HWND window = Mocks::WindowCreate(hInst, L"Test_Excluded");
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::Excluded, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsFalse(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+
+        TEST_METHOD (ProcessableWindow)
+        {
+            HWND window = Mocks::WindowCreate(hInst, L"", L"", 0, WS_TILEDWINDOW);
+
+            Assert::AreEqual(FancyZonesWindowProcessing::ProcessabilityType::Processable, FancyZonesWindowProcessing::DefineWindowType(window));
+            Assert::IsTrue(FancyZonesWindowProcessing::IsProcessable(window));
+        }
+    };
+}
--- a/src/modules/fancyzones/FancyZonesTests/UnitTests/WorkArea.Spec.cpp
+++ b/src/modules/fancyzones/FancyZonesTests/UnitTests/WorkArea.Spec.cpp
@@ -249,7 +249,7 @@ namespace FancyZonesUnitTests
            AppliedLayouts::instance().ApplyLayout(m_workAreaId, layout);

            const auto workArea = WorkArea::Create(m_hInst, m_workAreaId, m_parentUniqueId, m_workAreaRect);
-            const auto window = Mocks::WindowCreate(m_hInst);
+            const auto window = Mocks::WindowCreate(m_hInst, L"", L"", 0, WS_THICKFRAME);

            SetWindowPos(window, nullptr, 150, 150, 450, 550, SWP_SHOWWINDOW);