GNOME Bugzilla – Bug 104023
using CurrentTime for _NET_WM_PING in response to _NET_CLOSE
Last modified: 2004-12-22 21:47:04 UTC
The two snippets below amount to a problem. When we get a _NET_CLOSE we attempt to send a _NET_WM_PING to the window with timestamp == CurrentTime but meta_display_ping_window doesn't allow this because CurrentTime is no good for uniquely identifying pongs. I'll attach a test app to play around with the various behaviours that metacity handles when closing a window. ====== /* I think the wm spec should maybe put a time * in this message, CurrentTime here is sort of * bogus. But it rarely matters most likely. */ meta_window_delete (window, meta_display_get_current_time (window->display)); ===== if (timestamp == CurrentTime) { meta_warning ("Tried to ping a window with CurrentTime! Not allowed.\n"); return; } =====
Created attachment 13713 [details] test app
I think on wm-spec-list I requested a timestamp in _NET_CLOSE, need to be sure that's in the spec and implement in libwnck.
Created attachment 28268 [details] [review] Metacity fix I've added the timestamp in _NET_CLOSE_WINDOW now, and here's the metacity patch.
Created attachment 28269 [details] [review] libwnck fix And here's the libwnck fix.
metacity patch looks great, please commit. libwnck patch seems to have a bunch of extra stuff in it? For the part that seems relevant, I'd much rather add a timestamp arg to wnck_close and then pass in the event timestamp. The display-roundtrip mess is grotty and likely to cause races. Thanks!
Created attachment 28902 [details] [review] New libwnck patch I've committed the metacity patch and here's the new one for libwnck. I've looked around and it seems that nothing external uses wnck_window_close so we should probably be safe ABI/API wise.
Created attachment 28903 [details] [review] New patch Here's another metacity patch that passes the right timestamp when using the window menu to close a window.
Comment on attachment 28902 [details] [review] New libwnck patch Looks good
Comment on attachment 28903 [details] [review] New patch Thanks, looks good.
Both patches commited, I guess we can close this now