From e5aef0331996aa0c77f7ea2f0af390b40704f626 Mon Sep 17 00:00:00 2001
From: John Lindgren <john@jlindgren.net>
Date: Wed, 27 Sep 2023 18:37:28 -0400
Subject: [PATCH] desktop: switch workspaces and optionally raise in
 desktop_focus_view()

Make desktop_focus_view() always switch to the workspace containing the
view being focused. It doesn't make much sense for an invisible view to
have the keyboard focus.

Also add an optional "raise" parameter to desktop_focus_view(). This
allows the common pattern of desktop_focus_view() + view_move_to_front()
to be reduced to a single function call.
---
 include/labwc.h        |  2 +-
 include/workspaces.h   |  8 +++++---
 src/action.c           |  5 +++--
 src/cursor.c           | 10 ++--------
 src/desktop.c          | 21 ++++++++++++++++-----
 src/foreign.c          |  6 +-----
 src/keyboard.c         |  4 ++--
 src/view-impl-common.c |  3 +--
 src/workspaces.c       | 15 +++++++++++----
 src/xdg.c              | 10 +---------
 src/xwayland.c         |  3 +--
 11 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/include/labwc.h b/include/labwc.h
index 91c9cfdc..d7f5cbde 100644
--- a/include/labwc.h
+++ b/include/labwc.h
@@ -377,7 +377,7 @@ void foreign_toplevel_update_outputs(struct view *view);
  *              cannot assume this means that the window actually has keyboard
  *              or pointer focus, in this compositor are they called together.
  */
-void desktop_focus_view(struct view *view);
+void desktop_focus_view(struct view *view, bool raise);
 void desktop_arrange_all_views(struct server *server);
 void desktop_focus_output(struct output *output);
 struct view *desktop_topmost_mapped_view(struct server *server);
diff --git a/include/workspaces.h b/include/workspaces.h
index e533f7f9..997865f2 100644
--- a/include/workspaces.h
+++ b/include/workspaces.h
@@ -2,10 +2,12 @@
 #ifndef LABWC_WORKSPACES_H
 #define LABWC_WORKSPACES_H
 
+#include <stdbool.h>
+#include <wayland-util.h>
+
 struct seat;
-struct view;
 struct server;
-struct wl_list;
+struct wlr_scene_tree;
 
 /* Double use: as config in config/rcxml.c and as instance in workspaces.c */
 struct workspace {
@@ -20,7 +22,7 @@ struct workspace {
 };
 
 void workspaces_init(struct server *server);
-void workspaces_switch_to(struct workspace *target);
+void workspaces_switch_to(struct workspace *target, bool update_focus);
 void workspaces_destroy(struct server *server);
 void workspaces_osd_hide(struct seat *seat);
 struct workspace *workspaces_find(struct workspace *anchor, const char *name,
diff --git a/src/action.c b/src/action.c
index 8433268d..cf8b4558 100644
--- a/src/action.c
+++ b/src/action.c
@@ -634,7 +634,7 @@ actions_run(struct view *activator, struct server *server,
 			break;
 		case ACTION_TYPE_FOCUS:
 			if (view) {
-				desktop_focus_view(view);
+				desktop_focus_view(view, /*raise*/ false);
 			}
 			break;
 		case ACTION_TYPE_ICONIFY:
@@ -711,7 +711,8 @@ actions_run(struct view *activator, struct server *server,
 					follow = get_arg_value_bool(action, "follow", true);
 				}
 				if (follow) {
-					workspaces_switch_to(target);
+					workspaces_switch_to(target,
+						/*update_focus*/ true);
 				}
 			}
 			break;
diff --git a/src/cursor.c b/src/cursor.c
index 4a5b30cd..16d9e824 100644
--- a/src/cursor.c
+++ b/src/cursor.c
@@ -478,10 +478,7 @@ process_cursor_motion(struct server *server, uint32_t time)
 	}
 
 	if (ctx.view && rc.focus_follow_mouse) {
-		desktop_focus_view(ctx.view);
-		if (rc.raise_on_focus) {
-			view_move_to_front(ctx.view);
-		}
+		desktop_focus_view(ctx.view, rc.raise_on_focus);
 	}
 
 	struct mousebind *mousebind;
@@ -522,10 +519,7 @@ _cursor_update_focus(struct server *server)
 			&& !rc.focus_follow_mouse_requires_movement
 			&& !server->osd_state.cycle_view) {
 		/* Prevents changing keyboard focus during A-Tab */
-		desktop_focus_view(ctx.view);
-		if (rc.raise_on_focus) {
-			view_move_to_front(ctx.view);
-		}
+		desktop_focus_view(ctx.view, rc.raise_on_focus);
 	}
 
 	cursor_update_common(server, &ctx, msec(&now),
diff --git a/src/desktop.c b/src/desktop.c
index 58177224..cbaa0de6 100644
--- a/src/desktop.c
+++ b/src/desktop.c
@@ -35,7 +35,7 @@ desktop_arrange_all_views(struct server *server)
 }
 
 void
-desktop_focus_view(struct view *view)
+desktop_focus_view(struct view *view, bool raise)
 {
 	assert(view);
 	/*
@@ -56,7 +56,7 @@ desktop_focus_view(struct view *view)
 	if (view->minimized) {
 		/*
 		 * Unminimizing will map the view which triggers a call to this
-		 * function again.
+		 * function again (with raise=true).
 		 */
 		view_minimize(view, false);
 		return;
@@ -66,7 +66,19 @@ desktop_focus_view(struct view *view)
 		return;
 	}
 
