indeed it is; much thought has been put into this, and the necessary
machinery ought to make it into the next version of the WM
specification.  I could have sworn we already had a bug open for that,
but I guess not.

This will almost certainly be a Gnome 2.6 feature.

*** Bug 114757 has been marked as a duplicate of this bug. ***

bug 82921 has some of the historical discussion of this.

Some info to keep handy.  This is the wm-spec post detailing the
current spec and the KWin/Kicker implementation by Lubos Lunak:
http://mail.gnome.org/archives/wm-spec-list/2003-May/msg00013.html

Also, updating the summary to match what made it into the spec.

I believe that this is basically the same bug as bug 82921
(specifically noting Havoc's 2003-06-09 comment as to "future plans").
 Note that 82921 also brings up some accessibility issues, which will
probably be important to keep in mind.

Also, readams@hmc.edu, was 82921 the bug you were thinking of when you
said that "I could have sworn we already had a bug open for that."?

yea, that was probably it.  I like this bug better though; I hate
scrolling :-)

*** Bug 121530 has been marked as a duplicate of this bug. ***

*** Bug 121557 has been marked as a duplicate of this bug. ***

*** Bug 122471 has been marked as a duplicate of this bug. ***

*** Bug 126506 has been marked as a duplicate of this bug. ***

*** Bug 128556 has been marked as a duplicate of this bug. ***

This patch: http://bugzilla.gnome.org/showattachment.cgi?attach_id=22411
contains some preliminary support.

*** Bug 129799 has been marked as a duplicate of this bug. ***

Just thought I'd post to say I'm working on this bug.  I've taken Robs
patch and extended it.  I got it to work in tandem with bug 115650 so
that apps can set a don't-focus-on-map hint.  It still needs some work
with respect to not focusing a window on map when the user has
interacted with other windows after the initial launch.  I have most
of the infrastructure is in place, but it's not quite there yet.  I'll
post a patch in a while...

There are also some related issues that my testing has brought up in
order to make everything feel integrated and work nicely.  After I've
made a little more progress on the patch, I'll talk more about those
issues.

A little off topic: What I've done also includes a patch for
startup-notification to bring it up to date with the current
freedesktop wm-spec.  I checked bugs.freedesktop.org for libsn or
startup-notification but couldn't find anything.  Does anyone know
where I'd post such a patch?

You could just attach the startup notification stuff here as well,
since I need to look at that too.

Created attachment 23683 [details] [review]
Metacity patch to support _NET_WM_USER_TIME and TIMESTAMP hints, not quite complete

Created attachment 23684 [details] [review]
Startup-notification patch to support TIMESTAMP in wm-spec

I have lots of questions/comments about the patches and how to
implement this in general; some of this may even need to be brought up
on the wm-spec list:

 * The Metacity patch uses meta_warning's instead of meta_verbose's.
   This is only _temporary_ to help me with debugging and will be
   changed when I have a final version of the patch.
 * I don't have any support for KeyPress events, because Metacity 
   currently doesn't grab those.  I need to figure where/how/when to 
   add the key grab so Metacity can update _NET_WM_USER_TIME for
   those events as well.  (and perhaps KeyRelease, but that's
   supposedly against the spec)
 * For my startup-notification patch, is my method for obtaining a
   timestamp okay? (It might be ugly, but I dunno since I don't
   really understand what's going on and just cobbled it together
   from multiple sources)
 * Rob had both Metacity and GTK updating the _NET_WM_USER_TIME hints
   in his preliminary patches, but I think that just Metacity should 
   do it (that way it works for all applications and doesn't cause 
   redundant work for gtk apps).  However, can this handle XInput 
   events or does that need to be done in GTK?
 * Do we need to delete the extra X properties we are storing for the
   windows? That would seem to make sense, but I don't think I did
   so.
 * Should I used guint32 instead of guint? The X values are CARD32,
   IIRC, but I seem to use a mixture of unsigned long, unsigned int, 
   gulong, and guint, I think.  But then again, it seems Metacity and 
   libsn do that too.
 * Does the previous use of TIMESTAMP in metacity/src/display.c
   conflict with the wm_spec (or at least make it confusing since
   that means TIMESTAMP serves more than one purpose)?
 * In sloppy/mouse focus, if the window appears where the mouse is 
   then the window gets focus anyway (due to the mouse entering the
   window, not due to it being focused on map).  In click-to-focus
   modes, when can have an application obscuring what the user does
   if the new app isn't focused.  We need to decide whether this is 
   correct.  If we don't focus the window, we either run the risk of
   obscuring whatever the user is doing in another window (if it
   comes up on top) or else not being noticed at all by the user (if
   it starts out minimized or mapped below other windows).
 * Should we try to prevent windows from raising or focusing
   themselves or others if they aren't the current active app? KDE 
   does this, and it seems to make sense.
 * Both of the previous two issues could mean we need a way of 
   handling _NET_WM_STATE_DEMANDS_ATTENTION. Actually, we should
   handle it anyway since it is in the spec. But the last two items
   (especially in the cases of error dialogs) seem to help motivate
   why and how it's needed.
 * Can we convince the KDE guys to change _KDE_NET_USER_CREATION_TIME
   -> _NET_USER_CREATION_TIME and then use it as well? It seems that
   doing so would be helpful since not all apps would be launched
   with startup notification (I'm specifically thinking of things 
   launched via a gnome-terminal but I believe there are other cases 
   too).
 * Clicking on the "Desktop" window (which the user won't view as a
   window) can prevent a launched app from getting focus.  I'm not so
   sure that's what we want. The same is true for the panel, but that
   makes more sense to me.
 * Do I need to deal with the other issues Lubos mentioned (session
   saving (has two issues--both restoring & saving), whether the
   window belongs to another already running application, whether to
   deal with apps being raised, whether apps can attempt to
   raise/focus themselves or others when they aren't active, etc.)

I'm adding the PATCH and security keywords.  (Note that windows being
focused upon launch can cause apps such as GAIM to end up receiving
and sending a typed password.  Telsa reported on #gnome that she got
Alan's password accidentally once that way.  Also, error dialogs could
be missed if the user hits the enter key right when it appears)

Also, I'm trying to keep track of all the issues (there were many that
came up, mostly related to Metacity, and I was having a hard time
keeping track of links to all the appropriate locations) at
http://www.math.utah.edu/~newren/prevent-focus-stealing.html.  That
may be useful to others who run across this bug report and need to try
to track down the wm-spec files or wants to see all the gtk & metacity
& startup-notification (& maybe gnome-panel?) issues listed in one place.

I just noticed that my previous comments may have given the wrong
impression.  My Metacity patch is functional (and it works for all the
tests I have thought of), it's just not complete (it's missing
updating windows upon key press, and it may benefit from
_NET_WM_STATE_DEMANDS_ATTENTION and perhaps other additions as well).

I think that you're misunderstanding the wm specification here.  It is
the _application_ that should update the USER_TIME property on _its
own_ top level windows.  The window manager is then able to use this
time to gauge when the last user input timestamp was for that
particular window.

There is code in the partial patch that I did for metacity to also try
to keep track of soem subset of input events for windows that don't
support the specification.  The idea is to try to at least get it
"mostly right" but metacity does not and can not see every possible
user input event for all the applications, so this is really just a
workaround that happens to work for most windows.

The idea is that all windows on the display will keep their user time
timestamps updated, and also that new windows will specify the user
interaction timestamp that caused the window to be created.  This way,
metacity can compare the interaction timestamp on the currently
focused window to the one for the new window, and then make the policy
decision as to whether the new window should be focused.

Finally, the specification indicates that KeyRelease and ButtonRelease
events should not update the timestamp since windows can receive
suprious buttonrelease and keyrelease events from things like global
keybindings -- and this shouldn't really count as user input.

I don't know the details of XInput in gdk -- someone will have to
figure that out eventually.  I mentioned some sort of global variable
in gdk, incidentally, because gdk will have to be able to compute the
most recent timestamp that created the newly created window, and this
variable will need to be application scoped, not window scoped.  I
don't know what the best way in the gdk codebase to acheive this is.

