Skip to content

window-props: Read WM_TRANSIENT_FOR for override-redirect windows

Marco Trevisan requested to merge 3v1n0/mutter:override-redirect-transient_for-support into master

This is a continuation of !895 (merged) as per comments there (so only last commit matters).

As per the Extended Window Manager Hints standard version 1.3 [1] an override-redirect window can set a transient-for window per compositing and app-matching purposes.

So just read the WM_TRASIENT_FOR property also for such windows, adapting the error in case they are transient for another O-R window and adding a test to check such case.

[1] https://standards.freedesktop.org/wm-spec/wm-spec-latest.html#idm140200472512128

!920 (merged)

In order to check if we didn't break anything at mutter level (but I've done something also in the shell, and I didn't find anything that could create issues), I went through all the various usage of ->transient_for or get_transient_for.

Here's an analysis

Mutter

All good

src/core/window.c-  if (window->input_region)
src/core/window.c-    cairo_region_destroy (window->input_region);
src/core/window.c-
src/core/window.c:  if (window->transient_for)
src/core/window.c:    g_object_unref (window->transient_for);
src/core/window.c-
src/core/window.c-  g_free (window->sm_client_id);
src/core/window.c-  g_free (window->wm_client_machine);

A window can't have a O-R window as parent as per this MR and !895 (merged), while if window is an O-R, this code won't ever be called as per a previous check.

