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 152952 - Support _NET_WM_TAKE_ACTIVITY EWMH extension
Support _NET_WM_TAKE_ACTIVITY EWMH extension
Status: RESOLVED OBSOLETE
Product: metacity
Classification: Other
Component: general
2.8.x
Other All
: Normal enhancement
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
: 352567 499667 (view as bug list)
Depends on: 154260
Blocks: 155451
 
 
Reported: 2004-09-17 20:30 UTC by Elijah Newren
Modified: 2018-02-10 10:46 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
gtk proof of concept patch (against gtk-2-4 branch) (6.24 KB, patch)
2004-09-21 19:38 UTC, Elijah Newren
none Details | Review
metacity proof of concept patch (11.66 KB, patch)
2004-09-21 19:38 UTC, Elijah Newren
none Details | Review
Don't focus window upon ButtonPress; wait for take-activity pong message (11.59 KB, patch)
2004-09-28 20:27 UTC, Elijah Newren
rejected Details | Review
Support _NET_WM_MOUSE_ACTION (instead of TAKE_ACTIVITY) (15.26 KB, patch)
2004-10-11 16:24 UTC, Elijah Newren
none Details | Review
New version of support for _NET_WM_MOUSE_ACTION with a small fix (15.20 KB, patch)
2004-10-11 18:03 UTC, Elijah Newren
none Details | Review
Updated support for _NET_WM_MOUSE_ACTION (18.99 KB, patch)
2004-10-12 21:02 UTC, Elijah Newren
none Details | Review
Provide just a little more debugging information (20.52 KB, patch)
2004-10-13 17:02 UTC, Elijah Newren
needs-work Details | Review
Patch updated to HEAD (20.20 KB, patch)
2006-05-08 23:59 UTC, Elijah Newren
reviewed Details | Review

Description Elijah Newren 2004-09-17 20:30:30 UTC
Lubos introduced the _NET_WM_TAKE_ACTIVITY extension for EWMH in order to handle
special cases when raising on click shouldn't happen.  His email detailing how
the extension works can be found at
http://mail.gnome.org/archives/wm-spec-list/2004-April/msg00013.html
along with an example patch showing the how the toolkit side of things works.

