layout/master: fix rollprev/rollnext focusing the wrong window (#14209)

* layout/master: fix rollprev/rollnext focusing the wrong window

* tests/master: add tests for rollnext/rollprev focus

2 files changed, 81 insertions, 4 deletions

diff --git a/hyprtester/src/tests/main/master.cpp b/hyprtester/src/tests/main/master.cpp
index b9364c45..806cbdf7 100644
--- a/hyprtester/src/tests/main/master.cpp
+++ b/hyprtester/src/tests/main/master.cpp
@@ -1,6 +1,7 @@
 #include "../shared.hpp"
 #include "../../shared.hpp"
 #include "../../hyprctlCompat.hpp"
+#include <algorithm>
 #include "tests.hpp"
 
 TEST_CASE(focusMasterPrevious) {
@@ -141,3 +142,71 @@ TEST_CASE(fsBehavior) {
         EXPECT_CONTAINS(str, "fullscreen: 0");
     }
 }
+
+TEST_CASE(rollFocus) {
+    // test rollnext/rollprev dispatchers
+
+    OK(getFromSocket("r/eval hl.config({ general = { layout = 'master' } })"));
+
+    // set up windows
+    std::vector<std::string> windows = {"slave1", "slave2", "slave3", "master"};
+
+    // helper lambda thing
+    auto roll = [&](const std::string& dir) {
+        auto pivot = (dir == "rollnext") ? windows.begin() + 1 : windows.end() - 1;
+
+        // rotate the windows vector along with the actual windows
+        // the rolling behavior of the window focus should follow the
+        // rotating behavior of std::ranges::rotate
+        OK(getFromSocket("/dispatch hl.dsp.layout('" + dir + "')"));
+        std::ranges::rotate(windows.begin(), pivot, windows.end());
+        ASSERT_CONTAINS(getFromSocket("/activewindow"), "class: " + windows.back());
+    };
+
+    for (auto const& win : windows) {
+        if (!Tests::spawnKitty(win)) {
+            FAIL_TEST("Could not spawn kitty with win class `{}`", win);
+        }
+    }
+
+    // focus master
+    OK(getFromSocket("/dispatch hl.dsp.layout('focusmaster master')"));
+    ASSERT_CONTAINS(getFromSocket("/activewindow"), "class: master");
+
+    // put the windows in the washing machine
+    NLog::log("{}Testing rollnext", Colors::YELLOW);
+    for (int i = 0; i < 20; ++i) {
+        roll("rollnext");
+    }
+
+    NLog::log("{}Testing rollprev", Colors::YELLOW);
+    for (int i = 0; i < 20; ++i) {
+        roll("rollprev");
+    }
+
+    NLog::log("{}Testing rollnext with rollprev", Colors::YELLOW);
+    for (int i = 0; i < 10; ++i) {
+        for (int j = 0; j < 5; ++j) {
+            roll("rollnext");
+        }
+        roll("rollprev");
+    }
+
+    NLog::log("{}Testing rollnext/rollprev alternation", Colors::YELLOW);
+    for (int i = 0; i < 20; ++i) {
+        if (i % 2 == 0) {
+            roll("rollnext");
+        } else {
+            roll("rollprev");
+        }
+    }
+
+    NLog::log("{}Testing rollnext/rollprev burst calls", Colors::YELLOW);
+    for (int i = 0; i < 20; ++i) {
+        if (i / 5 % 2 == 0) {
+            roll("rollnext");
+        } else {
+            roll("rollprev");
+        }
+    }
+}
diff --git a/src/layout/algorithm/tiled/master/MasterAlgorithm.cpp b/src/layout/algorithm/tiled/master/MasterAlgorithm.cpp
index a0ea13c3..c57a3639 100644
--- a/src/layout/algorithm/tiled/master/MasterAlgorithm.cpp
+++ b/src/layout/algorithm/tiled/master/MasterAlgorithm.cpp
@@ -715,12 +715,15 @@ Config::ErrorResult CMasterAlgorithm::layoutMsg(const std::string_view& sv) {
         if (!OLDMASTER)
             return stateErr("no old master");
 
-        auto oldMasterIt = std::ranges::find(m_masterNodesData, OLDMASTER);
+        auto        oldMasterIt = std::ranges::find(m_masterNodesData, OLDMASTER);
+
+        SP<ITarget> newFocus;
 
         for (auto& nd : m_masterNodesData) {
             if (!nd->isMaster) {
                 const auto& newMaster = nd;
                 newMaster->isMaster   = true;
+                newFocus              = newMaster->pTarget.lock();
 
                 auto newMasterIt = std::ranges::find(m_masterNodesData, newMaster);
 
@@ -729,7 +732,6 @@ Config::ErrorResult CMasterAlgorithm::layoutMsg(const std::string_view& sv) {
                 else if (newMasterIt > oldMasterIt)
                     std::ranges::rotate(oldMasterIt, newMasterIt, std::next(newMasterIt));
 
-                switchToWindow(newMaster->pTarget.lock());
                 OLDMASTER->isMaster = false;
 
                 oldMasterIt = std::ranges::find(m_masterNodesData, OLDMASTER);
@@ -741,6 +743,8 @@ Config::ErrorResult CMasterAlgorithm::layoutMsg(const std::string_view& sv) {
         }
 
         calculateWorkspace();
+        if (newFocus)
+            switchToWindow(newFocus);
     } else if (command == "rollprev") {
         const auto PNODE = getNodeFromWindow(PWINDOW);
 
@@ -751,12 +755,15 @@ Config::ErrorResult CMasterAlgorithm::layoutMsg(const std::string_view& sv) {
         if (!OLDMASTER)
             return stateErr("no old master");
 
-        auto oldMasterIt = std::ranges::find(m_masterNodesData, OLDMASTER);
+        auto        oldMasterIt = std::ranges::find(m_masterNodesData, OLDMASTER);
+
+        SP<ITarget> newFocus;
 
         for (auto& nd : m_masterNodesData | std::views::reverse) {
             if (!nd->isMaster) {
                 const auto& newMaster = nd;
                 newMaster->isMaster   = true;
+                newFocus              = newMaster->pTarget.lock();
 
                 auto newMasterIt = std::ranges::find(m_masterNodesData, newMaster);
 
@@ -765,7 +772,6 @@ Config::ErrorResult CMasterAlgorithm::layoutMsg(const std::string_view& sv) {
                 else if (newMasterIt > oldMasterIt)
                     std::ranges::rotate(oldMasterIt, newMasterIt, std::next(newMasterIt));
 
-                switchToWindow(newMaster->pTarget.lock());
                 OLDMASTER->isMaster = false;
 
                 oldMasterIt = std::ranges::find(m_masterNodesData, OLDMASTER);
@@ -777,6 +783,8 @@ Config::ErrorResult CMasterAlgorithm::layoutMsg(const std::string_view& sv) {
         }
 
         calculateWorkspace();
+        if (newFocus)
+            switchToWindow(newFocus);
     } else
         return Config::configError(std::format("Unknown master layoutmsg: {}", sv), Config::eConfigErrorLevel::ERROR, Config::eConfigErrorCode::INVALID_ARGUMENT);