Ah, that makes a _lot_ more sense about the setting of the time. 
You're right, I did misunderstand the spec and thus misunderstood what
you had done.  I'll update by gtk patch to reincorporate that.
 
I think I understand the KeyRelease stuff better--not really the why's
or the how's of how spurious events occur but rather what should be
done.  Apparently part of my misunderstanding came in thinking about a
user holding a key down and forgetting that X sends multiple
ButtonPress events during such a case.  Ooops...  Anyway, thanks for
straightening me out on that.  I did read that part of the spec (more
than once too), but somehow forgot it.

I'm still a confused about the global variable for gtk part.  I even
went and re-read the spec documents I could find and multiple emails
from the wm-spec list.  It isn't clicking for some reason.  Maybe I
can explain what I understand and am thinking and you can tell me what
I'm missing:

My understanding of the basic idea:  Any window the user doesn't
expect shouldn't be focused.  (Which needs to be handled by having the
application set a hint to not focus such windows.)  Windows that the
user expects should be focused, unless between the time of that
window's launch and that window's map the user interacts with a
different window.  (This is achieved by setting and comparing timestamps).

Now, in the case of a totally new window, startup-notification is
responsible for setting that window's timestamp (though I would like
gtk to be responsible for an additional timestamp in case SN is not
used, which is what my former comment about KDE_NET_USER_CREATION_TIME
is about).  This can be compared to the focus windows
_NET_WM_USER_TIME timestamp.  Thus, the global gtk variable would be
unneeded for this.  

If that app then opens another window that the user expects, then most
every case I can think of would result in the window appearing near
instantaneously; since no timestamp is available from that window to
compare to any _NET_WM_USER timestamp, the window would get focus
which is exactly what I expect.  The only case it shouldn't get focus
would be if the window took a while to launch and the user decided to
do something else.  The only cases I can think of where there would be
enough time for that to happen is for apps like
nautilus/epiphany/mozilla/konqueror (& maybe OpenOffice?) where
launching those apps a second time don't really launch a second
instance of the application.  But even in this case I don't see how to
global variable helps.  Let me explain why, using the example of Mozilla:

Case 1: If the user launches one copy of Mozilla, works with it for a
while, does something else, and then launches a second Mozilla from
the command line, then the global variable timestamp for Mozilla will
be old and will result in the new Mozilla not getting focus when it
should.
Case 2: If the user launches one copy of Mozilla, and then opens up a
new window with Ctrl+N, then the global variable timestamp for Mozilla
equals the last _NET_WM_USER_TIME timestamp.  Do we decide in such
cases to focus the new window so that equality favors focusing new
windows?  Makes sense in this case, but...
Case 3: If the user launches one copy of Mozilla, possibly does some
stuff elsewhere, launches a second Mozilla from the command line, and
then decides to interact with the first Mozilla while waiting for the
second to launch, then both the global variable timestamp and the
_NET_WM_USER_TIME timestamp get updated and become equal again, yet in
this case we do _not_ want the new window to get focus.  You may think
this a strange case (and I'd agree with you), but let me note that
it's not so strange if you think about Nautilus instead of Mozilla.

So, that's all the cases I can think of.  What am I missing?

Two extra notes, however: (1) Lubos talked a lot in one of his emails
about "startup notification forwarding" for the case of apps like
nautilus/mozilla/konqueror/epiphany so that those cases can work
(though he doesn't explain how such forwarding supposedly works, at
least not that I could see).  (2) Since nautilus functions both as a
"window" in terms that the user thinks of and as "desktop", this could
confuse things even more in terms of an application global timestamp.

*** Bug 132543 has been marked as a duplicate of this bug. ***

There are really two places that metacity could get information about
the user interaction that results in a new window: the startup
notification spec and the _NET_WM_USER_TIME property.  It's fairly
straightforward how the startup notification end of it works, so let's
look at the USER_TIME property.

Before the toolkit maps the window, it has the opportunity to set the
_NET_WM_USER_TIME to the timestamp of the user interaction that
ostenstibly caused the window to appear.  Take a gdk application for
example.  The normal case is that you click on a button that causes a
dialog box to be launched.  Early in the window configuration process,
gdk would look at an application-scoped variable that holds the
timestamp of the most recent direct user interaction.  Assuming a
single-threaded application, this variable would have the timestamp of
the user-interaction that caused the window to be launched.  It's at
this point that the toolkit uses this variable to set the value of the
property _NET_WM_USER_TIME.

Of course, once the property is set changing the value of the variable
from which that value is copied won't affect how metacity sees that
variable...

This makes the algorithm for metacity very very simple:  If a window
is mapped, look at the most recent timestamp for the currently focused
window, and the most recent timestamp for the newly-mapped window. 
Focus the window whose timestamp is latest.  And, of course, server
timestamps can wrap around...

If you're still having trouble with this, I can just go ahead and
finish the patch.  It seems quite likely that I'll have time to do so
before Gnome 2.8 feature freeze, which is the soonest that this
feature could make an appearance, since we're in feature freeze
already for Gnome 2.6.

Your sentence that "Before the toolkit maps the window, it has the
opportunity to set the _NET_WM_USER_TIME to the timestamp of the user
interaction that ostenstibly caused the window to appear." is I think
the key I missed/misunderstood.  I had considered that before, but
according to <a href=
"http://freedesktop.org/Standards/wm-spec/1.3/ar01s05.html#id2768662">
this wording</a>, it seemed to me that doing this would violate the
spec.  The specific wording that confused me was, "Clients should
start setting the [_NET_WM_USER_TIME] property only after receiving
the first event from user interaction, they shouldn't set it before
receiving first input event."  Perhaps related to my confusion was the
fact that Lubos said he had a _NET_WM_USER_TIME property, a TIMESTAMP
property (for startup-notification), AND a
_KDE_NET_USER_CREATION_TIME.  From what I could tell, his description
of _KDE_NET_USER_CREATION_TIME matches what you just said
_NET_WM_USER_TIME would be used for.

I hope I'm not being a pain.  I was hoping to help and also have a
little fun learning exercise.  If I'm getting in the way more than
helping, let me know and I'll stop bugging you.  If it's not too much
trouble, though, I would like to try to continue so I can have my own
patched version of Gnome 2.6 that has this cool feature for my own use.

Regardless of whether I continue, however, I'd like to point out that
there is one serious issue that I haven't seen addressed: Preventing
new windows from being focused can mean that the new window obscures
the window that is focused (meaning the user couldn't see what they
are typing).  For sloppy/mouse focus this even has a problem in that
something must be done to prevent the new window from getting focus
anyway (the mouse will "enter the window" after it is mapped, causing
it to get the focus even if Metacity doesn't explicitly focus it
initially).  How should this be handled?  (I'm guess by supporting
_NET_WM_STATE_DEMANDS_ATTENTION, but what behavior do people expect
from that--a minimized window that flashes in the taskbar?)

By all means continue -- you could be a very nice asset to metacity
development.

I would say that we probably don't need to worry too much about
sloppy/mouse focus.  If we prevent a window from being focused, note
that we would also want to prevent it from being raised as well, so
that it would pop up _behind_ the currently focused window.

I had intended to implement something like the _NEEDS_ATTENTION KDE
extension and then propose it for inclusion in the spec, but that is
actually kind of separate and I would suggest that we get the rest of
the functionality working first.

Ok, I'll do that.  For some reason, it's taken me quite a bit of time
to wrap my head around concepts and terminology in GTK+ and X11;
thanks for being patient with all my questions.  :-)

Your idea of having the new window popup behind the focused one would
solve both the obscuring problem and the issue with the new window
getting focus anyway for sloppy/mouse focus (actually, it may only
solve it for mouse focus and there may be more problems for sloppy
focus, but as you say, we can worry about that later).  I'll do that.

Also, the KDE extension about demanding/needing attention
(_NET_WM_STATE_DEMANDS_ATTENTION) is already in the spec -- Lubos
committed it to freedesktop's icccm-extensions/wm-spec/wm-spec.xml on
16 Dec of last year.  So I guess that'll save you a little bit of
work.  :)

