fix(wl_data_device): drop and leave event handling #409
Conversation
wash2
force-pushed
the
fix-data-device-leave
branch
2 times, most recently
from
43321ba
to
251321d
Compare
examples/data_device.rs
Outdated
| @@ -860,6 +862,7 @@ impl DataSourceHandler for DataDeviceWindow { | |||
| _qh: &QueueHandle<Self>, | |||
| source: &wayland_client::protocol::wl_data_source::WlDataSource, | |||
| ) { | |||
| println!("FINISHED"); | |||
There was a problem hiding this comment.
Looks like a debug leftover, given how it's written compared to other logging we have around.
There was a problem hiding this comment.
I just copied the message above it, but I can change how both messages are written.
So far I was able to recreate the issue in Cosmic, sway, and Pop on wayland (Gnome), but I can double check vanilla Gnome too |
That's more than enough I guess, just mention that in the commit. We could probably use some RAII so users won't worry about that, but I'm not familiar with dnd myself much and can just look into the code. |
| pub(crate) fn drop_performed(&self) { | ||
| let mut inner = self.inner.lock().unwrap(); | ||
|
|
||
| if let DataDeviceOffer::Drag(ref mut o) = inner.deref_mut().offer { | ||
| o.dropped = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Do we really need this as a separate function if it's used only once in Drop handler?
| if !self.left || self.dropped { | ||
| receive(&self.data_offer, mime_type) |
There was a problem hiding this comment.
Could you document why it's like that?
There was a problem hiding this comment.
I've added a comment that explains why
wash2
force-pushed
the
fix-data-device-leave
branch
from
be3196d
to
48d4613
Compare
I was able to recreate the issue in Cosmic, sway, and Pop on wayland (Gnome) refactor(data-device): merge both cases in leave Co-authored-by: Kirill Chibisov <contact@kchibisov.com> chore(data-device): document that the offer may need to be manually destroyed after leave cleanup(data-device): example println capitalization
wash2
force-pushed
the
fix-data-device-leave
branch
from
48d4613
to
502b1bc
Compare
While the spec says to destroy the wl_data_offer after wl_data_device::leave, I think compositors often send the leave event right away after a drop. This is a bit problematic if an application hasn't finished receiving the data and then the data offer is destroyed before it can make the finish request. I believe this can currently be recreated in the data_device example. The source never receives a finish event because the offer was destroyed and isn't alive by the time we make the request.
This PR tries to avoid destroying an offer after the leave event if a drop event already occurred. It also tries to avoid making request besides finish or destroy.