I have worked on this a little bit, and will post patches when I get a little
further along (may be a while if bug 152000 keeps causing so many problems)
Comment 1 Elijah Newren 2004-09-21 19:38:15 UTC
Created attachment 31794 [details] [review]
gtk proof of concept patch (against gtk-2-4 branch)
Comment 2 Elijah Newren 2004-09-21 19:38:44 UTC
Created attachment 31795 [details] [review]
metacity proof of concept patch
Comment 3 Elijah Newren 2004-09-21 19:40:17 UTC
The above two patches are a proof of concept implementing _NET_WM_TAKE_ACTIVITY
support.  I consider them to just be proof of concept, because while they work,
there were something things I didn't understand about the spec (so I may have
implemented it incorrectly), and there's some other issues or ugliness in the
patches.  (I'm filing the gtk+ patches here because of this; once I understand
some of the below questions better, I'll file a real gtk+ bug).  In particular,
I'm wondering:

From the spec: "data.l[2] = the respective client window", and "The Window
Manager can uniquely identify the message by the timestamp and the data.l[2]
field if necessary.  The Client MUST NOT alter any field in the event other than
the window".  How can the WM uniquely identify the message using the data.l[2]
field, if that field is allowed to change?  Am I missing something?  

I don't see why the window field (data.l[2]) is useful at all.  Although I pass
something for that field, both my Metacity and Gtk+ patches ignore it.

Again, from the spec: "Note however that that the Window Manager should not do
anything if such operations have meanwhile taken place for another window (e.g.
if a new Client window has been mapped as a result of the click)."  I'm not sure
how this comes into play and I haven't taken it into account.  I guess the
example given makes sense, but what other cases are relevant?

My Gtk+ patches hard code the drag and drop threshold.  That's kind of lame, but
I thought calling a gtk function, like gtk_drag_check_threshold, from gdk was
taboo.  Besides, I haven't a clue what I'd pass for the widget parameter.

Should we only raise windows on ButtonRelease for frame clicks?  That would seem
to be more consistent to do that, but I haven't bothered with that yet.

For applications that don't support this spec, it'd be nice to do something
clever to emulate it.  Unfortunately, metacity doesn't get ButtonRelease events
for apps (I'm guessing due to a grab).  This also means we can't even fall back
to the dumb behavior of always raise on ButtonRelease.  We have to use the old
behavior of always raise on ButtonPress for such apps.  It makes the interface a
little inconsistent, but that's probably unavoidable.
Comment 4 Elijah Newren 2004-09-27 19:22:19 UTC
In case anyone is trying this out:  In doing further stress testing of my patch
for bug 152000 (which I still haven't posted yet), I happened to be running with
these patches too--as I have been doing  since I created them.  I haven't run
across any glitches for mouse or sloppy focus--they work great.  However, I did
find two issues for click-to-focus:

1) If a window is focused but not raised (which is possible since my patch
focuses on ButtonPress and then conditionally raises later depending on whether
a take_activity return message is received), then subsequent clicks won't raise
(apparently because take activity messages aren't sent in click-to-focus when
the window has focus).

2) If a window is focused but not raised, it's location in the MRU list changes.
 It looks odd to have a window at the front of the MRU list but not be  on top
of the other windows.

I'll fix these issues in the next couple days.

Rob, Havoc: do either of you have any answers to my questions in comment 3?
Comment 5 Elijah Newren 2004-09-28 20:27:38 UTC
Created attachment 32039 [details] [review]
Don't focus window upon ButtonPress; wait for take-activity pong message

This fixes the two issues I mentioned above that affected click-to-focus.
Comment 6 Elijah Newren 2004-09-30 23:05:11 UTC
Note that this protocol also has an additional possible use for us.  There are a
number of times when we don't want clicking on the panel to focus it, yet we do
need to focus it on click in at least a few cases (e.g. dictionary or command
line applets).  This _NET_WM_TAKE_ACTIVITY stuff allows the client to determine
not only when to raise but also when to focus (because we don't do either unless
we receive a pong message from the app).  So, if gdk provides a function to set
display_x11->tracking_take_activity_info to FALSE, then all we'd have left to do
is to patch gnome-panel to call that gdk function whenever we don't want clicks
to result in the panel getting focus.

So, Rob & Havoc--any comments on this?  In order to do that, we'd have to get it
done before the gtk+ API freeze (the "API slush-freeze" is a little over two
weeks away).  I can put in the time, but I'd like to hear your comments on
whether I should try that and on any thoughts you might have regarding my
questions in comment 3.
Comment 7 Rob Adams 2004-10-01 02:06:04 UTC
That sounds like a great idea to me, assuming Mark agrees.  Though of course we
can put the support into gtk and metacity and the panel can support it later, so
I guess it's if gtk maintainers agree, which they will since its in the spec.

Maybe you could send me an email with a list of your pending patches (in the
order that you want them reviewed) and we can get those in there as necessary,
then branch, then try to add the take_activity stuff and the focus stealing
prevention stuff.

This is a really busy time for me right now at work though so I don't think I'll
be able to write any code to help you out but I think I'll be able to be pretty
responsive on patches at least.  So if you can put in the work I think it would
be a great improvement.  We also have those remaining focus stealing problems
left, since it was a disappointment to everyone, and most especially you Elijah,
that we couldn't get that into 2.8.
Comment 8 Rob Adams 2004-10-01 02:06:55 UTC
also, do you have a gtk bug open about this somewhere?
Comment 9 Elijah Newren 2004-10-01 18:17:27 UTC
Comment on attachment 31794 [details] [review]
gtk proof of concept patch (against gtk-2-4 branch)

I just submitted an updated version against gtk+ HEAD as bug 154260, so I'm
marking this patch as obsolete now.
Comment 10 Havoc Pennington 2004-10-03 17:48:05 UTC
> How can the WM uniquely identify the message using the data.l[2]
> field, if that field is allowed to change?  Am I missing something?

The "window field" is the window the event is sent to, rather than 
data.l[2]

> I don't see why the window field (data.l[2]) is useful at all.  Although I pass
> something for that field, both my Metacity and Gtk+ patches ignore it.

It would be used if you had two events with the same timestamp, perhaps
theoretically possible. In current metacity the process_pong function should
probably be matching both window and timestamp instead of just timestamp.

> I guess the
> example given makes sense, but what other cases are relevant?

I would say that if we call meta_window_focus() again while a TAKE_ACTIVITY is
pending, we should cancel the pending one, as a start.

> Should we only raise windows on ButtonRelease for frame clicks? 

There was some old bug suggesting this. I don't think this is the effect we want
in GTK; we want to raise on button press almost always (gtk would normally
immediately reply to the TAKE_ACTIVITY), but raise on button release *if you
click on an area that may be dragged*. That's what Windows does too.
It will seem clunky to users I believe if we always wait for the release,
instead of only waiting for release when we aren't sure whether we'll focus.


Comment 11 Elijah Newren 2004-10-03 19:12:18 UTC
> The "window field" is the window the event is sent to, rather than 
> data.l[2]

If the ButtonPress and ButtonRelease events aren't in the same window, why are
we raising it?  And what is "it" (i.e. which of the two windows do we raise)?

> It would be used if you had two events with the same timestamp

What if both events occurred for the same window?  Just throw one away?

> I would say that if we call meta_window_focus() again while a TAKE_ACTIVITY
> is pending, we should cancel the pending one, as a start.

Would that introduce a race condition?  Consider the following:
  Basic idea: User clicks and releases in window A and then in window B
  More detail:
  1) User clicks in window A
  2) Metacity sends take-activity message to app running window A
  3) User releases mouse button just barely after clicking
  4) User quickly moves mouse to window B
  5) User clicks on window B
  6) Metacity sends take-activity message to app runing window B
  7) Metacity finally receives take-activity pong from app runing window A
  8) Metacity raises and focuses window A, and deletes take-activity
     message for B as a side effect of meta_window_focus