Finally, I've updated the patches a little bit (including the xserver
wraparound you mentioned), but I'll just keep them on my webpage that
I gave the link for earlier until I've made some more significant changes.

*** Bug 134763 has been marked as a duplicate of this bug. ***

Windows XP flashes the window and the taskbar item for the programs
that are trying to steal focus. Stealing focus should be entirely
blocked and not allowed. Flashing of the window should occur instead.
Programs stealing focus or raising themselves is intrusive and
uneccessary.

My comment in bug 134763:

When programs attempt to steal focus, they should be flashed within
the gnome taskbar and the windows should be flashed. This behavior is
what Windows XP does and its very useful.

Let's say I'm typing within a text editor, and someone sends me a
message with GAIM. Let's pretend that you can't turn off the r"aise
window on event" in GAIM or don't want to (in fact, turning this
behavior off seems to still cause windows to be raised for the first
message someone sends). So I'm typing away, and suddenly the window
pops up and I press enter to complete the line without reacting fast
enough and i send this junk I was writing in the text editor to the
person who's sending the message to me. Or even worse, it has to do
with them and I didn't want them to see it. Anyway, that's one
example. Another is when say a program finishes a task
and tries to raise itself and you don't want it to.

In Windows XP, the window is flashed, and not raised. This is a less
intrusive behavior but still noticeable. You could also add an
optional sound for accessibility reasons (embrace and improve).

If you are still not convinced, turn off popup blocking in Mozilla and
go here (be ready to kill the mozilla process, if you can):
http://www.albinoblacksheep.com/flash/you.html

What happens is the stupid little windows keep popping up and stealing
focus and they are all moving, so they are stealing focus when they
move, and you have to keep clicking in the console to type what's
necessary to kill this application.

Brian: Yes, we know.  That's why this bug exists and is still
open--because we're working on fixing it.

This bug is closely related to bug #131705 . In that report I mention
a way to fall-back to a good timestamp for initial mapping when apps
either don't use startup-notification or use it incorrectly.

Elijah, if the patches to gnome-desktop and nautilus on your page
are up-to-date, then both modules are using it incorrectly. They
should not be using CurrentTime as the last arg to
sn_launcher_context_initiate(). It is that timestamp that should be reported as the TIMESTAMP in the startup-notification spec. The
initiating apps should be passing the timestamp from the causative
X event to libsn. (Are there bugs against them?)

On the "prevent focus stealing" page, in the misc. section: the
TIMESTAMP in metacity/src/display.c is one of the required selection
targets for all selection owners, including metacity which owns WM_Sn
(where 'n' is a screen number). It's not related to TIMESTAMP used in
the startup-notification spec.

At this level of code, a timestamp of 0 pretty much means there is
no timestamp, so you don't need a second struct member. libsn itself
doesn't need a valid timestamp, it's just a medium for the info.
It's the listeners that need a valid time, so that, for example,
the WM has one to set focus with. The WM, seeing 0, can fall back to
the timestamp from the PropertyNotify generated by setting WM_STATE.

So there are three timestamps for mapping focus. The PropertyNotify
is the ultimate fallback. The startup-notification TIMESTAMP can be
used for a launched app's first mapping. Then _NET_WM_USER_TIME for
any mapping.

If the timestamp in a SetInputFocus request is prior to the last
SetInputFocus timestamp, or later than the current server time, then
the X server doesn't change focus. As long as you only pass valid
timestamps, there shouldn't be much checking to do in the WM.

By the way, it's not really "stealing" focus, when it's the WM
giving it at map time. ;-)  (There's really nothing to be done for
actual focus stealing - i.e., an app taking focus in violation of
the ICCCM. X just doesn't work that way, and people breaking the
rules will just break some other rule to work around attempts to
stop them.)

By mapping below the top of the stack and raising on real FocusIn
events, there's less to do to ensure that a mapped-but-not-focused
window doesn't obscure the focused window. But that's another thing
that won't work so well for sloppy focus.

Sorry, that's bug #131785 that's closely related.

Gregory: Thanks for the comments.  They helped clarify a number of
things for me (I'm still trying to get up to speed on X programming,
among other things), especially regarding the different uses of
'TIMESTAMP' and the use of WM_STATE PropertyNotify for a fallback.  I
do have some further questions/comments:
 
Regarding my patches to gnome-desktop and nautilus: You said that
"They should not be using CurrentTime as the last arg to
sn_launcher_context_initiate()."  Either I'm really confused or you
perhaps you read the patch backwards, because my patch was made in
order to get rid of the use of CurrentTime as the last argument.  So,
didn't I do what you said needs to be done?  

I haven't opened bugs against gnome-desktop and nautilus yet, because
my patches relied on changes to startup-notification.  I wanted to
hear first whether those changes were sane; after all, there's no use
making a fool of myself more than I have to.  :)
 
The reason I had a second struct member was that I was thinking about
the case of Xserver time wraparound.  It may be really rare, but
wouldn't it be better to handle that case than just ignoring it?  Or
does the Xserver time wraparound skip the value of 0?

Also, as a side note, the use of the phrase "Preventing focus
stealing" was actually stolen from the subject title of Lubos Lunak's
emails to the wm-spec list.  I couldn't think of a better title for
that page.  *shrug*

You did remove the use of CurrentTime, but there's a better way.
When an app calls sn_launcher_context_initiate() it should already
have a valid timestamp to pass as the last arg. So I'm saying:
leave that function's args as they are and fix the apps that call
the function so they pass in the timestamp. Setting a property and
waiting for PropertyNotify should only be a last resort, since it's
expensive (adds round-trips). Looking at the code currently in
gnome-desktop, I fear fixing this will be very tedious - it requires
an API change.

The X server should not be giving a time of 0, as that is the same
as CurrentTime. Afaik, you only need to worry about wrap-around
when comparing timestamps; see gtkclipboard.c, grep for "wrap".
(I may well be wrong. I've never had the "about 49.7 days" of
 X server uptime needed to run into the problem.)

*** Bug 135786 has been marked as a duplicate of this bug. ***

Filed bug #136278 for libgnome-desktop.

*** Bug 136918 has been marked as a duplicate of this bug. ***

*** Bug 137744 has been marked as a duplicate of this bug. ***

There is a subtle difference between allowing new windows to open in
the background and allowing metacity to not focus new windows
automatically.

