feat: add drag and drop support inside xdg-popup surfaces.
To my understanding smithay was not aware of the implicit grab on these kind of surfaces because of a missing set_grab in the pointer click handler. When the client then tried to start a drag, smithay would not know of this grab on serial and just ignore it.
This can be tested in the gtk4-widget-factory -> page 3 -> Open button. Inside this popup, there are 2 text input boxes, this text can be dragged and dropped now.
⚠️ I'm not well versed in smithay or wayland protocols, so please double check that I'm not doing something foolish :/
Also fixes https://github.com/YaLTeR/niri/issues/2519 for me
@Drakulix the rejection that I experienced happens here: https://github.com/Smithay/smithay/blob/4fbcc9252088825dbf1e3f900c99ae04df5053d6/src/wayland/selection/data_device/device.rs#L82. Since smithay did not know of an ongoing pointer-grab it doesn't start the dnd.
About the popup grab, I think it has keyboard focus/grab. But not a pointer grab when I hold down left mouse button anywhere on it.
I tested this with debug statements in:
impl<D: SeatHandler + 'static> PointerInnerHandle<'_, D> { among other places.
Leading me to never see pointer-grab being set inside popups.
I then found that the mouse clicks were going to the function I patched.
I can investigate further but I thought this pointer grab being missed was enough reason for why the drag cannot start.
@Drakulix the rejection that I experienced happens here:
https://github.com/Smithay/smithay/blob/4fbcc9252088825dbf1e3f900c99ae04df5053d6/src/wayland/selection/data_device/device.rs#L82 . Since smithay did not know of an ongoing pointer-grab it doesn't start the dnd.
That doesn't necessarily mean, that there is no grab, it might also mean that it doesn't consider the serial valid.
About the popup grab, I think it has keyboard focus/grab. But not a pointer grab when I hold down left mouse button anywhere on it.
I tested this with debug statements in:
impl<D: SeatHandler + 'static> PointerInnerHandle<'_, D> {among other places. Leading me to never see pointer-grab being set inside popups. I then found that the mouse clicks were going to the function I patched.
You patched a function in an implementation of PointerGrab on the popup grab though. That isn't called, if there is no pointer-grab.
You're right I see that another serial has a pointer grab, I'll look into it.
So smithay does indeed have a PopupPointerGrab set for the mouse button click that opened the popup.
2025-10-27T17:00:21.767149Z DEBUG input_seat:add_pointer:input_pointer:set_grab: smithay::input::pointer: Set grab PopupPointerGrab { popup_grab: PopupGrab { root: WlSurface { id: ObjectId(wl_surface@38), version: 6, data: Some(Any { .. }), handle: WeakHandle { handle: WeakInnerHandle[sys] { .. } } }, serial: Serial(941), previous_serial: None, keyboard_grab_start_data: GrabStartData { focus: Some(WlSurface { id: ObjectId(wl_surface@38), version: 6, data: Some(Any { .. }), handle: WeakHandle { handle: WeakInnerHandle[sys] { .. } } }) }, pointer_grab_start_data: GrabStartData { focus: Some("..."), button: 0, location: Point<smithay::utils::geometry::Logical> { x: 0.0, y: 0.0 } } } } for Serial(941) name="winit" serial=Serial(941) focus=Keep
- But when clicking inside the popup, the grab does not get updated.
- Gtk4 sends the serial of the latest mouse button down event for starting its dnd.
- Since clicking inside the popup did not update the grab, smithay does not know that serial and rejects setting a DndGrab at the location I mentioned.
Here's my logs but the smithay logs are mostly cursed print statements that I sprankled around smithay-gtk4-widget-factory-debug.log gtk4-widget-factory-debug.log
So the issue is, that we don't start a ClickGrab inside popup for the last mouse down event, because we already have a PopupGrab, thus the serial isn't matching.
What we want to do is probably relax this check to not require the serial to match, but to be equal or greater. I'll have to think if this has any security implications.
Relaxing the check is also not correct, given that the protocol expects an implicit click grab explicitly and not anything else.
We could potentially get away by internally updating the stored serial of the GrabStatus, but I am not sure what the implications of that might be.
A correct fix would be to track active ClickGrabs separately from any explicitly set grabs or even storing a queue of active grabs instead, but I don't think we want to get down that rabbit hole, if we can avoid it.
@ids1024 @YaLTeR any thoughts on this?
If we leave the ClickGrab issue aside for a moment, what should happen with regards to DnDGrab? Shouldn't it also happen "on top" of the popup grab, and then return back to the popup grab (or not, if the DnD went elsewhere)?
I've been kind of suspicious of the way ClickGrab works and how its mutually exclusive from having any other kind of pointer grab.
But I don't know what would be better. Not sure exactly how other compositors manage this.
If we leave the ClickGrab issue aside for a moment, what should happen with regards to DnDGrab? Shouldn't it also happen "on top" of the popup grab, and then return back to the popup grab (or not, if the DnD went elsewhere)?
Since the data-device protocol doesn't mention focus somewhere, I'd assume so? The alternative is, that start_dnd looses any sort of pointer focus. But the existence of data_device.enter/.motion/.leave suggests to me, that pointer focus is still expected to reside with the original implicit ClickGrab with the DndGrab being applied on top.
In fact I think we are already handling this incorrectly. The ClickGrab is meant to keep the focus on the surface, regardless of motion outside of it for example. So the client is expecting to get motion events, even as the pointer leaves the surfaces as long as a button is pressed. This should also apply on a popup-grab, where only a click outside of the popup is meant to dismiss the popup. Which is not what happens, the popup-grab currently unsets the focus in this case: https://github.com/Smithay/smithay/blob/4fbcc9252088825dbf1e3f900c99ae04df5053d6/src/desktop/wayland/popup/grab.rs#L567
I dislike the fact, that the PopupGrab has to effectively re-implement the ClickGrab semantics, but indeed with dnd-operations on top of PointerGrab this breaks completely, as can't fall back to the previous grab.
I'd argue we thus need to introduce a stack of grabs and also pass the event through all of them. (Which makes writing correct grabs even more complicated.)
Alternatively make implicit click-grabs something more inherently handled by the PointerHandle regardless of an active grab and make any previous grab part of the GrabStartData delegating the responsibility of restoring the previous grab to the new grab.