(#3309) selecting a correct next window after the focused one is clos…

…ed or disabled

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 {};

src/miral/application_selector.h

@@ -92,6 +92,8 @@ class ApplicationSelector
     Window selected;
 
     bool is_active_ = false;
+
+    bool is_last_traversal_within_app = false;
 };
 
 }

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());

tests/miral/application_selector.cpp

@@ -151,7 +151,7 @@ TEST_F(ApplicationSelectorTest, deleting_selected_window_makes_the_next_window_s
     application_selector.advise_focus_gained(window_manager_tools.info_for(windows[1]));
     application_selector.advise_delete_window(window_manager_tools.info_for(windows[1]));
     auto window = application_selector.get_focused();
-    EXPECT_EQ(window, windows[0]);
+    EXPECT_EQ(window, windows[1]);
 }
 
 TEST_F(ApplicationSelectorTest, moving_forward_once_within_an_app_when_there_isnt_another_window_results_in_the_currently_selected_window_remaining_the_same)