+	/*
+	 * Switch workspace if necessary to make the view visible
+	 * (unnecessary for "always on top" views).
+	 */
+	if (!view_is_always_on_top(view)) {
+		workspaces_switch_to(view->workspace, /*update_focus*/ false);
+	}
+
 	view_focus(view);
+
+	if (raise) {
+		view_move_to_front(view);
+	}
 }
 
 static struct wl_list *
@@ -225,8 +237,7 @@ desktop_focus_topmost_mapped_view(struct server *server)
 {
 	struct view *view = desktop_topmost_mapped_view(server);
 	if (view) {
-		desktop_focus_view(view);
-		view_move_to_front(view);
+		desktop_focus_view(view, /*raise*/ true);
 	} else {
 		/*
 		 * Defocus previous focused surface/view if no longer
@@ -262,7 +273,7 @@ desktop_focus_output(struct output *output)
 		}
 		if (wlr_output_layout_intersects(layout,
 				output->wlr_output, &view->current)) {
-			desktop_focus_view(view);
+			desktop_focus_view(view, /*raise*/ false);
 			wlr_cursor_warp(output->server->seat.cursor, NULL,
 				view->current.x + view->current.width / 2,
 				view->current.y + view->current.height / 2);
diff --git a/src/foreign.c b/src/foreign.c
index fd5237d7..e71292d7 100644
--- a/src/foreign.c
+++ b/src/foreign.c
@@ -34,11 +34,7 @@ handle_request_activate(struct wl_listener *listener, void *data)
 	struct view *view = wl_container_of(listener, view, toplevel.activate);
 	// struct wlr_foreign_toplevel_handle_v1_activated_event *event = data;
 	/* In a multi-seat world we would select seat based on event->seat here. */
-	if (view->workspace != view->server->workspace_current) {
-		workspaces_switch_to(view->workspace);
-	}
-	desktop_focus_view(view);
-	view_move_to_front(view);
+	desktop_focus_view(view, /*raise*/ true);
 }
 
 static void
diff --git a/src/keyboard.c b/src/keyboard.c
index 73b8c9fe..b3d44766 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -42,8 +42,8 @@ static void
 end_cycling(struct server *server)
 {
 	if (server->osd_state.cycle_view) {
-		desktop_focus_view(server->osd_state.cycle_view);
-		view_move_to_front(server->osd_state.cycle_view);
+		desktop_focus_view(server->osd_state.cycle_view,
+			/*raise*/ true);
 	}
 
 	/* osd_finish() additionally resets cycle_view to NULL */
diff --git a/src/view-impl-common.c b/src/view-impl-common.c
index 1c7a1e71..ea0fe1a7 100644
--- a/src/view-impl-common.c
+++ b/src/view-impl-common.c
@@ -51,8 +51,7 @@ view_impl_move_sub_views(struct view *parent, enum z_direction z_direction)
 void
 view_impl_map(struct view *view)
 {
-	desktop_focus_view(view);
-	view_move_to_front(view);
+	desktop_focus_view(view, /*raise*/ true);
 	view_update_title(view);
 	view_update_app_id(view);
 	if (!view->been_mapped) {
diff --git a/src/workspaces.c b/src/workspaces.c
index e3943ac4..48be68f3 100644
--- a/src/workspaces.c
+++ b/src/workspaces.c
@@ -255,8 +255,13 @@ workspaces_init(struct server *server)
 	}
 }
 
+/*
+ * update_focus should normally be set to true. It is set to false only
+ * when this function is called from desktop_focus_view(), in order to
+ * avoid unnecessary extra focus changes and possible recursion.
+ */
 void
-workspaces_switch_to(struct workspace *target)
+workspaces_switch_to(struct workspace *target, bool update_focus)
 {
 	assert(target);
 	struct server *server = target->server;
@@ -281,9 +286,11 @@ workspaces_switch_to(struct workspace *target)
 	 * Make sure we are focusing what the user sees.
 	 * Only refocus if the focus is not already on an always-on-top view.
 	 */
-	struct view *view = desktop_focused_view(server);
-	if (!view || !view_is_always_on_top(view)) {
-		desktop_focus_topmost_mapped_view(server);
+	if (update_focus) {
+		struct view *view = desktop_focused_view(server);
+		if (!view || !view_is_always_on_top(view)) {
+			desktop_focus_topmost_mapped_view(server);
+		}
 	}
 
 	/* And finally show the OSD */
diff --git a/src/xdg.c b/src/xdg.c
index 2f429aff..65f164d3 100644
--- a/src/xdg.c
+++ b/src/xdg.c
@@ -607,16 +607,8 @@ xdg_activation_handle_request(struct wl_listener *listener, void *data)
 		return;
 	}
 
-	/*
-	 * TODO: This is the exact same code as used in foreign.c.
-	 *       Refactor it into a public helper function somewhere.
-	 */
 	wlr_log(WLR_DEBUG, "Activating surface");
-	if (view->workspace != view->server->workspace_current) {
-		workspaces_switch_to(view->workspace);
-	}
-	desktop_focus_view(view);
-	view_move_to_front(view);
+	desktop_focus_view(view, /*raise*/ true);
 }
 
 /*
diff --git a/src/xwayland.c b/src/xwayland.c
index 3a0a8a9a..4423345b 100644
--- a/src/xwayland.c
+++ b/src/xwayland.c
@@ -256,8 +256,7 @@ handle_request_activate(struct wl_listener *listener, void *data)
 		return;
 	}
 
-	desktop_focus_view(view);
-	view_move_to_front(view);
+	desktop_focus_view(view, /*raise*/ true);
 }
 
 static void