WindowServer: Make Menus the input window when showing them

This solves a problem where windows don't receive a WindowInputLeft
event when popup menus are opened. This prevented ComboBox being
closed when right clicking the application on the task bar.

Services/WindowServer/Menu.cpp

@@ -284,12 +284,12 @@ MenuItem* Menu::hovered_item() const
     return const_cast<MenuItem*>(&item(m_hovered_item_index));
 }
 
-void Menu::update_for_new_hovered_item()
+void Menu::update_for_new_hovered_item(bool make_input)
 {
     if (hovered_item() && hovered_item()->is_submenu()) {
         ASSERT(menu_window());
         MenuManager::the().close_everyone_not_in_lineage(*hovered_item()->submenu());
-        hovered_item()->submenu()->popup(hovered_item()->rect().top_right().translated(menu_window()->rect().location()));
+        hovered_item()->submenu()->do_popup(hovered_item()->rect().top_right().translated(menu_window()->rect().location()), make_input);
     } else {
         MenuManager::the().close_everyone_not_in_lineage(*this);
         ensure_menu_window().set_visible(true);
@@ -313,7 +313,7 @@ void Menu::descend_into_submenu_at_hovered_item()
     ASSERT(hovered_item());
     auto submenu = hovered_item()->submenu();
     ASSERT(submenu);
-    MenuManager::the().open_menu(*submenu);
+    MenuManager::the().open_menu(*submenu, false);
     submenu->set_hovered_item(0);
     ASSERT(submenu->hovered_item()->type() != MenuItem::Separator);
 }
@@ -384,7 +384,7 @@ void Menu::event(Core::Event& event)
         // Default to the first item on key press if one has not been selected yet
         if (!hovered_item()) {
             m_hovered_item_index = 0;
-            update_for_new_hovered_item();
+            update_for_new_hovered_item(key == Key_Right);
             return;
         }
 
@@ -508,6 +508,11 @@ void Menu::redraw_if_theme_changed()
 }
 
 void Menu::popup(const Gfx::IntPoint& position)
+{
+    do_popup(position, true);
+}
+
+void Menu::do_popup(const Gfx::IntPoint& position, bool make_input)
 {
     if (is_empty()) {
         dbg() << "Menu: Empty menu popup";
@@ -532,7 +537,7 @@ void Menu::popup(const Gfx::IntPoint& position)
 
     window.move_to(adjusted_pos);
     window.set_visible(true);
-    MenuManager::the().open_menu(*this, false);
+    MenuManager::the().open_menu(*this, make_input);
     WindowManager::the().did_popup_a_menu({});
 }
 

Services/WindowServer/Menu.h

@@ -120,6 +120,7 @@ public:
     void close();
 
     void popup(const Gfx::IntPoint&);
+    void do_popup(const Gfx::IntPoint&, bool);
 
     bool is_menu_ancestor_of(const Menu&) const;
 
@@ -142,7 +143,7 @@ private:
     int item_index_at(const Gfx::IntPoint&);
     int padding_between_text_and_shortcut() const { return 50; }
     void did_activate(MenuItem&);
-    void update_for_new_hovered_item();
+    void update_for_new_hovered_item(bool make_input = false);
 
     ClientConnection* m_client { nullptr };
     int m_menu_id { 0 };

Services/WindowServer/MenuManager.cpp

@@ -311,7 +311,7 @@ void MenuManager::close_everyone()
     }
     m_open_menu_stack.clear();
     m_current_search.clear();
-    m_current_menu = nullptr;
+    clear_current_menu();
     refresh();
 }
 