Currently, metacity seems to (at least from a user's viewpoint)
*actively* give focus to new windows. This is dangerous.

The KDE way is to allow new windows to open in the background, i.e.,
*actively* move new windows to the background to avoid them receiving
focus. This is very safe, but the user might be confused the first few
times this happens.

The way older window managers (e.g., Window Maker) do it is to *not
automatically* give focus to new windows; there is no active effort to
either give them focus or avoid them receiving focus. If an app (e.g.,
yudit) gives itself focus (which IMHO is broken behaviour), the app
will have its say and the window manager won't bother to take the
focus back from it. This is safe most of the time, until a broken app
(e.g., yudit) comes along.

If the KDE way is difficult to do, IMHO at least the Window Maker way
should be implemented.

Ambrose: Yes, we understand the difference (see bug 82921).  This bug
fix should be included in Gnome 2.8 (code exists to handle most of
this already).

Am I right in thinking that the fix for this bug will result in an application
having to supply a hint that says "do not focus this window", or would the
ability to prevent any window grabbing focus (say when entering a username or
password somewhere) be a different bug?

Sorry for the confusion.

Such a hint needs to be provided, but it's handled transparently by the toolkit.
 This is supported (or will be supported) by Qt and Gtk+. Very few applications
will actually need to be modified.

Rob: Some of this discussion is beyond me and I'm just interested in this from a
user's perspective. I'm still not sure on what the answer to the question in
comment #40 is. Will a window still get raised/focused unless the application
supplies some hint? Or will an application get raised/focused ony if it supplies
a hint? 

By transparently, do you mean that without the hint, the application
window will not be raised or focused? If so, will  this also apply to when a
window hasexisted for a while and there is new activity (such as a gaim window
that was minimized popping up while you are typing an essay in openoffice)? If
not, it would be a long time before programs passed the hint as the programmers
would need to be aware, i.e. nothing would change.

Will the windows be flashed along with the entries in the window list?

Will this fix affect the sawfish Window manager too, or do they need independent
fixes? It'd be nice if it could be fixed for every window manager.

In the absence of the hint, the window manager will try to use values from
startup notification and then fall back to simply focusing the new window.

Primarily lack of the hint will be noticed in that applications not supporting
the hint will have their focus occasionally stolen _from them_, though metacity
will make a best effort to fill in the missing information based on events that
it notices going to the window.

We'll implement also some sort of visual display of the _NET_DEMANDS_ATTENTION
hint in the window list, likely flashing briefly the GTK highlight color.

Finally, the metacity implementation will not affect sawfish, certainly, but
there is nothing stopping sawfish from implementing the same standard EWMH hints
that metacity is implementing.  KWin, the KDE window manager, already implements
these hints, and the KWin implementation for these hints should at this point be
considered the reference implementation.  This will allow interoperation between
KDE and GNOME apps, and other window managers and toolkits implementing the EWMH.

I'm not a Gnome developer. I just want, as a user, a certain thing to stop
happening. Namely, windows raising themselves and stealing focus.

In practical terms, from a user's perspective, let's say this fix were on our
current systems with everything else left the same, would Gaim pop up a Window
while you are typing in Open Office? Or if a program pops up some dialog warning
when you are typing in Open Office, or even worse playing a game? That's the
kind of thing I want to stop. In my opinion there is no reason a Window ever has
to raise itself or steal focus. Perhaps very critical errors is one exception,
but in my opinion in 99% of the cases flashing is all that is needed. The only
case that it is necessary in my opinion is when its something we HAVE to see,
like a message that your system is about to reboot due to some fatal hard drive
controller error.

I don't understand what is meant by "use values from startup notification". I do
develop, but I'm not a Gnome developer and these terms are very Gnome specific
it seems.

Basically, this is what I want:

When you start an application, the application window usually comes to the front
. After that, if you switch to another application, it should never again bring
itself to the front (or steal focus as imho only raised windows should be
allowed to have focus). If the main application window of an application takes a
long time to appear, and you go back to working in another window, it should not
bring itself up when it finally displays. Instead, in both cases, it should
flash itself.

That's what I'm looking for, in plain English, and that's what the bug I filed
that got duped to this one was about. If that's not what is occuring, I'll
undupe my bug. I just need to know if this is basically what's happening -- in
layman's terms.

Yes, your description is exactly what we want to implement.

Excellent :-)
Thanks.

*** Bug 139860 has been marked as a duplicate of this bug. ***

Created attachment 26662 [details] [review]
Patch to support _NET_WM_USER_TIME

There's two issues with this patch: (1) It doesn't handle applications which
don't set the _NET_WM_USER_TIME hint themselves upon keypresses (yes,
applications are supposed to handle this (usually via their toolkit), but some
applications such as emacs don't).  (2) I need to replace meta_warning with
meta_topic in a number of places.

I'll take care of (2) once someone decides what should be done for (1).  For
reference, KWin simply ignores the problem.  I guess that's a valid choice, but
it might make using emacs painful (though I hear they have gtk2 love in CVS
which I hope will fix that problem for me).

Created attachment 26664 [details] [review]
Startup notification patch needed for _NET_WM_USER_TIME support

This is nearly the same as the previous startup-notification patch.  I just
updated it to current CVS.  I haven't attempted to address Gregory's issues
with it; I'm just providing it so that anyone who wants to test the related gtk
patch in bug 115650 under Gnome can do so.

(Note that this patch is needed for the Metacity patch to work since the
Metacity patch contains a call to sn_startup_sequence_get_timestamp.)

A nice touch for this problem might be to look at the starttime field in
/proc/<pid>. The idea is when a new window arrives to look at the start time for
the pid owning the window. If that is before the USER_TIME of any other window,
we know that the user has interacted with another window after spawning the new
window. In that case the new window shouldn't get focus. This should take of the
"start emacs from a terminal and keep typing" problem.

The starttime as reported in /proc is in jiffies, so there would have to be a
way to convert that to X timestamps (at least an approximation).

Oooh, cool idea.  I guess that'd be Linux specific, but this would handle a
major use-case of mine that isn't currently handled.

/me heads off to Google on /proc & starttime...

I found a web page describing the fields in /proc/<pid>/stat for linux 2.4.17. 
It listed the starttime field as the 20th entry in that "file".  By looking at
the source code for the version of the kernel I'm using (2.4.22), I found that
it was the 22nd entry in that "file".  Since these fields aren't labelled and I
can't see how to determine which field is the correct one at run time (without
access to the kernel source anyway), I don't think this is going to work.

I think that even with that value converting that to a server timestamp would in
practice be impossible.

According to "man proc" it's the 22nd entry. I know ps uses /proc on linux, so
it should be possible to use in a way that is relatively portable. (I would be
surprised if the contents weren't parsable independent of at least the minor
kernel version, as that would make the "file" essentially useless").

Note that the conversion to a server timestamp wouldn't have to be exact - as
long as we could make it precise enough to say "ok, the user _definitely_
interacted with some other window", we should be fine. The server timestamp is
in "milliseconds since some time in the past", so if metacity on startup checks
the current server timestamp, it should be possible to calculate a pretty
accurate approximation.

A different problem is, what do you do if the user does something that makes an
 "old" process pop up a new window? If the user didn't do anything else, you
would generally want that window focused, but to metacity it would look like the
user had done lots of interaction with lots of other windows since the process
started.

To fix that, we could look at some of the cases:

   - is it a dialog/property window/some other inferior window, with a
     focused parent? Then focus it.

   - did we expect the window to appear because of startup notification?
     Then focus it depending on the USER_TIME vs. startup notification

   - otherwise the window appeared out of nowhere. This could mean
     several things:

           (a) some IM-like program decided to pop up a window.

           (b) a user typed "emacs &" into a terminal

           (c) user did something to make an "old" process create a new
               window

For the "out of nowhere" cases I think the algorithm:

- "is the window !focus_on_map"  -> don't focus
- if the process is older than USER_TIME, but not by more than say
  30 seconds, then don't focus it
- otherwise focus

could work pretty well in practice. Case (a) would be hopefully be handled by
the "!focus on map". Case (b) would be handled provided Emacs doesn't take more
than 30 seconds to start. Case (c) would be handled provided the "old" program
is older than 30 seconds.

There would probably be both false positives and false negatives, but I think
they would be rare, and the consequences are never disastrous, especially if you
compare to the current situation where every "emacs &" is a false negative.

Soren: You seem to be making the same mistakes I was when I was first trying to
understand this.  Your question, "what do you do if the user does something that
makes an "old" process pop up a new window?", is misguided.  USER_TIME is not a
global value, but rather a per-process value.  And it is the application's
responsibility to track it, not the window manager's.  (Though the window
manager may try to fill in missing values for non-compliant applications)  So if
a user does something to an "old" process to make it pop up a new window, then
that app should update its timestamp (having the side effect that it's no longer
considered an "old" process).  If the app doesn't do this, then the application
itself is buggy.

Your only difference here is your suggestion for case (b) (everything else is
already handled).  But the root problem is not the one you mentioned, but rather
the problem is that the new window doesn't have a "launch time" associated with
it.  Metacity won't have a user_time for new applications, and if it doesn't
have a launch time either (normally provided by startup notification), then it
can't do anything but punt, really.  There's not even a way to determine whether
the app is "not more than 30 seconds" older than the most recent user_time.

If we could get an approximate launch time, (such as grepping through the man
pages for proc and then reading /proc/<pid>/stat), then we should be able to fix
this problem.  But the fix shouldn't involve a fudge factor, let alone a 30
second one.

