Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add focus stealing prevention to MinimalWindowManager #3693

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions debian/libmiral7.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -491,3 +491,7 @@ libmiral.so.7 libmiral7 #MINVER#
(c++)"miral::InputConfiguration::touchpad(miral::InputConfiguration::Touchpad const&)@MIRAL_5.1" 5.1.0
(c++)"miral::InputConfiguration::~InputConfiguration()@MIRAL_5.1" 5.1.0
(c++)"miral::WindowManagerTools::move_cursor_to(mir::geometry::generic::Point<float>)@MIRAL_5.1" 5.1.0
MIRAL_5.2@MIRAL_5.2 5.2.0
(c++)"miral::MinimalWindowManager::MinimalWindowManager(miral::WindowManagerTools const&, MirInputEventModifier, miral::FocusStealing)@MIRAL_5.0" 5.2.0
(c++)"miral::MinimalWindowManager::MinimalWindowManager(miral::WindowManagerTools const&, miral::FocusStealing)@MIRAL_5.0" 5.2.0
(c++)"miral::WindowManagementPolicy::~WindowManagementPolicy()@MIRAL_5.0" 5.2.0
12 changes: 11 additions & 1 deletion examples/example-server-lib/floating_window_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,17 @@ FloatingWindowManagerPolicy::FloatingWindowManagerPolicy(
std::shared_ptr<SplashSession> const& spinner,
miral::InternalClientLauncher const& launcher,
std::function<void()>& shutdown_hook) :
MinimalWindowManager(tools),
FloatingWindowManagerPolicy{tools, spinner, launcher, shutdown_hook, FocusStealing::allow}
{
}

FloatingWindowManagerPolicy::FloatingWindowManagerPolicy(
WindowManagerTools const& tools,
std::shared_ptr<SplashSession> const& spinner,
miral::InternalClientLauncher const& launcher,
std::function<void()>& shutdown_hook,
FocusStealing focus_stealing) :
MinimalWindowManager(tools, focus_stealing),
spinner{spinner},
decoration_provider{std::make_unique<DecorationProvider>()}
{
Expand Down
8 changes: 8 additions & 0 deletions examples/example-server-lib/floating_window_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ class FloatingWindowManagerPolicy : public miral::MinimalWindowManager
std::shared_ptr<SplashSession> const& spinner,
miral::InternalClientLauncher const& launcher,
std::function<void()>& shutdown_hook);

FloatingWindowManagerPolicy(
miral::WindowManagerTools const& tools,
std::shared_ptr<SplashSession> const& spinner,
miral::InternalClientLauncher const& launcher,
std::function<void()>& shutdown_hook,
miral::FocusStealing focus_stealing);

~FloatingWindowManagerPolicy();