@@ -332,7 +332,7 @@ void MenuManager::close_menus(const Vector<Menu*>& menus)
 {
     for (auto& menu : menus) {
         if (menu == m_current_menu)
-            m_current_menu = nullptr;
+            clear_current_menu();
         if (menu->menu_window())
             menu->menu_window()->set_visible(false);
         menu->clear_hovered_item();
@@ -373,8 +373,11 @@ void MenuManager::toggle_menu(Menu& menu)
 void MenuManager::open_menu(Menu& menu, bool as_current_menu)
 {
     if (is_open(menu)) {
-        if (as_current_menu)
+        if (as_current_menu || current_menu() != &menu) {
+            // This menu is already open. If requested, or if the current
+            // window doesn't match this one, then set it to this
             set_current_menu(&menu);
+        }
         return;
     }
 
@@ -390,16 +393,30 @@ void MenuManager::open_menu(Menu& menu, bool as_current_menu)
     if (m_open_menu_stack.find([&menu](auto& other) { return &menu == other.ptr(); }).is_end())
         m_open_menu_stack.append(menu.make_weak_ptr());
 
-    if (as_current_menu)
+    if (as_current_menu || !current_menu()) {
+        // Only make this menu the current menu if requested, or if no
+        // other menu is current
         set_current_menu(&menu);
+    }
 
     refresh();
 }
 
+void MenuManager::clear_current_menu()
+{
+    Menu* previous_current_menu = m_current_menu;
+    m_current_menu = nullptr;
+    if (previous_current_menu) {
+        // When closing the last menu, restore the previous active input window
+        auto& wm = WindowManager::the();
+        wm.restore_active_input_window(m_previous_input_window);
+    }
+}
+
 void MenuManager::set_current_menu(Menu* menu)
 {
     if (!menu) {
-        m_current_menu = nullptr;
+        clear_current_menu();
         return;
     }
 
@@ -409,7 +426,17 @@ void MenuManager::set_current_menu(Menu* menu)
     }
 
     m_current_search.clear();
+
+    Menu* previous_current_menu = m_current_menu;
     m_current_menu = menu->make_weak_ptr();
+
+    auto& wm = WindowManager::the();
+    if (!previous_current_menu) {
+        // When opening the first menu, store the current active input window
+        m_previous_input_window = wm.active_input_window()->make_weak_ptr();
+    }
+
+    wm.set_active_input_window(m_current_menu->menu_window());
 }
 
 void MenuManager::close_bar()

Services/WindowServer/MenuManager.h

@@ -56,6 +56,7 @@ public:
 
     Menu* current_menu() { return m_current_menu.ptr(); }
     void set_current_menu(Menu*);
+    void clear_current_menu();
     void open_menu(Menu&, bool as_current_menu = true);
     void toggle_menu(Menu&);
 
@@ -104,6 +105,7 @@ private:
     RefPtr<Window> m_window;
 
     WeakPtr<Menu> m_current_menu;
+    WeakPtr<Window> m_previous_input_window;
     Vector<WeakPtr<Menu>> m_open_menu_stack;
 
     RefPtr<Core::Timer> m_search_timer;

Services/WindowServer/Window.cpp

@@ -590,6 +590,18 @@ void Window::set_parent_window(Window& parent_window)
         parent_window.add_child_window(*this);
 }
 
+bool Window::is_accessory() const
+{
+    if (!m_accessory)
+        return false;
+    if (parent_window() != nullptr)
+        return true;
+    
+    // If accessory window was unparented, convert to a regular window
+    const_cast<Window*>(this)->set_accessory(false);
+    return false;
+}
+
 bool Window::is_accessory_of(Window& window) const
 {
     if (!is_accessory())

Services/WindowServer/Window.h

@@ -244,7 +244,7 @@ public:
     const Vector<WeakPtr<Window>>& accessory_windows() const { return m_accessory_windows; }
 
     void set_accessory(bool accessory) { m_accessory = accessory; }
-    bool is_accessory() const { return m_accessory; }
+    bool is_accessory() const;
     bool is_accessory_of(Window&) const;
 
     void set_frameless(bool frameless) { m_frameless = frameless; }

Services/WindowServer/WindowManager.cpp

@@ -210,17 +210,13 @@ void WindowManager::move_to_front_and_make_active(Window& window)
 
     bool make_active = true;
     if (window.is_accessory()) {
-        if (auto* parent = window.parent_window()) {
-            do_move_to_front(*parent, true, false);
-            make_active = false;
+        auto* parent = window.parent_window();
+        do_move_to_front(*parent, true, false);
+        make_active = false;
             
-            for (auto& accessory_window : parent->accessory_windows()) {
-                if (accessory_window && accessory_window.ptr() != &window)
-                    do_move_to_front(*accessory_window, false, false);
-            }
-        } else {
-            // If accessory window was unparented, convert to a regular window
-            window.set_accessory(false);
+        for (auto& accessory_window : parent->accessory_windows()) {
+            if (accessory_window && accessory_window.ptr() != &window)
+                do_move_to_front(*accessory_window, false, false);
         }
     }
 
@@ -258,7 +254,7 @@ void WindowManager::remove_window(Window& window)
     window.invalidate();
     m_windows_in_order.remove(&window);
     if (window.is_active())
-        pick_new_active_window(window);
+        pick_new_active_window(&window);
     if (m_switcher.is_visible() && window.type() != WindowType::WindowSwitcher)
         m_switcher.refresh();
 
@@ -376,7 +372,7 @@ void WindowManager::notify_minimization_state_changed(Window& window)
         window.client()->post_message(Messages::WindowClient::WindowStateChanged(window.window_id(), window.is_minimized(), window.is_occluded()));
 
     if (window.is_active() && window.is_minimized())
-        pick_new_active_window(window);
+        pick_new_active_window(&window);
 }
 
 void WindowManager::notify_occlusion_state_changed(Window& window)
@@ -390,18 +386,24 @@ void WindowManager::notify_progress_changed(Window& window)
     tell_wm_listeners_window_state_changed(window);
 }
 
