From 1d84931472842b38261b95af20960ec4eff2792a Mon Sep 17 00:00:00 2001
From: Matthew Kosarek <matt.kosarek@canonical.com>
Date: Thu, 9 May 2024 16:32:47 -0400
Subject: [PATCH] (#3309) selecting a correct next window after the focused one
 is closed or disabled

---
 src/miral/application_selector.cpp | 31 +++++++++++++++---------------
 src/miral/application_selector.h   |  2 ++
 src/miral/basic_window_manager.cpp |  6 +++++-
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/miral/application_selector.cpp b/src/miral/application_selector.cpp
index a0e4221a33c..bfedc85d54e 100644
--- a/src/miral/application_selector.cpp
+++ b/src/miral/application_selector.cpp
@@ -93,28 +93,26 @@ void ApplicationSelector::advise_delete_window(WindowInfo const& window_info)
         return;
     }
 
-    // If we delete the selected window, we will try to select the next available one
-    auto original_it = it;
-    if (*it == selected)
+    if (window_info.window() == selected)
     {
-        do {
-            it++;
-            if (it == focus_list.end())
-                it = focus_list.begin();
+        // If we delete the selected window, let's select the next window according
+        // to our last traversal scheme.
+        auto next_selected = next(is_last_traversal_within_app);
 
-            if (it == original_it)
-                break;
-        } while (!tools.can_select_window(*it));
+        // If we select the same window again, let's try and select with the other
+        // traversal scheme
+        if (next_selected == window_info.window())
+            next_selected = next(!is_last_traversal_within_app);
 
-        if (it != original_it)
-            selected = *it;
-        else
+        // We can complete here, since this is the window that we decided to select
+        complete();
+
+        // If it still doesn't work, then we have nothing else to select
+        if (next_selected == window_info.window())
             selected = Window();
     }
-    else
-        selected = Window();
 
-    focus_list.erase(original_it);
+    focus_list.erase(it);
 }
 
 auto ApplicationSelector::next(bool within_app) -> Window
@@ -169,6 +167,7 @@ auto ApplicationSelector::get_focused() -> Window
 
 auto ApplicationSelector::advance(bool reverse, bool within_app) -> Window
 {
+    is_last_traversal_within_app = within_app;
     if (focus_list.empty())
     {
         return {};
diff --git a/src/miral/application_selector.h b/src/miral/application_selector.h
index e1d47adae57..0258adb4152 100644
--- a/src/miral/application_selector.h
+++ b/src/miral/application_selector.h
@@ -92,6 +92,8 @@ class ApplicationSelector
     Window selected;
 
     bool is_active_ = false;
+
+    bool is_last_traversal_within_app = false;
 };
 
 }
diff --git a/src/miral/basic_window_manager.cpp b/src/miral/basic_window_manager.cpp
index 1897084be3b..ef2e4c258c3 100644
--- a/src/miral/basic_window_manager.cpp
+++ b/src/miral/basic_window_manager.cpp
@@ -231,7 +231,6 @@ void miral::BasicWindowManager::remove_surface(
 
 void miral::BasicWindowManager::remove_window(Application const& application, miral::WindowInfo const& info)
 {
-    bool const is_active_window{active_window() == info.window()};
     auto const workspaces_containing_window = workspaces_containing(info.window());
 
     {
@@ -247,6 +246,11 @@ void miral::BasicWindowManager::remove_window(Application const& application, mi
 
     policy->advise_delete_window(info);
 
+    // Warning: order is important here. The compositor may decide to
+    // select a different window in advise_delete_window in response
+    // to the selected window closing. Hence, is_active_window will
+    // only be true if they haven't elected to select a new window.
+    bool const is_active_window{active_window() == info.window()};
     info_for(application).remove_window(info.window());
     mru_active_windows.erase(info.window());
     fullscreen_surfaces.erase(info.window());