Also, let me note, that I doubt the maintainers want a solution that involves
grepping through the man pages or that is kernel-version dependent (as the /proc
stuff appears it might be).  So I hope that someone knows a way that the /proc
stuff could be used that avoids these two issues...

An alternate train of thought that might lead to a solution is, "How can we get
applications launched at the terminal to be launched with startup notification?"

Also, if you want to look at corner cases, consider applications like nautilus,
mozilla or konqueror--they can be launched with startup notification but they
each will notice when another instance is running and then just send a message
to the other instance that it should pop up a new window.  If the other instance
that was running hadn't been used for a while, this can result in unexpected
lack-of-focus.  I don't know what the solution is there.

Note that anyone who wants to discuss these cases should really read Lubos's
emails to the wm-spec list because he covered all of them in excruciating
detail, except for the launched-from-a-terminal case (note that he didn't have a
solution for the nautilus/mozilla/konqueror/epiphany problem either).

I checked the 2.4.17 sources, and starttime is the 22nd entry. I simply don't
believe this userspace API will change in the middle of a stable kernel
series.

I'm not misunderstanding USER_TIME. What I mean by "old" process is "user does
something in one window that makes an entirely different process create a new
window", ie. the nautilus/mozilla/konqueror problem. Metacity behavior
currently is to focus the new window, which is fine (even if Lubos and KDE
disagree), except when the user continues typing.

The problem is if you adobt the starttime algorithm this could happen:

      - a mozilla is running
      - user types "mozilla" into a terminal, and waits for new mozilla window
        (expecting it to be focused since he is not doing anyting)
      - existing mozilla detects this and creates a new window
      - metacity discovers new window and sees its pid is the pid of existing
        mozilla
      - metacity checks starttime and sees that the PID of the new window
        has been running forever and that the user has done lots of
        interaction in lots of windows since mozilla was started
      - consequently metacity decides *not* to focus the new mozilla, thinking
        the new window would steal unwantedly steal the focus.

which is wrong. The existing mozilla is what I call an "old" process, and I
suggest the fix of not doing the "not focus" thing if the "old" process is so
old (older than 30 seconds) that the new window was probably not caused by user
action.

So my suggestion doesn't address focus stealing prevention for process-reusing
applications like mozilla. These application would continue to yank focus away
from the terminal. The solution here, in my opinion, is that such applications
should use startup notification when they create a new window. Ie. when they get
a request to create a new instance of themselves, they should use startup
notification on the new window. The startup notification spec allows that, right?

The 30 second "fudge factor" is needed to distinguish between an "old"
application creating a new window and a "slow to start" process creating a new
window. In the absence of startup notification, these two cases are
indistinguishable except for how long the processes have been running.

Well, I apparently really misunderstood you previously.  Sorry about that. 
Also, I guess the web page I was looking at must have been wrong, because I
didn't check the 2.4.17 kernel sources myself.  That's good to hear.  :)

Now, I think there's still a problem, but I may be misunderstanding you
again--if so, correct me.

Startup notification is not used for windows to launch new instances of
themselves--and it isn't needed.  Since applications keep track of a per-process
user_time, any new windows that are launched will inherit this user_time value.
 Metacity can then compare this user_time value to that of the currently focused
window to determine whether to focus the new window or not.  This is taken care
of in gtk+ at window creation time by the following lines in the patch in bug
115650:

+  else if (GDK_DISPLAY_X11 (screen_x11->display)->user_time != 0)
+    gdk_x11_window_set_user_time(window, GDK_DISPLAY_X11
(screen_x11->display)->user_time);

See comment #23 by Rob in this bug for more information about this specific
issue.  So, the problem you presented can occur whether or not the starttime
algorithm is adopted--the only difference is that without starttime, it can only
occur so long as the user interacts with the initial window at least once.

I also don't understand how your solution involving the 30 second wait correctly
handles the various situations.  Compare the following:
  1) User launches Mozilla and interacts with it
  2) User does a bunch of stuff with other windows
  3) User launches mozilla from the terminal
  4) New Mozilla instance causes old one to create a new window
and
  1) User launches Mozilla and interacts with it
  2) User does a bunch of stuff with other windows
  3) User launches mozilla from the terminal
  4) User quickly types something else in the terminal
  5) New Mozilla instance causes old one to create a new window

I don't see how you're distinguishing between these two cases, yet they clearly
have different expected behavior.  And I don't see how to solve this without
some kind of forwarding of startup notification information being done by Mozilla.

> If the other instance that was running hadn't been used for a while, this can 
> result in unexpected lack-of-focus.

Flashing the window would still not be the end of the world. The user would see
that. It'd be nice if it could detect a new instance was launched of the same
process and then allow the window to be shown, but this is a concession I and
I'm sure a lot of people would be willing to take for now. If they see the
window flash, they'll know something happened, even if the old window wasn't
focused.

> and I suggest the fix of not doing the "not focus" thing if the "old" process
> is so old (older than 30 seconds) that the new window was probably not caused
> by user action.

Doesn't this allow popup windows for web pages to steal focus, new IMs to steal
focus, etc? Windows can pop up in applications without any user interaction.
This is the worst case of windows stealing focus because its totally unexptected.

Brian:

> Flashing the window would still not be the end of the world...

Yes, and that's why this is the currently planned behavior unless we can
correctly solve the cases with startup notification forwarding or whatever.

> Doesn't this allow popup windows for web pages to steal focus...

Applications that create windows that aren't in response to user input should
set a do-not-focus-on-map hint.  If they don't, they should be considered buggy.
 And I agree with you that the unexpected windows are the most important ones to
keep from stealing focus.  I think those were what originally caused the
_NET_WM_USER_TIME spec to be proposed...

I haven't looked at the big picture or the metacity patch yet, but why did you
remove the timestamp arg from sn_launcher_context_initiate()? Isn't it better to
leave the API unchanged, and force people to provide a timestamp?

I don't think the slowly_get_timestamp function really belongs in
libstartup-notification, at least not as a public function; maybe if you don't
set a timestamp it could call that automatically or something. But in real-world
cases (vs. test-launcher) there should be an event->time to use.

Uh, yeah...  I really can't remember why I split it into a different function,
it's been too many months ago.  And I don't see a good reason for it now, either. 

The slowly_get_timestamp thing was brought up by Gregory (I just haven't changed
it, yet, as I got distracted by other stuff).  He filed bug 136278 against
gnome-desktop since it is one of the applications that need to remove their use
of CurrentTime (another bug should also be filed against nautilus for the same
thing).

Elijah, no, my suggestion doesn't handle that case. With my algorithm the new
mozilla would still take focus, just as it does now. It's only in the case where
the new window has a new PID that it would work. The idea is that in cases where
we don't have startup notification, but *do* have a PID for that window that is
(a) younger than 30 seconds and (b) older than last user interaction, we can
make a qualified guess that the user doesn't want it to be focused.

The 30 seconds are there to make sure new mozilla instances get focus like they
do now (and yes, take focus unexpectedly like they do now). Without the 30
seconds, the new mozilla instance would not get focus, because metacity would
see that the new window has both an ancient USER_TIME, copied from the ancient
running mozilla, and an ancient starttime, also copied from the ancient running
mozilla.

Like you, I don't see a way to handle cases where we have neither startup
notification, nor a new PID, nor a sane USER_TIME. 

So, the "mozilla &" case should IMO be handled by making mozilla either use
startup notification or making it explicitly set USER_TIME to CurrentTime when
it sees an external request to create a new window. It could also just use a new
process.

Either of these things would provide the window manager with the necessary
information. But what I suggested does not and can not by itself handle this
situation. It does however handle the "emacs &" case.

We'll want to experiment with default no-focus or default focus if we can't make
the determination because we don't have enough information.  Default no-focus
would certainly encourage app authors to support the hint :-).

So, should the application of the patches in attachments 26662 and 26664 have
some visible effect on this problem?

I checked out/updated both libstartup-notification and metacity from their
respective CVS HEAD revisions, applied the respective patches and built both.  I
then proceeded to start metacity from it's source pool with a "--replace" but I
see no difference in focus behaviour.  I do see the output added by the patch to
metacity though:

