Now that we don't accidentally delay our selection ownership changes,
let's assert() if we somehow find ourselves downloading our own selection,
as the assertion is simpler to understand than the mess that ensues if
we hit this.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
If we don't xcb_flush() when we set the selection owner, we end up with
a ridiculous corner case where we can run use a command line X client
like 'xclip -i -selection clipboard' to crash weston.
Start weston, ensure Xwayland is running (set a selection with xclip), set
the clipboard from a wayland client, then set the clipboard with xclip
again.
Since xclip doesn't do anything xwm notices except set the clipboard, it
won't provoke a flush on our selection ownership change. xclip will take
ownership, then we call xcb_convert_selection(), and THEN we flush, sending
out our pending ownership change and the xcb_convert_selection() request.
The ownership change takes place first, we attempt to get our own selection
and weston explodes in a mess.
Stop this from happening with a flush when changing selection ownership.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
It's possible to set the clipboard with no seat present - one way is to
use the RDP backend and then run 'xclip -i -selection clipboard' locally
without making an RDP connection.
Check if seat is NULL to prevent this from crashing.
Fixes#698
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
It's possible that there are no seats present at startup (especially with
the RDP backend, which creates and removes seats for connections), and
previously we'd just fail to set up XWayland cut and paste properly.
We should set up a listener and find a seat when one becomes available -
but we also need to switch seats if ours is removed.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Co-authored-by: Hideyuki Nagase <hideyukn@microsoft.com>
Rather than creating a new PangoContext each time the menu redraw
handler is triggered re-use it if one was created previously.
All toytoolkit clients do create a layout (and implicitly a
PangoContext) but only those that have menu redraw
handler installed will create a new layout for each redraw of the menu,
effectively creating a new PangoContext each time.
Reported-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Currently we can't tell the difference between a window intentionally
created at 0,0 and a window that we can place anywhere.
Check the size hints to see if the flags indicating the placement
is intentional are present.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
WM_TAKE_FOCUS requires a valid timestamp that isn't XCB_TIME_CURRENT. To
get one, we set a property on the window and wait for the notification
that it was set - this notification comes with a valid timestamp.
Once we have that timestamp, delete the property, and fire off the slightly
delayed WM_TAKE_FOCUS client request.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
We've been doing this when clicking on windows, even if they're
already activated. This leads to sending extra WM_TAKE_FOCUS events
as well as re-rendering the decor every mouse click.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This should be XCB_EVENT_MASK_NO_EVENT, but was not.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
According to https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.7 we should
send this focus notification only if a client has WM_TAKE_FOCUS set in
their WM_PROTOCOLS property. We've been sending it unconditionally.
Rather, we've been not-sending it unconditionally because the event mask
is wrong, but that will be fixed in a future commit. Fixing the event
mask first would break some clients (such as xterm).
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
It's entirely possible, if ridiculous, for an X11 client to change a
window's override redirect flag while it's mapped. If this changes from
true to false we will start receiving Configure requests for the window.
That leads us to a crash when we try to query the window's current
position from the shell to send a configure notify event, as the shell
doesn't know about the surface.
Instead of trying to cleverly handle this, mostly go back to the behaviour
these clients would've seen before commit cf5aca5a and don't send them
a synthetic configure notify.
We also specifically check in weston_wm_handle_configure_request for
the same condition, and early return there, bypassing a couple of
other things we would've done previously.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Commit b18f788e2e76 broke motif applications by ensuring they could never
focus their menus - since then any attempt by an application to focus any
window would be met by the window manager immediately refocusing the
currently active toplevel window.
Later we loosened the restriction in 9e07d25a1b to allow clients that
received focus from a grab to do so - but motif applications like nedit
don't set focus in this way, and remain broken.
This patch further loosens our restrictions, now only reverting a focus
change to an inactive top level. This will hopefully prevent any
confusing input routing without breaking reasonable clients.
This restores functionality to motif menus.
Fixes#636
Fixes b18f788e2e
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
There are two versions of WM_NORMAL_HINTS: the original pre-ICCCM
version (standardised by Xlib itself?) provides 15 elements of 32 bits
each, with the ICCCM v1 extending this by 3 additional elements.
Since the flags are enough to identify which elements are present, and
the structure is append-only, we only need to read the minimum length
between what the user provided and what we support.
Fixes a heap overrun found with ASan.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Many programs use this information to help position pop-ups properly, and
without it funny things happen. For example, nedit and tkinter apps will
position their menus incorrectly either all the time or after an initial
window move, firefox may position right-click pop-ups incorrectly
depending on other internal state.
https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.5 has much detail on
how this should work, and the Advice to Implementors section shows that
common client practices will break in the face of our miserly handling
of ConfigureNotify events.
Instead of trying to send it only for configure requests received when a
client is in a fullscreen state, send them much more frequently.
Fixes#619
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Currently weston_wm_window_send_configurenotify is only called for
fullscreen clients, and it is written to be correct only in that case.
Fix it up to handle other cases properly so we can use it for them in a
later commit. Synthetic Configure Notify events are relative to the
root window, so this means adding our window co-ordinate when
necessary.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
https://specifications.freedesktop.org/wm-spec/1.4/ar01s05.html says
"The Window Manager MUST set _NET_FRAME_EXTENTS to the extents of the
window's frame", so this is probably something we should be doing.
Some programs (such as some versions of Firefox) expect this to be present,
and will render popups in wrong locations if it's not.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Avoid duplication of atom retrieval. This is particuarly useful
if one would one to reuse atom retrival in other parts, like tests.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Daniel Stone <daniel.stone@collabora.com>
According to the wm-spec we must keep the _NET_WM_STATE property updated
to reflect the current state of the window.
This has been biting me when firefox starts maximized, then I click the
maximize button to toggle to unmaximized state. The next time I mouse over
the maximize button (which causes the frame to be re-rendered with the
maximized button in a highlighted state) we re-read the window state and
weston then believes the window is maximized even though it is being
rendered in a not-maximized state.
Update the state when we change maximized status so this doesn't happen.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
If a client starts off maximized, clicking the unmaximize button would
result in a 0x0 window - basically a blob of decor with no content.
Instead, use 512x512 as a totally random default value.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
I guess this reverts commit 73bdc0ce85
"xwm: Fix fd leak in weston_wm_send_data()"
That commit closes the send half of a pipe in weston_wm_send_data,
claiming that it's dup()licated later, and we'll leak the fd if
we don't close it.
That may have been true at the time? But currently that fd is only
duplicated by wl_event_loop_add_fd() in its normal operation, and
closing our original before that fd handler ever fires results
in an EBADF on write, and the data never reaching its intended
destination.
Worse, by the time that handler is called there might be another
use for that fd, and we could push data into it and close it.
To provoke the problem, launch an app like FireFox over Xwayland,
cut something to the clipboard, then close the app (this is the
path where the wm has stored the clipboard contents and the
app has gone away). relaunch it and paste the clipboard content
back in. clipboard_client_data() will EBADF on write, and the
data won't be pasted.
Reported-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Toy toolkit apps already do this since commit 807cd2e589
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
We need to remove our listener link before we free the structure
it's inside, or the signal list walk will try to dereference it.
Remove the improbable NULL check at the same time.
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This leg of the if/else ladder is a duplicate of the previous conditional.
It appears to have been intended to log enter events, but we already
handle those earlier. The drop event is already logged as well,
so let's just discard this branch entirely.
Fixes#552
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
Memleak found by ASAN:
Direct leak of 21 byte(s) in 1 object(s) allocated from:
#0 0x7fe7a917fe8f in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.6+0xa9e8f)
#1 0x7fe7a9129874 (/usr/lib/x86_64-linux-gnu/libasan.so.6+0x53874)
#2 0x7fe7a5a23469 in weston_wm_window_read_properties ../xwayland/window-manager.c:574
#3 0x7fe7a5a28d3b in weston_wm_handle_map_request ../xwayland/window-manager.c:1178
#4 0x7fe7a5a31660 in weston_wm_handle_event ../xwayland/window-manager.c:2291
#5 0x7fe7a8c261a1 in wl_event_loop_dispatch ../src/event-loop.c:1027
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Memleak found by ASAN:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7fe7a917fe8f in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.6+0xa9e8f)
#1 0x7fe7a5a40736 in theme_create ../shared/cairo-util.c:419
#2 0x7fe7a5a3363c in weston_wm_create ../xwayland/window-manager.c:2619
#3 0x7fe7a5a2017e in weston_xwayland_xserver_loaded ../xwayland/launcher.c:313
#4 0x7fe7a90b4d14 in handle_sigusr1 ../compositor/xwayland.c:57
#5 0x7fe7a8c2585d in wl_event_source_signal_dispatch ../src/event-loop.c:685
#6 0x7ffcdb04ef6f ([stack]+0x1df6f)
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
There is more state than just the application window width and height that
affects whether calling weston_wm_window_configure() is necessary: everything
that affects the frame window, fullscreen state in particular. Therefore do not
skip the call by just width and height.
If send_configure() happens to be called "unnecessarily", this will now forward
some of those calls to the X11 clients. However, since it uses an idle task, it
should not result in a flood at least. And if send_configure() is spammed,
maybe that should then be fixed in its callers.
This patch should fix the misplacement of a fullscreen X11 window due to the
frame window being incorrectly sized and positioned, and the app window
incorrectly positioned inside the frame window.
The fullscreen window problems were observed in a case where the window does
not hit legacy_fullscreen() but first maps and then sets
_NET_WM_STATE_FULLSCREEN. Additionally the initial window size must match the
output size where it gets fullscreened. In that case the frame window was left
as if not fullscreened.
This practically reverts 3f53d9179b. I'm not sure
what problem that patch was fixing, but I couldn't make any resizing freeze.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Spotted this in debug log:
[xwm-wm-x11] XWM: configure window 4194324: x=32 y=32 width=1920 height=1080 border_width=0 stack_mode=0
[xwm-wm-x11] XWM: configure window 0: width=1984 height=1144
Trying to configure window 0 makes no sense. So do not try.
To avoid patching two different places with the same thing, refactor the code
into a common helper.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
It would lead to use-after-free if there was a pending idle callback to
weston_wm_window_configure() when the weston_wm_window gets destroyed. Make
sure the callback will not fire.
Found by inspection.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This function is called also directly from weston_wm_window_set_toplevel(). If
configure_source is set at that point, simply resetting the pointer will "leak"
the source until it fires and calls this function again.
Let's keep the variable up-to-date by removing the source when called,
dispatched or not. This removes the second call. I only hope it doesn't cause
issues. This is also necessary if we intend to remove the source on window
destruction too.
Found by inspection.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
It looks like commit ad0da4596d introduced a bug
for X11 windows that are initially fullscreen by adding code to the end of
xserver_map_shell_surface() while ignoring the 'return' that this patch
removes. That may have caused some annoying window state issues, but the
problem became more pronounced with 7ace831ca6
when used with an Xwayland version that honours _XWAYLAND_ALLOW_COMMITS.
In the latter case, there is a possiblity the window will never show up, as XWM
forgets to set allow_commits=true. However, the window may sometimes actually
show up due to an oversight in Xwayland: the Present code may be flipping the
window buffers and not checking _XWAYLAND_ALLOW_COMMITS if it is supposed
commit at all.
Since then, f568968f8a added more places where
allow_commits is set to true, masking the window-does-not-show-up issue. Window
pending state likely still remained an issue.
This patch properly fixes the "window never appears" issue by making sure
allow_commit=true is set. At the same time, it ensures the pending state
functions are called at the end of xserver_map_shell_surface(), which may fix
some window state issues like misplaced decorations and/or position of
initially-fullscreen windows. Unfortunately, it certainly does not fix all such
state problems.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Commit "weston-log: add function to avoid direct
access to compositor members in non-core code" added the
function weston_compositor_add_log_scope mainly to allow
libweston users to avoid direct accessing core structs, as
weston_compositor.
Replace weston_log_context_add_log_scope usage by
weston_compositor_add_log_scope.
Signed-off-by: Leandro Ribeiro <leandrohr@riseup.net>
Fixes missing prototypes compilation warnings emitted when a function
is defined before its prototype is declared.
These warnings were introduced over time since the switch to meson
because the -Wmissing-protoypes was not included in the compilation
arguments.
Signed-off-by: Guillaume Champagne <champagne.guillaume.c@gmail.com>
There's a function named weston_compositor_log_scope_destroy()
but it doesn't take a struct weston_compositor argument.
Rename it to weston_log_scope_destroy(), as the argument is a
struct weston_log_scope.
Signed-off-by: Leandro Ribeiro <leandrohr@riseup.net>
There's a function named weston_compositor_add_log_scope()
but it doesn't take a struct weston_compositor argument.
Rename it to weston_log_ctx_add_log_scope(), as
the log_scope is being added to a log_context.
Also, bump libweston_major to 9.
Signed-off-by: Leandro Ribeiro <leandrohr@riseup.net>
The mask argument is uint16_t so declare the variable with the same.
Suggested-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Knowing the kind of decoration drawn will help track down issues with
unexpected decorations.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Print the changes to the debug scope, helping to figure out why Xwayland is or
is not committing.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Add the missing newline to printing a property that is of type cardinal array.
Fixes messed up debug scope output.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Initially, `_XWAYLAND_ALLOW_COMMITS` was introduced in commit 7ace831ca
to avoid drawing the window content before it's ready to be shown.
But a repaint might also be triggered by the client damages before the
XWM has finished drawing its window decorations and drop shadows, which
previously was not too much of an issue since the XWM could still
finish updating the X11 window after the buffer was submitted.
However, with the addition of multiple window buffers in Xwayland [1]
which are aimed at preventing the X11 clients from updating the buffer
after it's been committed, this is no longer possible.
As a result, the use of multiple window buffers in Xwayland can cause
ugly repainting effects of the decorations if the buffer is submitted
before the XWM has finished painting its decorations.
Use the X11 property `_XWAYLAND_ALLOW_COMMITS` can be used to avoid
this, by controlling when Xwayland should commit changes to the Wayland
surface.
[1] https://gitlab.freedesktop.org/xorg/xserver/merge_requests/316
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
This introduces a new convention of checking through the compositor destroy
listener if the plugin is already initialized. If the plugin is already
initialized, then the plugin entry function succeeds as a no-op. This makes it
safe to load the same plugin multiple times in a running compositor.
Currently module loading functions return failure if a plugin is already
loaded, but that will change in the future. Therefore we need this other method
of ensuring we do not double-initialize a plugin which would lead to list
corruptions the very least.
All plugins are converted to use the new helper, except:
- those that do not have a destroy listener already, and
- hmi-controller which does the same open-coded as the common code pattern
did not fit there.
Plugins should always have a compositor destroy listener registered since they
very least allocate a struct to hold their data. Hence omissions are
highlighted in code.
Backends do not need this because weston_compositor_load_backend() already
protects against double-init. GL-renderer does not export a standard module
init function so cannot be initialized the usual way and therefore is not
vulnerable to double-init.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