> raise on button release *if you click on an area that may be dragged*

The window can be dragged around.  I guess my question was whether we should
have the window raised when it's being moved.  Yeah, that does make sense. 
Thinking of it in those terms, I totally agree with you for the general case
because the vast majority of users (i.e. people who aren't weird like me) don't
like working with an obscured object.
Comment 12 Elijah Newren 2004-10-10 20:06:21 UTC
> The "window field" is the window the event is sent to, rather than 
> data.l[2]

Um, I just noticed something else.  Isn't the event sent from the client to the
root window?  That's how I'm reading what Lubos wrote...
Comment 13 Elijah Newren 2004-10-11 16:24:32 UTC
Created attachment 32476 [details] [review]
Support _NET_WM_MOUSE_ACTION (instead of TAKE_ACTIVITY)

See the 14th additional comment to bug 154260 and/or
http://mail.gnome.org/archives/wm-spec-list/2004-October/msg00008.html to learn
more about why I prefer this method to TAKE_ACTIVITY.
Comment 14 Elijah Newren 2004-10-11 16:29:13 UTC
Ooops, forgot to mention two things about the MOUSE_ACTION stuff:

1) This patch uses Olivier's idea from the wm-spec-list (for text selection,
raise on ButtonPress, then lower back to normal position upon ButtonRelease). 
I'm not sure I like that, but this patch exists partially to show how easy
things like this are to do.  (Though my implementation is slightly incomplete;
for sheer simplicity since it's proof-of-concept, I just lowered the window to
the bottom of the stack instead of lowering it to its original position)

2) Perhaps I should only queue a mouse-action thingy if the window isn't raised;
the patch above doesn't do that.
Comment 15 Elijah Newren 2004-10-11 18:03:30 UTC
Created attachment 32483 [details] [review]
New version of support for _NET_WM_MOUSE_ACTION with a small fix

I found a small bug.  This patch is almost the same as the previous one; the
only difference is that I replaced the second occurrence of
  else if (button_release_event_with_motion)
in the SELECTION section of process_mouse_action_message with
  else if (button_release_event_without_motion)
Comment 16 Elijah Newren 2004-10-11 18:45:54 UTC
I believe I figured out my confusion regarding the usage of the window in the
TAKE_ACTIVITY spec.  The words "the respective client window" appeared in two
places in the spec and Lubos was only referring to changing one of them (namely
the window to which the XSendEvent message should be sent).
Comment 17 Elijah Newren 2004-10-12 21:02:19 UTC
Created attachment 32529 [details] [review]
Updated support for _NET_WM_MOUSE_ACTION