Window manager warning: Showing window 0xc0001c (Terminal), shaded: 0 iconic: 0
placed: 0
Window manager warning: COMPARISON:
Window manager warning:   net_wm_user_time_set : 0
Window manager warning:   net_wm_user_time     : 272
Window manager warning:   initial_timestamp_set: 0Window manager warning:  
initial_timestamp    : 0
Window manager warning:   focus_window         : 0x2000030 (brian@pc: )Window
manager warning:   fw->net_wm_user_time : 851694051Window manager warning:
STARTUP--no information about window 0xc0001c (Terminal) found

It just seems that new windows are still always stealing focus.


(Perhaps I am being premature in trying these patches, but this problem has gone
on so long and is such a PIMA that I am willing to guinea pig it)

To make this go you're going to need new startup-notification, gnome-panel, and
other stuff I think. I got the impression in previous discussions there's a
fallback mode for apps that don't support _NET_WM_USER_TIME (say emacs), but if
there's no fallback done yet you'd need the GTK patch as well probably.

gnome-bugs@interlinx.bc.ca:
The problem is that you are launching apps without startup-notification, so
Metacity has no clue when they were launched--it thus can't do anything but fall
back to the old behavior of focusing new windows.  (Note the debugging message
that states, "STARTUP--no information about window 0xc0001c (Terminal) found", a
result of the net_wm_user_time_set and inital_timestamp_set properties both
being 0).

In order to test this, you must either use the test-launcher in
startup-notification to launch applications (that is, the patched
test-launcher), or else you must apply patches to gnome-desktop and nautilus
(and any other applications that have developed a dependency on
startup-notification) and then launch applications using them.  Launching from
the command-line does not work at all (Søren's idea should correct that problem).

Havoc:
There is a fall back for mouse button clicks, but not for keystrokes (I looked
into doing all the appropriate keyboard grabs, but then decided to just wait and
see if that's what you guys really wanted to be done before spending any more
time on it).

Søren:
Okay, I think I'm following now.  I like those ideas, though I may not have time
in the near future to attempt to implement them.

Also, note that bug 120439 is about URGENT flashing for the tasklist.  So fixing
that bug would probably be the hard part of getting
_NET_WM_STATE_DEMANDS_ATTENTION to work.  Granted, that's a different hint than
what this bug is about, but it's closely related so I thought I'd leave the
pointer here...

*** Bug 141008 has been marked as a duplicate of this bug. ***

141008 presents an interesting idea: if a fullscreen window has focus, trigger
the focus-stealing prevention regardless of timestamps.

Is the latest patch attached here ready for review? I was just blogging about
the need to work on this but then I went back to the bug and of course it's been
in progress for a while. ;-)

If we're going to do this for GNOME 2.7 we should branch and get it in CVS
pretty soon for testing. Please pester me as often as required to make that happen.

Havoc: I haven't tried to implement Soeren's really cool suggested extension,
but other than that, yes the patch is here ready for review.  It's the one I
added on 2004-04-14 (which, honestly, hasn't changed much since the end of
January), and has been simply waiting for (blocking on) maintainer review since
that time.

It works for me, and I've even tested the related gtk+ patch with applications
under KWin to make sure everything worked basically the same under KDE with Kwin
and Gnome with Metacity.

Here are the remaining issues with this bug:
  1) Replace meta_warnings with meta_topics in the patch (I just left the
     meta_warnings to make it easy for people to test and see what the patch
     was doing).  Dirt simple and quick.
  2) Decide whether to have a fallback for keypress.  As mentioned in comment
     48, I'm waiting for a maintainer opinion on this.  (For reference, KWin
     doesn't have a fallback for keypress)
  3) Implement Soeren's really cool extension idea (though the patch could be
     committed without that, and that feature added at a later time)

Here are the related issues:
  1) The core part of the gtk+ patch (bug 115650) was committed on the gtk-2-4
     branch.  That's the necessary part.  The API portion (allowing
     applications to easily set a do-not-focus on map hint via a gtk function
     call) was committed on HEAD for gtk+ and will be part of gtk+-2.6.
  2) startup-notification, nautilus, and gnome-desktop need to be patched
     for this metacity feature to be meaningful.  I haven't made the
     suggested changes to the startup-notification patch yet (I figured there
     wasn't much point in doing so until the Metacity patch had been reviewed 
     and could be committed)  Gregory filed bug 136278 for gnome-desktop.  No
     bug has been filed for nautilus.
  3) While not necessary, it would be helpful to also implement
     _NET_WM_STATE_DEMANDS_ATTENTION support along with panel flashing (bug
     120439).

I was planning on pinging you once Metacity branched, although I have also
become occupied with writing a <a
href="http://www.gnome.org/~newren/tutorials/developing-with-gnome/">developing
with gnome</a> tutorial (an effort by the blind to lead the blind) and a few
other projects so I may be a bit short on time.  So anyone who wants to step in
and help bring the bug to completion is more than welcome.

All this timestamp stuff seems like you're solving the wrong problem.

I'm perfectly happy with the first window an application pops up getting focus.
 Well, not perfectly happy, but it can be lived with.  What really annoys me is
when the application pops up another window or a dialog box while I'm doing
something else.

If I'm not mistaken, that window is recorded as being attached to the main
application window for the process.  The fact that the two windows came from the
same top-level application is known to metacity.  So, metacity can group windows
by what application they belong two.

If the window being popped up is a modal dialog box then it's appropriate for
the window to steal focus if the application currently has focus.  That, and the
first window an application displays are the only two reasons a window should
steal focus.

The timestamp thing is useful for long delays between launching a program and
when it displays its first window.  But, that's not actually the most annoying
or pevalent problem, and not the one I'd like solved first.

Comment on attachment 26662 [details] [review]
Patch to support _NET_WM_USER_TIME

On your list of work outstanding:
1. agree, meta_topic
2. I'd commit for now without the WM falling back to monitoring key press for
the client (not sure this is even possible?)
3. definitely let's eval and implement Soeren's idea separately from the base
USER_TIME implementation

Other thoughts:

I'm cringing a bit at adding more size to MetaWindow; my best thought on how to
avoid at least initial_timestamp* is to take that and initial_workspace and
move them into some kind of data structure that only exists when the window
hasn't been mapped yet. Probably best not to do this change in your patch, but
it would be good to have a bug open on it. What I'm thinking is e.g. a hash
table from unmapped windows to the initial state info to use for them, or
something.


"below_this_one->stack_position-1" should have spaces around the minus sign 
;-)
the meta_warning() following should not have a line break after open paren (

Would rename place_window_below_other_in_stack because "place" has sort of a
special meaning in metacity. Suggest instead meta_window_stack_just_below() or
something like that.

This meta_window_ prefixed function should really be in window.c - it looks
like the problem here is that set_stack_position() doesn't sync_to_server(), if
it did then we could just call that from window.c. However, the uses of
set_position() in stack.c now assume it doesn't sync (they either sync after
calling it, or use set_position() as part of the sync process)

So I'd suggest having set_stack_position_no_sync() as a static function which
does what the current one does, using that inside stack.c, and
set_stack_position() public that does the sync, and use set_stack_position()
public to implement meta_window_stack_below() in window.c

(this is really a small change just takes a while to explain ;-))

XSERVER_TIME_IS_LATER needs spaces around comma and minus sign

Multiline comment in window_takes_focus_on_map is supposed to have * down the
side (emacs alt+q will do this for you if you get it started)

You can't have "Time compare" in the middle of a function (C99/GNUC extension),
variables have to be declared at the top of the block.