src/core/window.c-       * being recorded as a fallback for potential transients
src/core/window.c-       */
src/core/window.c-      window->net_wm_user_time = window->initial_timestamp;
src/core/window.c:    else if (window->transient_for != NULL)
src/core/window.c:      meta_window_set_user_time (window, window->transient_for->net_wm_user_time);
src/core/window.c-    else
src/core/window.c-      /* NOTE: Do NOT toggle net_wm_user_time_set to true; this is just
src/core/window.c-       * being recorded as a fallback for potential transients

This is covered already by !895 (merged)

src/core/window.c-   */
src/core/window.c-  if (!window->override_redirect && window->workspace == NULL)
src/core/window.c-    {
src/core/window.c:      if (window->transient_for != NULL)
src/core/window.c-        {
src/core/window.c-          meta_topic (META_DEBUG_PLACEMENT,
src/core/window.c-                      "Putting window %s on same workspace as parent %s\n",
src/core/window.c:                      window->desc, window->transient_for->desc);
src/core/window.c-
src/core/window.c:          g_assert_false (window->transient_for->override_redirect);
src/core/window.c-          set_workspace_state (window,
src/core/window.c:                               should_be_on_all_workspaces (window->transient_for),
src/core/window.c:                               window->transient_for->workspace);
src/core/window.c-        }
src/core/window.c-      else if (window->on_all_workspaces)
src/core/window.c-        {

These are in meta_window_activate_full() which is called only for non-override-redirect windows as per meta_window_activate_* checks.

src/core/window.c-  /* For non-transient windows, we just set up a pulsing indicator,
src/core/window.c-     rather than move windows or workspaces.
src/core/window.c-     See http://bugzilla.gnome.org/show_bug.cgi?id=482354 */
src/core/window.c:  if (window->transient_for == NULL &&
src/core/window.c-      !allow_workspace_switch &&
src/core/window.c-      !meta_window_located_on_workspace (window, workspace))
src/core/window.c-    {
--
src/core/window.c-      /* We've marked it as demanding, don't need to do anything else. */
src/core/window.c-      return;
src/core/window.c-    }
src/core/window.c:  else if (window->transient_for != NULL)
src/core/window.c-    {
src/core/window.c-      /* Move transients to current workspace - preference dialogs should appear over
src/core/window.c-         the source window.  */

No problem here (get_modal_transient())

src/core/window.c-    {
src/core/window.c-      MetaWindow *transient = tmp->data;
src/core/window.c-
src/core/window.c:      if (transient->transient_for == modal_transient &&
src/core/window.c-          transient->type == META_WINDOW_MODAL_DIALOG)
src/core/window.c-        {
src/core/window.c-          modal_transient = transient;

No problem here (meta_window_recalc_skip_features()), since in such case it would be marked as META_WINDOW_OVERRIDE_OTHER so this code wouldn't be either reached.

src/core/window.c-    case META_WINDOW_MODAL_DIALOG:
src/core/window.c-      /* only skip taskbar if we have a real transient parent
src/core/window.c-         (and ignore the application hints) */
src/core/window.c:      if (window->transient_for != NULL)
src/core/window.c-        window->skip_taskbar = TRUE;
src/core/window.c-      else
src/core/window.c-        window->skip_taskbar = FALSE;

meta_window_foreach_ancestor() is fine as well

src/core/window.c-  w = window;
src/core/window.c-  do
src/core/window.c-    {
src/core/window.c:      if (w->transient_for == NULL)
src/core/window.c-        break;
src/core/window.c-
src/core/window.c:      w = w->transient_for;
src/core/window.c-    }
src/core/window.c-  while (w && (* func) (w, user_data));
src/core/window.c-}

meta_window_get_transient_for() should of course return it anyways, and in fact we mostly use this for matching purposes around (i.e. in both stack and shell tracker which we want)

src/core/window.c-{
src/core/window.c-  g_return_val_if_fail (META_IS_WINDOW (window), NULL);
src/core/window.c-
src/core/window.c:  if (window->transient_for)
src/core/window.c:    return window->transient_for;
src/core/window.c-  else if (window->xtransient_for)
src/core/window.c-    return meta_x11_display_lookup_x_window (window->display->x11_display,
src/core/window.c-                                             window->xtransient_for);

check_transient_for_loop(), this is fine to have (although, repeated)

src/core/window.c-    {
src/core/window.c-      if (parent == window)
src/core/window.c-          return TRUE;
src/core/window.c:      parent = parent->transient_for;
src/core/window.c-    }
src/core/window.c-
src/core/window.c-  return FALSE;

This is all part of meta_window_set_transient_for() and there's nothing wrong in case the current window is an O-R, and we already protect in such case the calls to meta_stack_update_transient() and meta_window_queue()

src/core/window.c-      return;
src/core/window.c-    }
src/core/window.c-
src/core/window.c:  if (meta_window_appears_focused (window) && window->transient_for != NULL)
src/core/window.c-    meta_window_propagate_focus_appearance (window, FALSE);
src/core/window.c-
src/core/window.c-  /* may now be a dialog */
--
src/core/window.c-           * parents, we need to destroy the MetaWindow and let a new one
src/core/window.c-           * be created (which happens as a side effect of
src/core/window.c-           * meta_window_unmanage()). The condition below is correct
src/core/window.c:           * because we know window->transient_for has changed.
src/core/window.c-           */
src/core/window.c-          if (window->attached || meta_window_should_attach_to_parent (window))
src/core/window.c-            {
--
src/core/window.c-      return;
src/core/window.c-    }
src/core/window.c-  /* We know this won't create a reference cycle because we check for loops */
src/core/window.c:  g_clear_object (&window->transient_for);
src/core/window.c:  window->transient_for = parent ? g_object_ref (parent) : NULL;
src/core/window.c-
src/core/window.c-  /* update stacking constraints */
src/core/window.c-  if (!window->override_redirect)
--
src/core/window.c-  if (!window->constructing && !window->override_redirect)
src/core/window.c-    meta_window_queue (window, META_QUEUE_MOVE_RESIZE | META_QUEUE_CALC_SHOWING);
src/core/window.c-
src/core/window.c:  if (meta_window_appears_focused (window) && window->transient_for != NULL)
src/core/window.c-    meta_window_propagate_focus_appearance (window, TRUE);
src/core/window.c-}
src/core/window.c-

Not applies here

src/core/meta-close-dialog-default.c-    {
src/core/meta-close-dialog-default.c-      MetaWindow *w = tmp->data;
src/core/meta-close-dialog-default.c-
src/core/meta-close-dialog-default.c:      if (w->transient_for == window && w->res_class &&
src/core/meta-close-dialog-default.c-          g_ascii_strcasecmp (w->res_class, "mutter-dialog") == 0)
src/core/meta-close-dialog-default.c-        {
src/core/meta-close-dialog-default.c-          meta_window_activate (w, CLUTTER_CURRENT_TIME);

OR windows aren't stacked anyways

src/core/stack.c-           w->type == META_WINDOW_UTILITY)
src/core/stack.c-
src/core/stack.c-#define WINDOW_TRANSIENT_FOR_WHOLE_GROUP(w)                     \
src/core/stack.c:  (WINDOW_HAS_TRANSIENT_TYPE (w) && w->transient_for == NULL)
src/core/stack.c-
src/core/stack.c-static void meta_window_set_stack_position_no_sync (MetaWindow *window,
src/core/stack.c-                                                    int         position);
--
src/core/stack.c-   */
src/core/stack.c-  if (window->layer != META_LAYER_DESKTOP &&
src/core/stack.c-      WINDOW_HAS_TRANSIENT_TYPE(window) &&
src/core/stack.c:      window->transient_for == NULL)
src/core/stack.c-    {
src/core/stack.c-      /* We only do the group thing if the dialog is NOT transient for
src/core/stack.c-       * a particular window. Imagine a group with a normal window, a dock,
--
src/core/stack.c-
src/core/stack.c-          g_slist_free (group_windows);
src/core/stack.c-        }
src/core/stack.c:      else if (w->transient_for != NULL)
src/core/stack.c-        {
src/core/stack.c-          MetaWindow *parent;
src/core/stack.c-
src/core/stack.c:          parent = w->transient_for;
src/core/stack.c-
src/core/stack.c-          if (parent && meta_window_is_in_stack (parent))
src/core/stack.c-            {

This handles later alrady the case of override_redirect windows, and in such case, the window will be marked as META_WINDOW_OVERRIDE_OTHER, so nothing to worry about.

src/x11/window-x11.c-            XFree (atom_name);
src/x11/window-x11.c-        }
src/x11/window-x11.c-    }
src/x11/window-x11.c:  else if (window->transient_for != NULL)
src/x11/window-x11.c-    {
src/x11/window-x11.c-      type = META_WINDOW_DIALOG;
src/x11/window-x11.c-    }

meta_window_should_attach_to_parent(), we'd alread returned before this if the window was an O-R (i.e not a dialog).

src/core/window.c-      window->type != META_WINDOW_MODAL_DIALOG)
src/core/window.c-    return FALSE;
src/core/window.c-
src/core/window.c:  parent = meta_window_get_transient_for (window);
src/core/window.c-  if (!parent)
src/core/window.c-    return FALSE;
src/core/window.c-

This is never happening again, since meta_window_should_attach_to_parent() Won't return TRUE for O-R windows.

src/core/window.c-  if (window->attached)
src/core/window.c-    {
src/core/window.c-      /* Only return the immediate children of the window being unmanaged */
src/core/window.c:      parent = meta_window_get_transient_for (window);
src/core/window.c-      if (parent->unmanaging)
src/core/window.c-        *children = g_list_prepend (*children, window);
src/core/window.c-    }

meta_window_propagate_focus_appearance(), don't seem to be wrong again

src/core/window.c-  focus_window = window->display->focus_window;
src/core/window.c-
src/core/window.c-  child = window;
src/core/window.c:  parent = meta_window_get_transient_for (child);
src/core/window.c-  while (parent && (!focused || should_propagate_focus_appearance (child)))
src/core/window.c-    {
src/core/window.c-      gboolean child_focus_state_changed;
--
src/core/window.c-        }
src/core/window.c-
src/core/window.c-      child = parent;
src/core/window.c:      parent = meta_window_get_transient_for (child);
src/core/window.c-    }
src/core/window.c-}
src/core/window.c-

meta_window_process_placement() called only if window has window->placement_rule set, never true for O-R windows.

src/core/place.c-                               int               *x,
src/core/place.c-                               int               *y)
src/core/place.c-{
src/core/place.c:  MetaWindow *parent = meta_window_get_transient_for (window);
src/core/place.c-  MetaRectangle parent_rect;
src/core/place.c-  MetaRectangle anchor_rect;
src/core/place.c-  int window_width, window_height;

No such window type in any O-R window ever.

src/core/place.c-  if (window->type == META_WINDOW_DIALOG ||
src/core/place.c-      window->type == META_WINDOW_MODAL_DIALOG)
src/core/place.c-    {
src/core/place.c:      MetaWindow *parent = meta_window_get_transient_for (window);
src/core/place.c-
src/core/place.c-      if (parent)
src/core/place.c-        {

constrain_modal_dialog(), we'd have already returned here if not a dialog

src/core/constraints.c-
src/core/constraints.c-  adjusted_unconstrained = info->current;
src/core/constraints.c-
src/core/constraints.c:  parent = meta_window_get_transient_for (window);
src/core/constraints.c-  meta_window_get_frame_rect (parent, &parent_rect);
src/core/constraints.c-
src/core/constraints.c-  switch (window->placement_state)

constrain_custom_rule(), again not possible to have a placement rule set

src/core/constraints.c-                        gboolean            check_only)
src/core/constraints.c-{
src/core/constraints.c-  int x, y;
src/core/constraints.c:  MetaWindow *parent = meta_window_get_transient_for (window);
src/core/constraints.c-  MetaRectangle child_rect, parent_rect;
src/core/constraints.c-  gboolean constraint_already_satisfied;
src/core/constraints.c-

called by meta_display_begin_grab_op(), O-R windows should not be called for such function anyways, but in any case meta_window_is_attached_dialog() would not return TRUE for a such window.

src/core/display.c-get_first_freefloating_window (MetaWindow *window)
src/core/display.c-{
src/core/display.c-  while (meta_window_is_attached_dialog (window))
src/core/display.c:    window = meta_window_get_transient_for (window);
src/core/display.c-
src/core/display.c-  /* Attached dialogs should always have a non-NULL transient-for */
src/core/display.c-  g_assert (window != NULL);

Shell

No applies here

./js/ui/altTab.js-                                              workspace);
./js/ui/altTab.js-    // ... map windows to their parent where appropriate ...
./js/ui/altTab.js-    return windows.map(w => {
./js/ui/altTab.js:        return w.is_attached_dialog() ? w.get_transient_for() : w;
./js/ui/altTab.js-    // ... and filter out skip-taskbar windows and duplicates
./js/ui/altTab.js-    }).filter((w, i, a) => !w.skip_taskbar && a.indexOf(w) == i);
./js/ui/altTab.js-}

All good here too

./js/ui/workspaceThumbnail.js-        if (this._isOverviewWindow(win)) {
./js/ui/workspaceThumbnail.js-            this._addWindowClone(win);
./js/ui/workspaceThumbnail.js-        } else if (metaWin.is_attached_dialog()) {
./js/ui/workspaceThumbnail.js:            let parent = metaWin.get_transient_for();
./js/ui/workspaceThumbnail.js-            while (parent.is_attached_dialog())
./js/ui/workspaceThumbnail.js:                parent = parent.get_transient_for();
./js/ui/workspaceThumbnail.js-
./js/ui/workspaceThumbnail.js-            let idx = this._lookupIndex(parent);
./js/ui/workspaceThumbnail.js-            if (idx < 0) {

Ditto

./js/ui/workspace.js-    }
./js/ui/workspace.js-
./js/ui/workspace.js-    addDialog(win) {
./js/ui/workspace.js:        let parent = win.get_transient_for();
./js/ui/workspace.js-        while (parent.is_attached_dialog())
./js/ui/workspace.js:            parent = parent.get_transient_for();
./js/ui/workspace.js-
./js/ui/workspace.js-        // Display dialog if it is attached to our metaWindow
./js/ui/workspace.js-        if (win.is_attached_dialog() && parent == this.metaWindow) {
--
./js/ui/workspace.js-            return;
./js/ui/workspace.js-
./js/ui/workspace.js-        if (!this._isOverviewWindow(win)) {
./js/ui/workspace.js:            if (metaWin.get_transient_for() == null)
./js/ui/workspace.js-                return;
./js/ui/workspace.js-
./js/ui/workspace.js-            // Let the top-most ancestor handle all transients

Don't see issues here as well

./js/ui/windowManager.js-        windows.forEach(window => {
./js/ui/windowManager.js-            // If the window is attached to an ancestor, we don't need/want
./js/ui/windowManager.js-            // to move it
./js/ui/windowManager.js:            if (window.get_transient_for() != null)
./js/ui/windowManager.js-                return;
./js/ui/windowManager.js-            // Same for OR windows
./js/ui/windowManager.js-            if (window.is_override_redirect())
--
./js/ui/windowManager.js-        window.foreach_transient(win => {
./js/ui/windowManager.js-            if (win != ignoreWindow &&
./js/ui/windowManager.js-                win.is_attached_dialog() &&
./js/ui/windowManager.js:                win.get_transient_for() == window) {
./js/ui/windowManager.js-                count++;
./js/ui/windowManager.js-                return false;
./js/ui/windowManager.js-            }
--
./js/ui/windowManager.js-                    return;
./js/ui/windowManager.js-                if (type == Meta.WindowType.MODAL_DIALOG ||
./js/ui/windowManager.js-                    actor._windowType == Meta.WindowType.MODAL_DIALOG) {
./js/ui/windowManager.js:                    let parent = actor.get_meta_window().get_transient_for();
./js/ui/windowManager.js-                    if (parent)
./js/ui/windowManager.js-                        this._checkDimming(parent);
./js/ui/windowManager.js-                }
--
./js/ui/windowManager.js-                actor._windowType = type;
./js/ui/windowManager.js-            });
./js/ui/windowManager.js-        actor.meta_window.connect('unmanaged', window => {
./js/ui/windowManager.js:            let parent = window.get_transient_for();
./js/ui/windowManager.js-            if (parent)
./js/ui/windowManager.js-                this._checkDimming(parent);
./js/ui/windowManager.js-        });
./js/ui/windowManager.js-
./js/ui/windowManager.js-        if (actor.meta_window.is_attached_dialog())
./js/ui/windowManager.js:            this._checkDimming(actor.get_meta_window().get_transient_for());
./js/ui/windowManager.js-
./js/ui/windowManager.js-        let types = [Meta.WindowType.NORMAL,
./js/ui/windowManager.js-                     Meta.WindowType.DIALOG,
--
./js/ui/windowManager.js-        }
./js/ui/windowManager.js-
./js/ui/windowManager.js-        if (window.is_attached_dialog())
./js/ui/windowManager.js:            this._checkDimming(window.get_transient_for(), window);
./js/ui/windowManager.js-
./js/ui/windowManager.js-        let types = [Meta.WindowType.NORMAL,
./js/ui/windowManager.js-                     Meta.WindowType.DIALOG,
--
./js/ui/windowManager.js-            this._destroying.push(actor);
./js/ui/windowManager.js-
./js/ui/windowManager.js-            if (window.is_attached_dialog()) {
./js/ui/windowManager.js:                let parent = window.get_transient_for();
./js/ui/windowManager.js-                actor._parentDestroyId = parent.connect('unmanaged', () => {
./js/ui/windowManager.js-                    actor.remove_all_transitions();
./js/ui/windowManager.js-                    this._destroyWindowDone(shellwm, actor);
--
./js/ui/windowManager.js-
./js/ui/windowManager.js-    _destroyWindowDone(shellwm, actor) {
./js/ui/windowManager.js-        if (this._removeEffect(this._destroying, actor)) {
./js/ui/windowManager.js:            let parent = actor.get_meta_window().get_transient_for();
./js/ui/windowManager.js-            if (parent && actor._parentDestroyId) {
./js/ui/windowManager.js-                parent.disconnect(actor._parentDestroyId);
./js/ui/windowManager.js-                actor._parentDestroyId = 0;
--
./src/shell-window-tracker.c-  MetaWindow *transient_for;
./src/shell-window-tracker.c-  const char *startup_id;
./src/shell-window-tracker.c-
./src/shell-window-tracker.c:  transient_for = meta_window_get_transient_for (window);
./src/shell-window-tracker.c-  if (transient_for != NULL)
./src/shell-window-tracker.c-    return get_app_for_window (tracker, transient_for);
./src/shell-window-tracker.c-
--
./src/shell-window-tracker.c-   *   - 'nautilus' should not appear focused when the DESKTOP has focus
./src/shell-window-tracker.c-   */
./src/shell-window-tracker.c-  while (new_focus_win && meta_window_is_skip_taskbar (new_focus_win))
./src/shell-window-tracker.c:    new_focus_win = meta_window_get_transient_for (new_focus_win);
./src/shell-window-tracker.c-
./src/shell-window-tracker.c-
Edited by Marco Trevisan

Merge request reports