After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 104023 - using CurrentTime for _NET_WM_PING in response to _NET_CLOSE
using CurrentTime for _NET_WM_PING in response to _NET_CLOSE
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: EWMH specification
2.4.x
Other Linux
: High normal
: METACITY2.8.x
Assigned To: Metacity maintainers list
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2003-01-21 02:33 UTC by Mark McLoughlin
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: ---
GNOME version: 2.3/2.4


Attachments
test app (5.62 KB, text/plain)
2003-01-21 02:34 UTC, Mark McLoughlin
  Details
Metacity fix (1.45 KB, patch)
2004-06-02 12:10 UTC, Anders Carlsson
accepted-commit_now Details | Review
libwnck fix (7.02 KB, patch)
2004-06-02 12:11 UTC, Anders Carlsson
needs-work Details | Review
New libwnck patch (4.10 KB, patch)
2004-06-21 10:16 UTC, Anders Carlsson
accepted-commit_now Details | Review
New patch (3.25 KB, patch)
2004-06-21 10:29 UTC, Anders Carlsson
accepted-commit_now Details | Review

Description Mark McLoughlin 2003-01-21 02:33:55 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;
    }
=====
Comment 1 Mark McLoughlin 2003-01-21 02:34:57 UTC
Created attachment 13713 [details]
test app
Comment 2 Havoc Pennington 2003-02-25 22:20:11 UTC
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.
Comment 3 Anders Carlsson 2004-06-02 12:10:30 UTC
Created attachment 28268 [details] [review]
Metacity fix

I've added the timestamp in _NET_CLOSE_WINDOW now, and here's the metacity
patch.
Comment 4 Anders Carlsson 2004-06-02 12:11:15 UTC
Created attachment 28269 [details] [review]
libwnck fix

And here's the libwnck fix.
Comment 5 Havoc Pennington 2004-06-18 00:39:48 UTC
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!
Comment 6 Anders Carlsson 2004-06-21 10:16:36 UTC
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.
Comment 7 Anders Carlsson 2004-06-21 10:29:38 UTC
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 8 Havoc Pennington 2004-06-21 15:28:35 UTC
Comment on attachment 28902 [details] [review]
New libwnck patch

Looks good
Comment 9 Havoc Pennington 2004-06-21 15:29:21 UTC
Comment on attachment 28903 [details] [review]
New patch

Thanks, looks good.
Comment 10 Anders Carlsson 2004-06-21 16:48:52 UTC
Both patches commited, I guess we can close this now