This allows identifying XWayland views using the ICCCM "Globally Active"
input model. Later commits will improve handling of these views.
No functional change in this commit.
The unmap() handlers should only call desktop_focus_topmost_view() if
the unmapped view was the focused view. Unmapping a view that was not
focused should not change the focus.
I expect this rarely had any effect in practice; it would only matter in
a focus-follows-mouse config where some view other than the one on top
was focused. But it still seems better to fix.
Rather than repeating the logic in two places, create a small
view_impl_unmap() helper. Perhaps more common "unmap" logic could be
moved there in future.
xsurface->data is presumed to be a (struct view *) if set, so it must be
left NULL for an unmanaged surface. Otherwise view_from_wlr_surface()
may return a (struct xwayland_unmanaged *) where a (struct view *) was
expected, leading to a crash.
valgrind backtrace:
Invalid read of size 1
at 0x48F8FFC: wlr_scene_node_set_enabled (wlr_scene.c:903)
by 0x124C9F: ssd_set_active (ssd.c:323)
by 0x124C9F: ssd_set_active (ssd.c:318)
by 0x124C9F: view_set_activated (view.c:215)
by 0x118851: focus_change_notify (seat.c:353)
by 0x487E01D: wl_signal_emit_mutable (wayland-server.c:2241)
by 0x48FC8F2: wlr_seat_keyboard_enter (wlr_seat_keyboard.c:298)
by 0x119E60: seat_focus.lto_priv.0 (seat.c:473)
by 0x1248FD: seat_focus_surface (seat.c:491)
by 0x1248FD: unmanaged_handle_map (xwayland-unmanaged.c:51)
by 0x487E01D: wl_signal_emit_mutable (wayland-server.c:2241)
by 0x487E01D: wl_signal_emit_mutable (wayland-server.c:2241)
by 0x490FC91: surface_commit_state (wlr_compositor.c:499)
by 0x56A24F5: ffi_call_unix64 (unix64.S:104)
by 0x569EF5D: ffi_call_int.lto_priv.0 (ffi64.c:673)
Address 0xa0e15ff30788b68 is not stack'd, malloc'd or (recently) free'd
Fixes: 4028a9482f
("seat: use focus_change event to update focused/active view")
XWayland views can self-declare that they don't want keyboard focus via
the ICCCM WM_HINTS property. Most of the logic is already in place to
avoid giving focus to such views (e.g. taskbars).
Add a couple of missing pieces to make this work:
- Hook up view_isfocusable() to look at WM_HINTS for XWayland views
- Adjust desktop_focus_topmost_mapped_view() to skip unfocusable views
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.
Our current approach to handling the focused/active view is a bit
confusing. In particular, it's hard to be sure when server->focused_view
is or isn't in sync with the real wlroots keyboard focus.
Try to clean things up a bit. In particular:
- Add comments to server->focused_view and desktop_focused_view() to
clarify that they should match, but it's not guaranteed.
- desktop_focused_view() now prints a warning if it detects that
server->focused_view is out of sync. We should keep an eye out for
this warning, and if we see it, try to figure out why it happened.
- For consistency, use only "focus/defocus" as the verbs in function
names rather than "activate". This is a bit arbitrary, but the idea is
that focus is the primary action while the active/inactive state is a
side effect.
- view_focus/defocus() replace view_set_activated() and now update both
focus and active/inactive state, to try to keep them in sync.
- Add comments at view_focus/defocus() to warn against calling them
directly (we should generally call the desktop.c functions).
- desktop_focus_view(NULL) is now forbidden and is no longer handled as
a special case to clear the focus. This was (at least to me) a
surprising behavior and caused trouble when working on another change.
- To maintain existing behavior, desktop_focus_topmost_mapped_view() now
explicitly clears the focus if there are no mapped views.
There should be no behavioral change here.
For views with a non-pixel size increment (e.g. X11 terminals), it's
helpful to subtract the base size of the window (typically including
menu bar, scrollbars, etc.) before computing the number of size
increments (e.g. cells/characters). This way, the displayed size will
exactly match the terminal grid (e.g. 80x25 or whatever).
wlr_box isn't really the best fit for size hints, so let's define a
struct view_size_hints and a nice view_get_size_hints() function,
wrapping view->impl->get_size_hints().
This also seems like a great opportunity to make view_adjust_size()
window-system-agnostic and eliminate xwayland_apply_size_hints().
When a view is destroyed (including override_redirect in the xwayland
case), the view_destroy() handler is called which checks for a currently
open A-Tab window switcher and causes an update there to remove the
destroying view from the list. Before view_destroy() is called, both
xwayland and xdg handlers reset the xdg_surface / xwayland_surface.
The window switcher update then creates a list of all windows which do
not have the 'skipWindowSwitcher' window rule property set. If there is
at least one 'matchOnce' window rule configured, this also tries to get
string properties of the destroying view which already had their
xdg_surface / xwayland_surface reset and thus run into an assert.
This patch fixes that so that the string_prop() handlers always return
an empty string in those cases rather than running into the assert.
For a more in-depth analyses and alternative solutions see the linked
issue.
Fixes#1082
...to share common code with minimize_sub_views()
Also, fix a bug in the move-to-back functions to move the window
hierarchy in the right order.
Helped-by: @Consolatis
Minimize the whole view-hierarchy from top to bottom regardless of which
one in the hierarchy requested the minimize. For example, if an 'About' or
'Open File' dialog is minimized, its toplevel is minimized also, and vice
versa.
For reference:
- This is consistent with in openbox, where child views (dialogs) can be
minimized, but when doing so the parent is also minimized.
- In mutter these types of dialogs cannot be minimized (via client-menu or
otherwise).
- In both openbox and mutter, when a toplevel window is minimized any open
children are also minimized.
...which may occur if a user minimizes an xwayland view (typically a
child view such as a dialog) at the same time as the client sends a
request-unmap, which xwayland clients sometimes do without actually
requesting destroy and just leave them dangling.
...so that other window cannot be positioned between modal dialogs and
their parent windows. This is consistent with Gtk3 and Qt5 applications on
mutter and openbox.
This makes explicit the subtle behavioral difference between
xwayland_view_unmap() and handle_unmap().
With this change, the XDG and XWayland versions of handle_map/unmap()
are now identical, which will make further refactoring possible.
This is a trivial cleanup to make xwayland_view_create() symmetrical with
xwayland_unmanaged_create(), and avoid the need to access view->impl from
xwayland-unmanaged.c.
The return value of xwayland_view_create() is no longer user, so return void.
No functional change.
Two types of window rules are supported, actions and properties. They are
defined as shown below.
<windowRules>
<!-- Action -->
<windowRule identifier="some-application">
<action name="Maximize"/>
</windowRule>
<!-- Property -->
<windowRule identifier="foo*" serverDecoration="yes|no"/>
</windowRules>
Rules are applied if windows match the criteria defined by the
'identifier' attribute which relates to app_id for native Wayland windows
and WM_CLASS for XWayland clients.
Matching against patterns with '*' (wildcard) and '?' (joker) is
supported.
Add 'serverDecoration' property.
view_minimize() does not need to call desktop_move_to_back() because the
stacking order is not changed and the windowSwitcher uses the scene-tree
nodes anyway.
Note: Movement of xwayland sub-views still relies on keeping server->views
in sync with z-order
Currently, we anchor the right/bottom edge of the view whenever the top/
left edge is moving (current.x/y != pending.x/y). Doing so doesn't make
much sense when the right/bottom edge is also moving. In that case it's
probably best to move the view (or at least its top/left corner)
directly to its final position.
The most noticeable effect of this change is with views that don't
accept their requested size exactly when tiled or maximized (examples:
havoc, xfce4-terminal). Previously, their right-bottom corner would be
aligned with the screen edge, leaving gaps on the left and top. Now the
top-left corner will be aligned and the gaps will be on the right and
bottom. This is still not ideal, but IMHO less surprising to the user.
...to increase xwayland and xdg-shell encapsulation and to avoid passing a
function pointer as an argument in `xwayland_move_sub_views_to_front()`
which is inconsistent with labwc design patterns.
Rename view-impl.c to view-impl-common.c
Move function declarations that are common to view-implementations from
view.h into view-impl-common.h
The fix caused a couple of issues:
1. Ignoring client configure requests caused some clients to hang
and not repaint correctly. We are supposed to synthesize a
ConfigureNotify event when ignore/override a client configure
request, but this isn't possible with current wlroots.
2. Setting view->natural_geometry from client configure requests
resulted in overwriting good values with bad in some cases (e.g.
with tiled xfce4-terminal in xwayland mode).
For now, revert the fix. This does allow clients to mess with view
positioning for maximized/fullscreen/tiled views, but right now the
alternatives seem worse.
The original specific issue (VLC undoing its fullscreen geometry)
is arguably a bug in VLC anyway.
This reverts commit 09599861ac.
view->impl->move() is a specific case of view->impl->configure().
To reduce code duplication, we can use view->impl->configure() for
pure moves (without resize) as well.
xwayland's move() function also possibly contained a race condition
when there was a pending resize, as it used the current surface
width/height rather than the pending width/height. This is fixed.
- Don't overwrite pending size in map() if it was already set
- Don't reposition view in map() if maximized/fullscreen
Also, as future-proofing in case we one day allow initially-tiled views,
replace explicit maximized/fullscreen checks with view_is_floating().
