From 463aff827e4d47afacd6693a031e8e0d868affc8 Mon Sep 17 00:00:00 2001 From: thankyouverycool <66646555+thankyouverycool@users.noreply.github.com> Date: Wed, 7 Sep 2022 07:49:00 -0400 Subject: [PATCH] LibGUI+WindowServer: Notify Windows on input preemption Previously Menus set themselves as active input solely to make sure CaptureInput modals would close, but this is a functional half-truth. Menus don't actually use the active input role; they preempt normal Windows during event handling instead. Now the active input window is notified on preemption and Menus can remain outside the active input concept. This lets us make more granular choices about modal behavior. For now, the only thing clients care about is menu preemption on popup. Fixes windows which close on changes to active input closing on their own context menus. --- Userland/Libraries/LibGUI/ComboBox.cpp | 1 + Userland/Libraries/LibGUI/CommandPalette.cpp | 5 ++++ .../LibGUI/ConnectionToWindowServer.cpp | 6 +++++ .../LibGUI/ConnectionToWindowServer.h | 1 + Userland/Libraries/LibGUI/Window.cpp | 6 +++++ Userland/Libraries/LibGUI/Window.h | 2 ++ Userland/Libraries/LibGUI/WindowMode.h | 1 + .../Services/WindowServer/MenuManager.cpp | 23 ++++++++----------- Userland/Services/WindowServer/MenuManager.h | 1 - .../Services/WindowServer/WindowClient.ipc | 1 + .../Services/WindowServer/WindowManager.cpp | 6 +++++ .../Services/WindowServer/WindowManager.h | 1 + Userland/Services/WindowServer/WindowMode.h | 9 ++++++++ 13 files changed, 48 insertions(+), 15 deletions(-) diff --git a/Userland/Libraries/LibGUI/ComboBox.cpp b/Userland/Libraries/LibGUI/ComboBox.cpp index 3e447d4d173..0b7e1c574d4 100644 --- a/Userland/Libraries/LibGUI/ComboBox.cpp +++ b/Userland/Libraries/LibGUI/ComboBox.cpp @@ -117,6 +117,7 @@ ComboBox::ComboBox() } m_open_button->set_enabled(true); }; + m_list_window->on_input_preemption = [this](auto) { close(); }; m_list_view = m_list_window->set_main_widget(); m_list_view->horizontal_scrollbar().set_visible(false); diff --git a/Userland/Libraries/LibGUI/CommandPalette.cpp b/Userland/Libraries/LibGUI/CommandPalette.cpp index 91616ff784f..2a0d63a2c08 100644 --- a/Userland/Libraries/LibGUI/CommandPalette.cpp +++ b/Userland/Libraries/LibGUI/CommandPalette.cpp @@ -227,6 +227,11 @@ CommandPalette::CommandPalette(GUI::Window& parent_window, ScreenPosition screen if (!is_active_window) close(); }; + + on_input_preemption = [this](InputPreemptor preemptor) { + if (preemptor != InputPreemptor::ContextMenu) + close(); + }; } void CommandPalette::collect_actions(GUI::Window& parent_window) diff --git a/Userland/Libraries/LibGUI/ConnectionToWindowServer.cpp b/Userland/Libraries/LibGUI/ConnectionToWindowServer.cpp index 88fb2db3095..7ceca0e2779 100644 --- a/Userland/Libraries/LibGUI/ConnectionToWindowServer.cpp +++ b/Userland/Libraries/LibGUI/ConnectionToWindowServer.cpp @@ -401,6 +401,12 @@ void ConnectionToWindowServer::window_state_changed(i32 window_id, bool minimize window->notify_state_changed({}, minimized, maximized, occluded); } +void ConnectionToWindowServer::window_input_preempted(i32 window_id, i32 preemptor) +{ + if (auto* window = Window::from_window_id(window_id)) + window->notify_input_preempted({}, static_cast(preemptor)); +} + void ConnectionToWindowServer::display_link_notification() { if (m_display_link_notification_pending) diff --git a/Userland/Libraries/LibGUI/ConnectionToWindowServer.h b/Userland/Libraries/LibGUI/ConnectionToWindowServer.h index e4f25cff2f5..893b0bac173 100644 --- a/Userland/Libraries/LibGUI/ConnectionToWindowServer.h +++ b/Userland/Libraries/LibGUI/ConnectionToWindowServer.h @@ -54,6 +54,7 @@ private: virtual void update_system_fonts(String const&, String const&, String const&) override; virtual void update_system_effects(Vector const&) override; virtual void window_state_changed(i32, bool, bool, bool) override; + virtual void window_input_preempted(i32, i32) override; virtual void display_link_notification() override; virtual void track_mouse_move(Gfx::IntPoint const&) override; virtual void ping() override; diff --git a/Userland/Libraries/LibGUI/Window.cpp b/Userland/Libraries/LibGUI/Window.cpp index e6459d73b6c..99690165308 100644 --- a/Userland/Libraries/LibGUI/Window.cpp +++ b/Userland/Libraries/LibGUI/Window.cpp @@ -1110,6 +1110,12 @@ void Window::notify_state_changed(Badge, bool minimize } } +void Window::notify_input_preempted(Badge, InputPreemptor preemptor) +{ + if (on_input_preemption) + on_input_preemption(preemptor); +} + Action* Window::action_for_shortcut(Shortcut const& shortcut) { Action* found_action = nullptr; diff --git a/Userland/Libraries/LibGUI/Window.h b/Userland/Libraries/LibGUI/Window.h index 7e684061294..cab90738b9d 100644 --- a/Userland/Libraries/LibGUI/Window.h +++ b/Userland/Libraries/LibGUI/Window.h @@ -91,6 +91,7 @@ public: Function on_close_request; Function on_active_input_change; Function on_active_window_change; + Function on_input_preemption; int x() const { return rect().x(); } int y() const { return rect().y(); } @@ -197,6 +198,7 @@ public: static void for_each_window(Badge, Function); static void update_all_windows(Badge); void notify_state_changed(Badge, bool minimized, bool maximized, bool occluded); + void notify_input_preempted(Badge, InputPreemptor); virtual bool is_visible_for_timer_purposes() const override { return m_visible_for_timer_purposes; } diff --git a/Userland/Libraries/LibGUI/WindowMode.h b/Userland/Libraries/LibGUI/WindowMode.h index 93bbad07010..9aeff8c803f 100644 --- a/Userland/Libraries/LibGUI/WindowMode.h +++ b/Userland/Libraries/LibGUI/WindowMode.h @@ -11,5 +11,6 @@ namespace GUI { using WindowMode = WindowServer::WindowMode; +using InputPreemptor = WindowServer::InputPreemptor; } diff --git a/Userland/Services/WindowServer/MenuManager.cpp b/Userland/Services/WindowServer/MenuManager.cpp index 07d8cab2fef..ad0750c6371 100644 --- a/Userland/Services/WindowServer/MenuManager.cpp +++ b/Userland/Services/WindowServer/MenuManager.cpp @@ -331,17 +331,14 @@ void MenuManager::open_menu(Menu& menu, bool as_current_menu) 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 + if (m_current_menu) { auto& wm = WindowManager::the(); - wm.restore_active_input_window(m_previous_input_window); if (auto* window = wm.window_with_active_menu()) { window->invalidate_menubar(); } wm.set_window_with_active_menu(nullptr); } + m_current_menu = nullptr; } void MenuManager::set_current_menu(Menu* menu) @@ -356,19 +353,17 @@ void MenuManager::set_current_menu(Menu* menu) return; } - Menu* previous_current_menu = m_current_menu; m_current_menu = menu; auto& wm = WindowManager::the(); - if (!previous_current_menu) { - // When opening the first menu, store the current active input window - if (auto* active_input = wm.active_input_window()) - m_previous_input_window = *active_input; - else - m_previous_input_window = nullptr; + if (auto* window = wm.active_input_window()) { + InputPreemptor preemptor { InputPreemptor::OtherMenu }; + if (window->rect().contains(m_current_menu->unadjusted_position())) + preemptor = InputPreemptor::ContextMenu; + else if (!m_current_menu->rect_in_window_menubar().is_null()) + preemptor = InputPreemptor::MenubarMenu; + wm.notify_input_preempted(*window, preemptor); } - - wm.set_active_input_window(m_current_menu->menu_window()); } Menu* MenuManager::previous_menu(Menu* current) diff --git a/Userland/Services/WindowServer/MenuManager.h b/Userland/Services/WindowServer/MenuManager.h index 77d214c8526..a770597f14c 100644 --- a/Userland/Services/WindowServer/MenuManager.h +++ b/Userland/Services/WindowServer/MenuManager.h @@ -58,7 +58,6 @@ private: void refresh(); WeakPtr m_current_menu; - WeakPtr m_previous_input_window; Vector> m_open_menu_stack; int m_theme_index { 0 }; diff --git a/Userland/Services/WindowServer/WindowClient.ipc b/Userland/Services/WindowServer/WindowClient.ipc index 73189312334..4688b052f9c 100644 --- a/Userland/Services/WindowServer/WindowClient.ipc +++ b/Userland/Services/WindowServer/WindowClient.ipc @@ -20,6 +20,7 @@ endpoint WindowClient window_activated(i32 window_id) =| window_deactivated(i32 window_id) =| window_state_changed(i32 window_id, bool minimized, bool maximized, bool occluded) =| + window_input_preempted(i32 window_id, i32 preemptor) =| window_close_request(i32 window_id) =| window_resized(i32 window_id, Gfx::IntRect new_rect) =| diff --git a/Userland/Services/WindowServer/WindowManager.cpp b/Userland/Services/WindowServer/WindowManager.cpp index eb07b9cb63b..e89aa6dca6b 100644 --- a/Userland/Services/WindowServer/WindowManager.cpp +++ b/Userland/Services/WindowServer/WindowManager.cpp @@ -642,6 +642,12 @@ void WindowManager::notify_progress_changed(Window& window) tell_wms_window_state_changed(window); } +void WindowManager::notify_input_preempted(Window& window, InputPreemptor preemptor) +{ + if (window.client()) + window.client()->async_window_input_preempted(window.window_id(), (i32)preemptor); +} + void WindowManager::pick_new_active_window(Window* previous_active) { Window* desktop = nullptr; diff --git a/Userland/Services/WindowServer/WindowManager.h b/Userland/Services/WindowServer/WindowManager.h index 93b3a8b4020..632520cf6db 100644 --- a/Userland/Services/WindowServer/WindowManager.h +++ b/Userland/Services/WindowServer/WindowManager.h @@ -87,6 +87,7 @@ public: void notify_occlusion_state_changed(Window&); void notify_progress_changed(Window&); void notify_modified_changed(Window&); + void notify_input_preempted(Window&, InputPreemptor = InputPreemptor::Other); Gfx::IntRect tiled_window_rect(Window const&, WindowTileType tile_type = WindowTileType::Maximized, bool relative_to_window_screen = false) const; diff --git a/Userland/Services/WindowServer/WindowMode.h b/Userland/Services/WindowServer/WindowMode.h index c70e8213371..3cb82ee1f83 100644 --- a/Userland/Services/WindowServer/WindowMode.h +++ b/Userland/Services/WindowServer/WindowMode.h @@ -24,4 +24,13 @@ enum class WindowMode { _Count, }; +// InputPreemptors are Objects which take input precedence over the active input +// window without changing its activity state or joining its modal chain +enum class InputPreemptor { + ContextMenu = 0, + MenubarMenu, + OtherMenu, + Other, +}; + }