-void WindowManager::pick_new_active_window(Window& previous_active)
+bool WindowManager::pick_new_active_window(Window* previous_active)
 {
     bool new_window_picked = false;
+    Window* first_candidate = nullptr;
     for_each_visible_window_of_type_from_front_to_back(WindowType::Normal, [&](Window& candidate) {
-        if (!candidate.is_accessory_of(previous_active)) {
+        first_candidate = &candidate;
+        if ((!previous_active && !candidate.is_accessory()) || (previous_active && !candidate.is_accessory_of(*previous_active))) {
             set_active_window(&candidate);
             new_window_picked = true;
+            return IterationDecision::Break;
         }
-        return IterationDecision::Break;
+        return IterationDecision::Continue;
     });
-    if (!new_window_picked)
-        set_active_window(nullptr);
+    if (!new_window_picked) {
+        set_active_window(first_candidate);
+        new_window_picked = first_candidate != nullptr;
+    }
+    return new_window_picked;
 }
 
 void WindowManager::start_window_move(Window& window, const MouseEvent& event)
@@ -1095,14 +1097,7 @@ bool WindowManager::is_active_window_or_accessory(Window& window) const
     if (!window.is_accessory())
         return false;
 
-    auto* parent = window.parent_window();
-    if (!parent) {
-        // If accessory window was unparented, convert to a regular window
-        window.set_accessory(false);
-        return false;
-    }
-
-    return m_active_window == parent;
+    return m_active_window == window.parent_window();
 }
 
 static bool window_type_can_become_active(WindowType type)
@@ -1110,6 +1105,38 @@ static bool window_type_can_become_active(WindowType type)
     return type == WindowType::Normal || type == WindowType::Desktop;
 }
 
+void WindowManager::restore_active_input_window(Window* window)
+{
+    // If the previous active input window is gone, fall back to the
+    // current active window
+    if (!window)
+        window = active_window();
+    // If the current active window is also gone, pick some other window
+    if (!window && pick_new_active_window(nullptr))
+        return;
+    
+    set_active_input_window(window);
+}
+
+Window* WindowManager::set_active_input_window(Window* window)
+{
+    if (window == m_active_input_window)
+        return window;
+
+    Window* previous_input_window = m_active_input_window;
+    if (previous_input_window)
+        Core::EventLoop::current().post_event(*previous_input_window, make<Event>(Event::WindowInputLeft));
+
+    if (window) {
+        m_active_input_window = window->make_weak_ptr();
+        Core::EventLoop::current().post_event(*window, make<Event>(Event::WindowInputEntered));
+    } else {
+        m_active_input_window = nullptr;
+    }
+
+    return previous_input_window;
+}
+
 void WindowManager::set_active_window(Window* window, bool make_input)
 {
     if (window && window->is_blocked_by_modal_window())
@@ -1118,32 +1145,16 @@ void WindowManager::set_active_window(Window* window, bool make_input)
     if (window && !window_type_can_become_active(window->type()))
         return;
 
-    if (make_input) {
-        auto* new_active_input_window = window;
-        if (window && window->is_accessory()) {
-            if (auto* parent = window->parent_window()) {
-                // The parent of an accessory window is always the active
-                // window, but input is routed to the accessory window
-                window = parent;
-            } else {
-                // If accessory window was unparented, convert to a regular window
-                window->set_accessory(false);
-            }
-        }
-
-        if (new_active_input_window != m_active_input_window) {
-            if (m_active_input_window)
-                Core::EventLoop::current().post_event(*m_active_input_window, make<Event>(Event::WindowInputLeft));
-
-            if (new_active_input_window) {
-                m_active_input_window = new_active_input_window->make_weak_ptr();
-                Core::EventLoop::current().post_event(*new_active_input_window, make<Event>(Event::WindowInputEntered));
-            } else {
-                m_active_input_window = nullptr;
-            }
-        }
+    auto* new_active_input_window = window;
+    if (window && window->is_accessory()) {
+        // The parent of an accessory window is always the active
+        // window, but input is routed to the accessory window
+        window = window->parent_window();
     }
 
+    if (make_input)
+        set_active_input_window(new_active_input_window);
+
     if (window == m_active_window)
         return;
 

Services/WindowServer/WindowManager.h

@@ -107,7 +107,10 @@ public:
     void start_dnd_drag(ClientConnection&, const String& text, Gfx::Bitmap*, const String& data_type, const String& data);
     void end_dnd_drag();
 
+    Window* active_window() { return m_active_window.ptr(); }
     const Window* active_window() const { return m_active_window.ptr(); }
+    Window* active_input_window() { return m_active_input_window.ptr(); }
+    const Window* active_input_window() const { return m_active_input_window.ptr(); }
     const ClientConnection* active_client() const;
     bool active_window_is_modal() const { return m_active_window && m_active_window->is_modal(); }
 
@@ -145,6 +148,8 @@ public:
     bool set_resolution(int width, int height);
     Gfx::IntSize resolution() const;
 
+    Window* set_active_input_window(Window*);
+    void restore_active_input_window(Window*);
     void set_active_window(Window*, bool make_input = true);
     void set_hovered_button(Button*);
 
@@ -207,7 +212,7 @@ private:
     void tell_wm_listener_about_window(Window& listener, Window&);
     void tell_wm_listener_about_window_icon(Window& listener, Window&);
     void tell_wm_listener_about_window_rect(Window& listener, Window&);
-    void pick_new_active_window(Window&);
+    bool pick_new_active_window(Window*);
 
     void do_move_to_front(Window&, bool, bool);