virtual miral::WindowSpecification place_new_window(
Expand Down
16 changes: 15 additions & 1 deletion examples/miral-shell/shell_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "miral/minimal_window_manager.h"
#include "tiling_window_manager.h"
#include "floating_window_manager.h"
#include "wallpaper_config.h"
Expand Down Expand Up @@ -68,9 +69,18 @@ int main(int argc, char const* argv[])

SpinnerSplash spinner;
InternalClientLauncher launcher;
auto constexpr default_focus_stealing_prevention = false;
auto const to_focus_stealing = [](bool focus_stealing_prevention)
{
if (focus_stealing_prevention)
return FocusStealing::prevent;
else
return FocusStealing::allow;
};
auto focus_stealing_prevention = to_focus_stealing(default_focus_stealing_prevention);
WindowManagerOptions window_managers
{
add_window_manager_policy<FloatingWindowManagerPolicy>("floating", spinner, launcher, shutdown_hook),
add_window_manager_policy<FloatingWindowManagerPolicy>("floating", spinner, launcher, shutdown_hook, focus_stealing_prevention),
add_window_manager_policy<TilingWindowManagerPolicy>("tiling", spinner, launcher),
};

Expand Down Expand Up @@ -139,6 +149,10 @@ int main(int argc, char const* argv[])
WaylandExtensions{},
X11Support{},
ConfigureDecorations{},
pre_init(ConfigurationOption{[&](bool option)
{ focus_stealing_prevention = to_focus_stealing(option); },
"focus-stealing-prevention", "allow or prevent focus stealing",
default_focus_stealing_prevention}),
window_managers,
display_configuration_options,
external_client_launcher,
Expand Down
17 changes: 17 additions & 0 deletions include/miral/miral/minimal_window_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,35 @@

namespace miral
{
enum class FocusStealing
{
prevent,
allow
};

/// Minimal implementation of a floating window management policy
/// \remark Since MirAL 2.5
class MinimalWindowManager : public WindowManagementPolicy
{
public:
explicit MinimalWindowManager(WindowManagerTools const& tools);

/// Allows shells to enable or disable focus stealing prevention.
/// \remark Since MirAL 5.2
explicit MinimalWindowManager(WindowManagerTools const& tools, FocusStealing focus_stealing);

/// Allows shells to change the modifer used to identify a window drag gesture
/// The default is mir_input_event_modifier_alt
/// \remark Since MirAL 3.7
MinimalWindowManager(WindowManagerTools const& tools, MirInputEventModifier pointer_drag_modifier);

/// Allows shells to to change the modifer used to identify a window drag
/// gesture and enable or disable focus stealing prevention.
/// The default drag modifier is mir_input_event_modifier_alt.
/// \remark Since MirAL 5.2
MinimalWindowManager(
WindowManagerTools const& tools, MirInputEventModifier pointer_drag_modifier, FocusStealing focus_stealing);

~MinimalWindowManager();

/// Honours the requested specification
Expand Down
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
set(MIRPLATFORM_ABI 30)

set(MIRAL_VERSION_MAJOR 5)
set(MIRAL_VERSION_MINOR 1)
set(MIRAL_VERSION_MINOR 2)
set(MIRAL_VERSION_PATCH 0)
set(MIRAL_VERSION ${MIRAL_VERSION_MAJOR}.${MIRAL_VERSION_MINOR}.${MIRAL_VERSION_PATCH})

Expand Down
22 changes: 17 additions & 5 deletions src/miral/application_selector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ void ApplicationSelector::advise_new_window(WindowInfo const& window_info)
focus_list.push_back(window_info.window());
}

void ApplicationSelector::advise_new_window(WindowInfo const& window_info, bool focused)
{
if(!focused && !focus_list.empty())
focus_list.insert(focus_list.end() - 1, window_info.window());
else
advise_new_window(window_info);
}

void ApplicationSelector::select(miral::Window const& window)
{
if (selected)
Expand Down Expand Up @@ -204,6 +212,7 @@ auto ApplicationSelector::advance(bool reverse, bool within_app) -> Window
auto it = find(selected);

std::optional<Window> next_window = std::nullopt;
auto should_continue = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty much the same logic, but I think this is a bit clearer. Not sure if I should revert or keep these changes..

do {
if (reverse)
{
Expand All @@ -228,9 +237,10 @@ auto ApplicationSelector::advance(bool reverse, bool within_app) -> Window
if (within_app)
{
if (it->application() == (*originally_selected_it).application() && tools.can_select_window(*it))
{
next_window = *it;
else
next_window = std::nullopt;
should_continue = false;
}
}
else
{
Expand All @@ -243,15 +253,17 @@ auto ApplicationSelector::advance(bool reverse, bool within_app) -> Window

if (prev_it->application() == it->application())
{
next_window = std::nullopt;
already_encountered = true;
break;
}
}
if (!already_encountered)
{
next_window = tools.window_to_select_application(it->application());
should_continue = !next_window.has_value();
}
}
} while (next_window == std::nullopt);
} while (should_continue);

if (next_window == std::nullopt)
return selected;
Expand Down Expand Up @@ -281,4 +293,4 @@ auto ApplicationSelector::find(Window window) -> std::vector<Window>::iterator
{
return window == other;
});
}
}
2 changes: 2 additions & 0 deletions src/miral/application_selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ class ApplicationSelector
/// Called when a window is created
void advise_new_window(WindowInfo const&);

void advise_new_window(WindowInfo const&, bool focused);

/// Called when focus is given to a window.
void advise_focus_gained(WindowInfo const&);

Expand Down
12 changes: 11 additions & 1 deletion src/miral/basic_window_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,16 @@ void miral::BasicWindowManager::surface_ready(std::shared_ptr<scene::Surface> co
{
Locker lock{this};
policy->handle_window_ready(info_for(surface));

// Need to push onto the MRU list for alt+tab to work correctly.
//
// Originally, applications were pushed only when selected, but due to
// focus stealing prevention, they were not activated and thus weren't
// selected nor added to mru_active_windows, breaking alt+tab
auto window = info_for(surface).window();
auto const top = mru_active_windows.top();
if(top && top != window)
mru_active_windows.insert_below_top(window);
}

void miral::BasicWindowManager::modify_surface(
Expand Down Expand Up @@ -2993,4 +3003,4 @@ void miral::BasicWindowManager::move_cursor_to(mir::geometry::PointF point)
sink->handle_input(std::move(event));
});
});
}
}
50 changes: 45 additions & 5 deletions src/miral/minimal_window_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,14 @@ auto touch_center(MirTouchEvent const* event) -> mir::geometry::Point

