Re: TAKE_ACTIVITY patch

[Elijah Newren <newren gmail com>]

On Mon, 11 Oct 2004 17:00:13 +0200, Lubos Lunak <l lunak suse cz> wrote:
> On Monday 11 of October 2004 07:01, Elijah Newren wrote:

First of all, I found my bug and I got it working (freeing a node in a
linked list, yet still leaving that node in the list isn't good--thank
goodness for Valgrind).  I can now easily switch between the following
behaviors with just a few lines of code in the window manager:
  1) - For MOUSE_ACTION_NONE, focus on ButtonPress for MOUSE_ACTION_NONE
     - For everything else, act like TAKE_ACTIVITY.
  2) - For MOUSE_ACTION_DND, act like TAKE_ACTIVITY.
     - For MOUSE_ACTION_SELECTION, do what Olivier wanted (raise on
       ButtonPress, if threshold mouse motion then lower on
       ButtonRelease, if not threshold mouse motion then focus on
       ButtonRelease)
     - For everything else, do what Rob wants (raise on ButtonPress,
       not on ButtonRelease)
 
I could mix-and-match the two or even get more complicated behavior
easily, and do it all within the window manager.  I don't yet have all
widgets handled, but adding them is easy.  (Getting nautilus to work
with my spec proposal was a two-liner)

> > C) Olivier wants to be able to raise upon ButtonPress at the beginning
> >    of text selection and then lower upon ButtonRelease of text
> >    selection if the mouse has moved.  I don't think I'd like that.
> >    TAKE_ACTIVITY can't enable this currently, and further, it places
> >    the decision making into the toolkits instead of the window
> >    manager.
> 
>  I think you misunderstand Olivier's proposal at
> http://mail.gnome.org/archives/wm-spec-list/2004-April/msg00025.html (that,
> or I have misunderstood it the same way). As I read it, the app sends one
> message back to WM right after it finds out which operation is triggered by
> the press, and sends a second after release. That should make it both
> policy-free and allow what you want.

Um, I don't see how I misunderstood his proposal, and I don't see how
what you say here is different than what I have done.  Could you
elaborate?

> >   3) Whenever the client receives a ButtonRelease event, it must send
> >      a _NET_WM_MOUSE_ACTION message to the root window.  The message
> >      contains the timestamp of the ButtonPress & window (again, so the
> >      WM can uniquely identify the queued info), the timestamp of the
> >      ButtonRelease, the type of mouse action, and whether there was
> >      "threshold" mouse motion between ButtonPress and ButtonRelease.
> 
>  Why should the threshold be necessary? If there wasn't any such motion, then
> there wasn't any action -> say the mouse action was none.

I believe you are suggesting a number of choices that encode WM policy
into the toolkit, instead of allowing the window manager to dictate
policy.  I believe this is one such case.  The reason I believe this
wouldn't work is the following:

  Writers of window manager A only ever want to raise on ButtonRelease.

  Writers of window manager B want to raise on ButtonPress for
  MOUSE_ACTION_NONE events, and otherwise want to raise on ButtonRelease.

So, the writers of B ignore MOUSE_ACTION_NONE events for
ButtonRelease.  Writers of A use them to raise the window.  As far as
I can tell, what you suggest prevents the possibility of both choices
by having the toolkit specify.  (And, I'm afraid that will lead to
inconsistent toolkits as toolkit writers will have differing opinions
on what should be done)

My spec avoids this by saying that a ButtonPress message with
MOUSE_ACTION_DND is matched later by a ButtonRelease message with
MOUSE_ACTION_DND.  The threshold part is necessary because if the
mouse didn't move enough, the action wasn't really a drag-and-drop.

> >   MISC:      A mouse click-and-drag action not listed below (use of this
> >              should be avoided; it is preferred to add an extra type
> >              after gaining consensus on the wm-spec-list for extending
> >              the _NET_WM_MOUSE_ACTION section of the EWMH.  It is
> >              suggested that this type only be used while waiting for
> >              consensus and an extension)
> 
>  This probably shouldn't have so strong wording. Getting every single widget
> which can handle mouse press+move+release to report correct type can be a lot
> of work. My Qt patch doing only the old way (i.e. only MISC) is a couple of
> lines, supporting all this won't be. Maybe only supporting MISC should be
> called a simple support or so? After all, I don't know about others, but I'd
> only rarely need this feature, so I wonder if developers would consider this
> to be worth the trouble.

If you let toolkits just use MISC, then you are again allowing
toolkits to specify policy.  Rob wants to raise on ButtonPress for
SLIDING movement, and he wants to avoid raising on ButtonPress for
DND.  If you use MISC for everything, he can't differentiate and this
spec becomes unuseful to him.

I really think this isn't anywhere near as difficult as
_NET_WM_USER_TIME.  If you are merely worried about work, I'll even
offer to help out with the KDE & QT side of the patch (I may be a bit
slow getting started, not being familiar with KDE and QT though).

> > Define STATUS
> >   STATUS = 3*ACTION_TYPE + EVENT_TYPE
> 
>  I'd like to see this at least split to bitfields, instead of such strange
> maths. But I think this is not necessary, one more field could be saved by
> having different message types in the X11 ClientMessage for every event type.
> IMHO that's a bit cleaner.

I thought about different message types for every action type.  But I
think it's suboptimal when considering extensibility.  You see, the WM
waits for the client to send a MOUSE_ACTION message in order to
determine whether to raise/lower/focus/whatever (it has to wait unless
it just wants to always raise on ButtonPress--but in that case the
window manager won't need to support any part of this spec anyway). 
Now, if you add a new message type, then old window managers that
don't know about it won't be able to receive and act on these
messages--meaning that neither button presses nor button releases will
result in the window being raised.  It's much better to have a backup
default behavior--which my spec provides via MISC (and the requirement
that unknown statuses be treated as MISC).

I can get rid of the combination of ACTION_TYPE and STATUS_TYPE into a
single event field (and instead use two fields) if we remove the
window field of the TAKE_ACTIVITY spec (or any other field...).  I
sort of like that idea anyway, as I don't really understand the window
field and don't know how to make use of it.  (See
http://bugzilla.gnome.org/show_bug.cgi?id=152952)

> > So, does this sound reasonable?  Would it be useful with some
> > modifications (and with what modifications)?
> 
>  Besides what I already commented above, this looks fine to me.

I'll try to generate a patch to the EWMH with my proposal.  I agree
that the combination of ACTION_TYPE and EVENT_TYPE is a little weird,
but perhaps you or others can help me see how to free up a slot so
that they can be separate fields within the ClientMessage.


Thanks,
Elijah