The MOUSE_ACTION proposal has undergone some changes due to comments from
Lubos.	The newest proposal, at
  http://mail.gnome.org/archives/wm-spec-list/2004-October/msg00014.html
hasn't been commented on yet, but I'm guessing it won't be too different from
the final version.  This patch implements the newest proposal fully and
provides debugging information (mostly to aid in making patches to various
widgets and to make sure those patches are correct).
Comment 18 Elijah Newren 2004-10-12 21:15:22 UTC
Oh, I forgot to mention one thing.  Bug 152952 has a new patch that helps with
debugging, especially with finding widgets that aren't calling
gdk_window_begin_mouse_action() and friends but need to.
Comment 19 Elijah Newren 2004-10-12 21:16:00 UTC
Doh!  Sorry, I meant to paste that last comment in bug 154260...
Comment 20 Elijah Newren 2004-10-13 17:02:32 UTC
Created attachment 32565 [details] [review]
Provide just a little more debugging information

This patch makes it easier to catch widgets that aren't yet supported or to
track down bugs in their support.  I'm using this while trying to generate the
necessary gtk+, vte, nautilus, etc. patches and debugging them.  I thought I'd
attach it here because gtk+ API slush freeze is today--I'm guessing due to the
lack of response so far (in both bugzilla and on wm-spec-list) that these
patches won't be able to go in so I'm just going to attach the latest of
everything I have before this all gets shelved for gtk+-2.8
Comment 21 Elijah Newren 2004-10-13 17:41:30 UTC
Oh, one more thing I forgot.  Neither TAKE_ACTIVITY nor MOUSE_ACTION allow for
distinguishing between a drop with the target window equal to the source window
and a drop with the target and source windows being different.  That effectively
means that drag and drops to and from the same window result in that window not
being raised.  Is that what we want, or do we need a more thorough proposal?

I'm actually assuming this is fine, because we need a separate proposal that
allows for raising a window when it's a potential drop target.  We would just
need to make sure that it can raise on EnterNotify if the window is different
than the source window, and raise on ButtonRelease if the window equals the
source window.  Not that I know how to accomplish that or know if it's feasible...
Comment 22 Elijah Newren 2004-10-13 19:08:16 UTC
Oh, I was wrong.  gtk+ API slush-freeze is still two days away...

Also, I'm going to mark this bug as depending on 154260.  Technically it doesn't
depend on it, but there probably isn't much point in adopting this patch if gtk+
doesn't support the feature.
Comment 23 Elijah Newren 2005-02-25 21:28:52 UTC
Comment on attachment 32039 [details] [review]
Don't focus window upon ButtonPress; wait for take-activity pong message

This patch no longer applies, and further the _NET_WM_TAKE_ACTIVITY proposal
puts clients in charge of policy, which isn't what we want.
Comment 24 Elijah Newren 2005-02-25 21:30:52 UTC
Comment on attachment 32565 [details] [review]
Provide just a little more debugging information

This patch needs updating, as it no longer applies to HEAD.
Comment 25 Elijah Newren 2006-05-08 23:59:07 UTC
Created attachment 65057 [details] [review]
Patch updated to HEAD

See extra notes in bug 154260 comment 28 for changes that should probably affect this patch too
Comment 26 Elijah Newren 2006-08-23 16:15:44 UTC
*** Bug 352567 has been marked as a duplicate of this bug. ***
Comment 27 Elijah Newren 2007-04-09 05:55:53 UTC
Pulling it off the patch-review list.
Comment 28 Elijah Newren 2007-11-26 04:17:00 UTC
*** Bug 499667 has been marked as a duplicate of this bug. ***
Comment 29 dsargeant 2008-03-08 04:53:19 UTC
I'd like to help fix this; is there anything I can do?
Comment 30 Thomas Thurman 2009-03-09 16:33:08 UTC
Hi dsargeant, good to have your help!

Per Elijah's comments at

http://blogs.gnome.org/metacity/2008/06/11/drag-and-drop/

his patch in comment 25 should implement the "sane subset" which is as much as is needed of the spec.  It has probably rotted, however, so a first step would be to clean it up and apply it to trunk.  Once that's done I'll check it in and we'll see about getting the equivalent done to GTK.