space before paren in 
use of XSERVER_TIME_IS_LATER(

Anyway, this is pretty much all nitpicks, the patch looks great. Thanks and
apologies for my over the top sucking at patch review lately.

Should I be looking at the startup notification patch here also?

Created attachment 28649 [details] [review]
Patch with changes suggested by Havoc

This contains the changes suggested by Havoc.  I'll file a separate bug on the
size of the MetaWindow struct in a minute.

One point to note.  In testing this, I noticed that the times when a window
isn't supposed to take focus and it appears, it is always behind the focused
window as it is supposed to be.  However, it is also sometimes behind other
windows as well (which aren't on top of the focused window).  It's not
completely lowered, though, since I have seen it below the focused window yet
on top of another.  I'm not sure what the issue is, but I believe it's
relatively minor.  Since I may not have a lot of time with an upcoming move,
I'm posting the patch without tracking this issue down.

Created attachment 28650 [details] [review]
Fixed startup notification patch needed for _NET_WM_USER_TIME

Okay, I made the changes to the startup notification patch Havoc and Gregory
mentioned.  It's ready for review.  :)

Comment on attachment 28649 [details] [review]
Patch with changes suggested by Havoc

I just branched metacity, so this can go on HEAD.

I just noticed one more issue, when you set the current time from
KeyPress/ButtonPress, isn't it a bug if we didn't set display->current_time
from those? And if so, why double-check that display->current_time !=
CurrentTime? If I'm right, maybe you want a g_assert (CurrentTime !=
display->current_time) instead. Sorry I didn't notice this before.

Comment on attachment 28650 [details] [review]
Fixed startup notification patch needed for _NET_WM_USER_TIME

For future reference, putting diff -p in .cvsrc makes patches nicer (shows the
function being changed)

In sn_launcher_context_initiate(), double-check you don't need to increase
MAX_PROPS (I didn't check)

Should you avoid setting the TIMESTAMP prop if CurrentTime is passed in?

In the test app you have a variable definition not at the top of the block,
won't build with old compilers.

Good to commit with those things resolved, thanks.

CRAP!  I don't have write access to freedesktop.org's CVS.  So, I'll just attach
both the fixed patches (with the ChangeLogs) here in a moment (I don't want to
commit the metacity one without the startup-notifiction one or else builds will
break).

Yes, you're right about the display->current_time stuff.  I changed it to an
g_assert.

MAX_PROPS was 12, and only 9 at most could be used, so no change needed yet. 
(There's also an assert to double check and make sure things are cosher)  :-)

Personally, I'm leaning towards having TIMESTAMP set to CurrentTime if that is
what is passed in.  I want to intentionally do exactly that in order to launch
many applications from a terminal without having them steal focus (something
that may be difficult to do otherwise with applications that launch quickly). 
That may be a small use case, but I can't think of _any_ use case for doing the
opposite, other than allowing broken apps to remain broken.  And I'm guessing
that it's quite likely that some launchers will remain broken (i.e. continue to
only pass CurrentTime) and therefore make TIMESTAMP useless unless we accept
CurrentTime.  So broken apps just seems like a second reason to accept
CurrentTime for TIMESTAMP.  Of course, we might get a lot of complaints until
they do...  Let me know if you'd rather I changed it.

I fixed the variable definition stuff too.

Both patches will be attached shortly.

Created attachment 28816 [details] [review]
Metacity patch with ChangeLog and Havoc's last suggested change

Created attachment 28817 [details] [review]
Startup notification patch with ChangeLog and Havoc's suggested changes

startup-notification patch committed. Releasing statup-notification-0.7 now.

Committed the Metacity patch to CVS...  Here come all the complaints that new
windows start unfocused and behind the focused window (due to gnome-desktop bug
136278 and nautilus bug 144987)...

OK now we need to support DEMANDS_ATTENTION or this will be a usability
nightmare.  The metacity part of this is trivial; we need to implement some sort
of blinking for the tasklist and/or window-list applet.  Should probably create
a gnome-panel bug for that.

I've updated the COMPLIANCE file to indicate that USER_TIME is supported.

Incidentally, do we need to make this into a preference?  This has the potential
to cause problems with accessibility support, like screen readers.  Perhaps we
disable it if the xsetting for accessibility support is enabled?

Rob: No need to file a new bug for the panel flashing; bug 120439 already exists.

In comment 84, it was bug 144897 I meant, not 144987...

Also, this sucks but I unfortunately have run out of time and won't be able to
help with DEMANDS_ATTENTION or other related issues before API/feature/module
freeze.  (School stuff).  Sorry about that.

*** Bug 147336 has been marked as a duplicate of this bug. ***

A patch to implement the DEMANDS_ATTENTION in metacity has been attached to bug
148370.

No, that patch implements it in wnck.  Bug 148364 has the metacity patch.

OK the last patch has been committed for this support.  There's still a bug
related to unminimizing but I think there's something already open on that.

Thanks for your help on this work, Elijah; you did a great job.

Created attachment 30123 [details] [review]
Fix bug on unmimizing

Unfortunately there appears to be no good way to actually set the timestamp for
real on unmimimizing the way it currently works.  So this hack just disables
user_time during a show generated by meta_window_unminimize.

I'm just committing this since it's so simple, and it works.

Comment on attachment 30123 [details] [review]
Fix bug on unmimizing

I don't understand why libwnck/gtk can't set user time properly on unminimize?
Or just send ACTIVATE?

This patch doesn't feel very warm and fuzzy.

Yea I didn't really like it much either, and I'd considered doing it in libwnck,
but don't you think that it's a problem if metacity requires some weird
technique to get unminimizing to work correctly?

People might not being using tasklist with metacity, in which case they'd be in
for a bit of a rude awakening.

Rob: Thanks for all your patience in explaining things so that I could help.  It
probably would have been quicker for you to do it yourself, but I'm really glad
you took the time to help me out.  :-)

As far as the unminimize problem, we already have bug 147950 open.  Should we
just mark it as a dupe?  Either way, I think Rob's last comment reinforces the
one I made in the 10th comment of bug 102665--namely, that the current
libwnck/metacity communication method sucks for minimizing/unminimizing and that
_NET_WM_STATE_MINIMIZED would be helpful.  Of course, it's a little late to
extend the spec and implement it in time for Gnome 2.8...

Since Rob closed 147950 pointing to this one, I guess I'll reopen this one. 
There are three issues I can see with Rob's patch for unminimizing (although 
one is perhaps the same as not making Havoc "feel very warm and fuzzy" :-)

1) Part of Rob's rationale for the change in Metacity was that unminimzing
should work correctly whether or not people were using libwnck.  While that's
true, it never has worked correctly anyway (and still doesn't)--see bug 107681,
bug 128200, and bug 102665.  user_time made it worse, but the problem was still
there.  Rob's fix only tries to return things to the less broken state.  I
really think we need a good communication methods to handle minimizing and
unminziming between the window manager and other apps.  (*cough*
_NET_WM_STATE_MINIMIZED *cough*)

2) (Assume click-to-focus for a second; since it's clearest to explain with that
focus method.)  If the user uses right-click on the taskbar and then selects
unminimize from the menu that appears, then the unminimized window does appear
on top but it is not focused as one would expect.  So the patch doesn't quite
work in this (uncommon) case.  I'm not sure why.

3) Rob's patch, with the exception of the unintentional bug in issue 2, always
focuses the unminimized window.  This may not be the desired behavior for sloppy
or mouse focus methods (see bug 135810).  However, I'm starting to believe that
this may be the correct behavior--note the 10th additional comment in bug 102665.

Of course, Rob's patch does make things _A LOT_ better than they previously
were, so I am by no means suggesting removing it--at least not until an
alternate fix is available.

Wrt. to not working in the right click case: Maybe the _menu_ window gets
focused causing it to appear in mru lists? I remember this (popup menus get
keyboard focus, but menu bar menus don't) from debugging an unrelated GTK+ issue.

I tracked down the cause of issue 2.  Apparently it's due to a weird
inconsistency in libwnck.  Left-clicking on the item in the taskbar results in
libwnck sending a _NET_ACTIVE_WINDOW message.  Right-clicking and selecting
unminimize from the popup menu results in libwnck calling either gdk_window_show
or XMapRaised.  So the two types of unminimizing are treated differently.  This
seems like a bug to me, though perhaps there was a reason for that.

But this brings up a different point that confuses me even more.  libwnck was
sending a _NET_ACTIVE_WINDOW message (which apparently doesn't come with a
timestamp right now, but Metacity's window.c's meta_window_client_message
specifically checks for that and provides a fallback) to Metacity.  This results
in Metacity calling meta_window_activate, which, after unminimizing the window
calls meta_window_raise and meta_window_focus.  So why weren't these windows
being raised and focused even without Rob's patch?  I don't understand...

Okay, I think I get it.  Regarding the thing confusing me in comment 99:
Metacity's fallback of meta_display_get_current_time can return 0 (and does in
this case), which doesn't help.  But I believe patching libwnck to send a valid
timestamp on _NET_ACTIVE_WINDOW messages would solve that issue.  But now I'm no
longer on topic for this bug, so I'll file this elsewhere...