struct miral::MinimalWindowManager::Impl
{
Impl(WindowManagerTools const& tools, MirInputEventModifier pointer_drag_modifier) :
tools{tools}, application_selector(tools), pointer_drag_modifier{pointer_drag_modifier} {}
Impl(WindowManagerTools const& tools, MirInputEventModifier pointer_drag_modifier, FocusStealing focus_stealing) :
tools{tools},
application_selector(tools),
pointer_drag_modifier{pointer_drag_modifier},
focus_stealing{focus_stealing}
{
}

WindowManagerTools tools;

Gesture gesture = Gesture::none;
Expand Down Expand Up @@ -98,17 +104,32 @@ struct miral::MinimalWindowManager::Impl

void apply_resize_by(Displacement movement);

bool prevent_focus_stealing(WindowInfo const& info);

MirInputEventModifier const pointer_drag_modifier;
FocusStealing const focus_stealing;
};

miral::MinimalWindowManager::MinimalWindowManager(WindowManagerTools const& tools) :
MinimalWindowManager{tools, mir_input_event_modifier_alt}
{
}

miral::MinimalWindowManager::MinimalWindowManager(WindowManagerTools const& tools, FocusStealing focus_stealing) :
tools{tools},
self{new Impl{tools, mir_input_event_modifier_alt, focus_stealing}}
{
}

miral::MinimalWindowManager::MinimalWindowManager(WindowManagerTools const& tools, MirInputEventModifier pointer_drag_modifier):
tools{tools},
self{new Impl{tools, pointer_drag_modifier}}
self{new Impl{tools, pointer_drag_modifier, FocusStealing::allow}}
{
}

miral::MinimalWindowManager::MinimalWindowManager(WindowManagerTools const& tools, MirInputEventModifier pointer_drag_modifier, FocusStealing focus_stealing):
tools{tools},
self{new Impl{tools, pointer_drag_modifier, focus_stealing}}
{
}

Expand All @@ -126,7 +147,7 @@ auto miral::MinimalWindowManager::place_new_window(

void miral::MinimalWindowManager::handle_window_ready(WindowInfo& window_info)
{
if (window_info.can_be_active())
if (!self->prevent_focus_stealing(window_info) && window_info.can_be_active())
{
tools.select_active_window(window_info.window());
}
Expand Down Expand Up @@ -276,7 +297,17 @@ auto miral::MinimalWindowManager::confirm_inherited_move(WindowInfo const& windo

void miral::MinimalWindowManager::advise_new_window(miral::WindowInfo const& window_info)
{
self->application_selector.advise_new_window(window_info);
// If the window is prevented from stealing focus, swap the old focused
// window (now in the back) with the new window in the front.
//
// If it's a legitimate window, it'll be focused and raised via
// xdg-activation.
auto in_background = self->prevent_focus_stealing(window_info);

self->application_selector.advise_new_window(window_info, in_background);

if (in_background)
tools.swap_tree_order(tools.active_window(), window_info.window());
}

void miral::MinimalWindowManager::advise_focus_gained(WindowInfo const& window_info)
Expand Down Expand Up @@ -606,3 +637,12 @@ void miral::MinimalWindowManager::Impl::apply_resize_by(Displacement movement)
gesture = Gesture::none;
}
}

bool miral::MinimalWindowManager::Impl::prevent_focus_stealing(miral::WindowInfo const& info)
{
auto const normal_app_without_parent = (info.depth_layer() == mir_depth_layer_application && !info.parent());
auto const decoration_surface = info.type() == mir_window_type_decoration;
return (focus_stealing == FocusStealing::prevent) && tools.active_window() &&
(normal_app_without_parent || decoration_surface) &&
tools.active_window().application() != info.window().application();
}
9 changes: 9 additions & 0 deletions src/miral/mru_window_list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ void miral::MRUWindowList::push(Window const& window)
windows.push_back(window);
}

void miral::MRUWindowList::insert_below_top(Window const& window)
{
windows.erase(remove(begin(windows), end(windows), window), end(windows));
if(!windows.empty())
windows.insert(windows.end()-1, window);
else
windows.push_back(window);
}

void miral::MRUWindowList::erase(Window const& window)
{
windows.erase(remove(begin(windows), end(windows), window), end(windows));
Expand Down
1 change: 1 addition & 0 deletions src/miral/mru_window_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class MRUWindowList
public:

void push(Window const& window);
void insert_below_top(Window const& window);
void erase(Window const& window);
auto top() const -> Window;

Expand Down
Loading