Okay, further investigation and I have some patches that solve the problem an
alternate way from Rob's fix.  I don't know if you want these patches, but I
provide them in case they are at least partially useful.  One extra thing they
do is make the right-click-then-select-unminimize thingy work.   What the
patches do are:

1) Update libwnck to follow the EWMH spec (the CVS version; the version on
   the website is horribly out of date; this just means sending a
   timestamp with a _NET_ACTIVE_WINDOW message)
2) Have Metacity actually use the timestamp passed by the _NET_ACTIVE_WINDOW
   message
3) Make right-click-then-select-unminimize in libwnck have the same behavior
   as left-click, i.e. call meta_window_activate_transient in both cases to
   make the window become unminimized, instead of doing it different ways for
   each case.
4) Undo Rob's patch (no point in testing out these patches if you have Rob's
   patch applied since then you can't tell if my patches really work)

I think this is the way that Lubos handled things in KDE, because there's extra
stuff in the spec about the id of the window sending the _NET_ACTIVE_WINDOW
message, which is used to potentially ignore the request by KDE (it tries to
discount messages that come from windows without the focus).  I didn't follow it
completely and just put a 0 for that part of the libwnck message, though. 
(Anyone care to explain further and how I obtain the "requestor's currently
active window"?)

Two other notes: (1) this does fix bug 102665 as well, but still leaves bug
107681 and bug 128200 (which are really the same bug but just filed against
different products) open.  (2) This patch assumes always-focus-the-unminimized
window.  I think that is the correct logic, though.

I will attach the patches momentarily.

Created attachment 30166 [details] [review]
libwnck -- timestamp for _NET_ACTIVE_WINDOW, use activate instead of deiconify

Created attachment 30167 [details] [review]
metacity -- use the timestamp from _NET_ACTIVE_WINDOW (if nonzero)

Heh, so while I was offline the last few days I was thinking about this patch
and I thought "why not just set the user time to the current time, instead of
adding the little flag to ignore user time?" - so glad to see a patch waiting. ;-)

+  timestamp = gtk_get_current_event_time();
+  g_assert (timestamp != 0);

Is the != 0 really guaranteed? (too lazy to check docs, but please check)

Otherwise, both patches look good.

gtk_get_current_event_time does not guarantee that it will not return 0 in all
cases.  So the question really is whether there's a guarantee for this
particular case.  I didn't know, so I put the assert in (basically because I
didn't want to get a time of 0 passed and then assume that I just screwed up the
metacity patch).  Then, I considered removing it before posting the patch, but:

If we remove it, and anything changes in the future such that it does return 0,
then we have a hard to track down bug (i.e. reports of windows sometimes not
appearing on top and being focused when being unminimized, but which we may not
be able to duplicate).  If we don't remove it and such a change occurs in the
future, we have a lot of unhappy users that get constant crashes.

Perhaps a better thing to do would be check if it's 0, and if it is, then create
a window and use the get_server_time function in libwnck's xutils.c to obtain a
valid timestamp.  Does that sound like a good idea?  Can you think of anything
better?

*** Bug 149329 has been marked as a duplicate of this bug. ***

Gregory has a much more thorough patch for libwnck and timestamps in bug 135024.
 It should probably be reviewed in preference to the patch I posted here. 
(However, his patch doesn't cover my change in libwnck/window.c which will still
be needed).  I'm marking this bug as depending on that one.

Another solution to get the timestamp would be to pass it through the library
from the event causing the _NET_ACTIVE_WINDOW (i.e. make wnck_window_activate()
take a timestamp arg, and pass it in) (or add
wnck_window_activate_with_timestamp() to avoid breaking API, though libwnck can
hypothetically break API if we want)

Otherwise I think either a g_warning() on 0 timestamp or fetch the server time.

Can you merge the two timestamp patches, or commit yours since I already looked
at it and then post what gregory's would add with respect to HEAD?

I committed the two patches, changing the g_assert into a conditional g_warning.
 I tried fetching the server time, but couldn't get it to work; the tasklist
just appeared to stop responding to all requests altogether.  I don't know why.
 I'll update Gregory's patch and post it to bug 135024 (so that this bug can
finally be closed and rest in peace).

However, note that these last two patches really accentuate the problem in bug
149589, so without the patch there, things kinda suck.

The patch in bug 149589 was committed, so I'm going to close this bug.

May it rest in peace.

*** Bug 150034 has been marked as a duplicate of this bug. ***

*** Bug 151438 has been marked as a duplicate of this bug. ***

To all the people watching this bug: This unfortunately won't be a feature found
in Gnome 2.8; there were still some remaining issues so we had to disable it
(see the 56th comment of bug 149028).  In other words, there's no need to reopen
this bug saying the feature isn't working when you try out Gnome 2.8.  Sorry, I
was hoping we could get it done in time too...

One thing that shouldn't be overlooked is the serious security implications of
focus policy. This is perhasps best looked at as a special case, overriding all
other focus policy, but what I'd like to see is this:

If a user is actively and furiously typing away, window focus should NOT CHANGE
FOR ANY REASON except direct user request. (Hitting alt tab, clicking on a
window...) For all you know, a password is being typed. This should NOT BE
INTERRUPTED FOR ANY REASON.

I would implement this as a typing timer. If a key has been hit within the last
3 seconds (or more, this is open for fine tuning...) DO NOT ALLOW FOCUS TO
CHANGE. You could queue up any focus changes, and perform them after the timer
expires.

This is much like a screen-saver. This does not need per-application activity
timing. In fact, as a potential security issue, applications shouldn't even have
a say in this. A badly implemented or even an outright malicious application
should not be able to interfere during typing. All that matters is if there's
ANY active keyboard activity at all, window focus should NOT BE CHANGED FOR ANY
REASON. With the possible exception of window manager specific keybindings.

I probably missed something, but its something to consider.

Timer is no good, since you won't focus new windows opened via key shortcut.

Don't worry, we are making it work.

Focus should not be stolen by other applications, period. A timer is pointless
because even if you stop typing for a little while, you still don't want
programs stealing focus. For instance, I could be entering a password, then,
say, go up and get a cup of water, then come back and type it in and press enter
without looking at the screen and what do you know? It was sent to my buddy on
AIM. The way Windows NT 5+ does it, with the flashing window (and on Windows XP
it stops flashing and just stays highlighted eventually) is the way to go.

Focus should not be stolen by the same application either. For example, you go
to a website in Mozilla. Meanwhile, you go to another website in another window.
When the first website is not found and you get an error, you don't want that
error in your face, but also as a flashing entry in the taskbar. Windows fails
here if I'm not mistaken.

Guys, we know about all these cases as well as several others that you may not
have thought of.  This bug is closed.  If you want to comment further, please
either (1) try out one of the betas of Gnome 2.10 in a month or so (so that
application authors have a little time to fix some remaining known issues) and
submit any issues you find, or (2) read the EWMH spec from the icccm-extensions
freedesktop.org CVS module and the startup-notification documentation in the
startup-notification freedesktop.org CVS module and all dependencies of bug
149028 in addition to this complete bug and bug 115650 and then add any further
comments to bug 149028.

Thanks.

*** Bug 163797 has been marked as a duplicate of this bug. ***

*** Bug 165041 has been marked as a duplicate of this bug. ***

*** Bug 166966 has been marked as a duplicate of this bug. ***

:cry: 2 years and counting :(