This animation  illustrates how this bug appears:
http://xml.peet.spb.ru/files/out.swf

Aha!  nautilus is using gnome-desktop to do startup notification, and
gnome-desktop is using gtk_get_current_event_time to obtain the timestamp, but
gtk_get_current_event_time returns 0 in the cases you show.  So, we clearly have
to do something better than that.  Thanks for pointing this out.

For those that don't have flash:  If you open up a nautilus browser and go to
preferences:/// and then double click on the various preferences programs to
launch them, they will often be launched with a TIMESTAMP of 0.  Putting an
"assert (timestamp != 0)" in sn_launcher_context_initiate shows that the 0
timestamp was passed from ditem_execute in
gnome-desktop/libgnome-desktop/gnome-desktop-item.c via a call to
gtk_get_current_event_time.

I'm just going to reassign this to gnome-desktop instead of duping and reopening
bug 136278 (which is where the choice of using gtk_get_current_event_time was
introduced).

This is only very tentatively a 2.8.0 stopper (meaning it isn't, but it would be
really nice to have it fixed).

focus-stealing-prevention has been disabled in Metacity, so this bug is
irrelevant for the 2.8.0 release (well, except for people using Kwin as their
window manager in Gnome, but I don't know if we're worried enough about that to
break hard code freeze).  I'm changing the Gnome Milestone to 2.10.0.

Elijah: need to make sure we can get at display->user_time in GTK+ 2.6, right?

Maybe, I don't know.  I believe that would work fine, but there's also another
possibility: adding API to gnome-desktop for setting the timestamp (then apps
like the panel that know gtk_get_current_event_time are safe can use it and
other apps, such as nautilus in this case, can do something more involved).  I'm
going to cc gtk-bugs to get their opinion on the best way to obtain a timestamp
here.

I don't have flash installed on this machine, so I have no clue
what cases this bug is about.... generally you do want to make
sure that you use the real event time from the triggering event
not just an arbitrary last received event when possible.

A particular app can track the last received event time themselves (hackily) 
by using gdk_event_handler_set() after gtk_init() and gtk_main_do_event()
to forward the events to GTK+.

Something like gdk_display_get_last_event_time() would probably be an
acceptable addition for a future version of GTK+.

Owen: I tried to explain the cases this bug is about in comment 2, but let me
try again: A user double clicks on an icon in nautilus to launch an application,
nautilus calls gnome-desktop to do the launch with startup-notification, and
gnome-desktop tries to use gtk_get_current_event_time() to obtain the timestamp
of the event causing the launch (i.e. the double-click).  Unfortunately,
gtk_get_current_event_time() returns 0 and we need a real timestamp.

Is gdk_display_get_last_event_time() (returning display_x11->user_time) the best
way to do this?  You comment about using "the real time...not just an arbitrary
last received event" makes me think that perhaps apps could obtain a more
accurate timestamp and thus it'd be better to add API to gnome-desktop for
setting the timestamp--am I understanding correctly?  Would we still need a
gdk_display_get_last_event_time() in order for nautilus to be able to obtain a
timestamp for the double-click?

Isn't gnome-desktop a library? Why isn't gtk_get_current_event_time()
returning the right result then?

gtk_get_current_event_time() will definitely return the right time
in the double click handler.

Yes, gnome-desktop is a library.  I'm not sure why it's not returning the right
result, but I tried digging a little more:

1) I added a bunch of debugging code and determined that calling
gtk_get_current_event_time() within handle_icon_button_press() in
nautilus/libnautilus-private/nautilus-icon-container.c will return the right
timestamp (although it's not needed since we have a GtkButtonPress* event passed
to that function, and we can just use event->time which holds the same value).  

2) Nautilus may be queueing these events or something, because a stack trace
involving the gtk_get_current_event_time() call within gnome-desktop does not
include the handle_icon_button_press() function call.  I don't understand
nautilus very well, but I'm guessing this may be why you warned that we should
"make sure that you use the real event time from the triggering event not just
an arbitrary last received event when possible"?

Focus stealing prevention has been re-enabled on head, so we should look at this
issue again.  Since nautilus appears to be queueing an event for the button
press, I think it'd make sense to either modify the API of the
gnome_desktop_item_launch_* functions to take a timestamp argument or else to
add a gnome_desktop_item_set_launch_time function that must be called.  Doing
one of these are the only way I can see handling Owen's suggestion in comment 7
to use the real timestamp.  (Note that according to an exhaustive grep, the only
modules which currently call a gnome_desktop_item_launch_* function are
gnome-control-center, gnome-panel, and nautilus).

I'm going to cc nautilus-maint to get their opinion/input on the matter, as well
as so that they can correct anything wrong I said above.

Guys: this should be done ASAP...

In eclipse, we currently keep around the timestamp of the last event we
processed for handling cases like this where you need an event time from
something that got queued up, rather than directly from the handler.  Adding a
gdk_display_get_last_event_time() API would be a nice addition.

Ooops.  I've been meaning to jump back here and ping Mark but I, uh, got
distracted.  Anyway, in bug 136278 Mark went with gtk_get_current_event_time
instead of the patch to add API to gnome_desktop.  So, I'm waiting for his input
on the matter...

My current viewpoint:  I think using a gdk_display_get_last_event_time() kind of
call could work as a backup if an app didn't specify any timestamp, but that it
should not be forced as the selected timestamp as I can now think of some corner
cases where I believe this would fail.  So, I think we need a
gnome_desktop_item_set_launch_time() API either way.  As Owen said, "generally
you do want to make sure that you use the real event time from the triggering
event not just an arbitrary last received event when possible."

Besides, it's too late to add a gdk_display_get_last_event_time() API to
gtk+-2.6, and gtk+-2.8 won't be released until well after Gnome 2.10.

I've filed bug 163119 for gdk_display_get_last_event_time()

So, my understanding was that we went with gtk_get_current_event_time() for 2.8
because we were going to make sure we had something like
gdk_display_get_last_event_time() for GNOME 2.10.

I guess it is cleaner if we do actually allow setting the timestamp, though.

I'd prefer it to be part of the gnome_desktop_item_launch() arguments, but I
don't want to break the ABI for this and I don't relish
gnome_desktop_item_launch_on_screen_with_env_and_timestamp() either :-)

So, gnome_desktop_item_set_launch_timestamp() should be fine, but we should
clear the timestamp immediately after using it, allowing it to fallback to
gtk_get_current_event_time() if the next launch doesn't specify a timestamp.

Sorry Mark, I didn't have the same understanding.  I guess that's where this got
hung up.  :/

Anyway, I'll attach a patch momentarily that implements your suggestion.

Created attachment 35684 [details] [review]
Implement Mark's suggestion

This patch is a little ugly, because of conflicting requirements: (1) don't
change the API, and (2) unset the timestamp immediately after use so it isn't
accidentally reused.  Since the GnomeDesktopItem is passed as a const, I had to
put the timestamp stuff in a separate struct in order to be able to unset the
timestamp.

 - Bah, I'd ignore the "const" in gnome_desktop_item_launch() rather than
   adding another struct. Oh, the ugliness :/

 - Don't make the availability of the API dependant on 
   HAVE_STARTUP_NOTIFICATION.You need the ABI to be the same with or without 
   HAVE_STARTUP_NOTIFICATION. Also, each app would need to have 
   HAVE_STARTUP_NOTIFICATION set

 - I don't like the GDK_CURRENT_TIME warning, but maybe its neccessary. Are
   you sure we couldn't have a pre-condition that you can't pass
   GDK_CURRENT_TIME?

 - I also don't want apps to start spewing warnings if they update to newer
   libgnome-desktop but the app doesn't set the launch time. So, the warning
   in ditem_execute() should be removed.

 - Spuriosity:

-* Clearing attributes
+ * Clearing attributes

Do you want me to remove the const from the function definition?  (Or do you
want me to cast away the const-ness?)

Passing GDK_CURRENT_TIME should be allowed, though rare.  It's a "please don't
focus this app on launch" request.  It's similar to the reasons for having
gtk_window_set_focus_on_map(window,FALSE).

I guess I don't understand why having the warning in ditem_execute is bad.  What
would be optimal, as you pointed out, is if we could make a timestamp argument
part of ditem_execute().  Since you don't want to change API/ABI though, what is
wrong with warning app writers that things have changed and their old usage
patterns are wrong?  If we give no warnings in ditem_execute then some people
will wonder why their apps are behaving incorrectly.  Granted, we have a decent
default to fall back to and it will improve with
gdk_display_get_last_event_time(), but that won't work for all cases either and
having apps provide a timestamp shouldn't be viewed as optional.  Obviously,
it's somehwat of a tradeoff between spurious warnings, and not providing info
when it'd be very helpful, so if you still want it removed, I'll go ahead and do
so, but could you perhaps explain to me why you weight things towards the no
warning end of things?

Sorry for the spuriosity--I guess I saw some nearby code misaligned and I just
couldn't resist fixing it.  ;-)

Elijah, I'm looking at writing gdk_display_get_last_event_time(), and I wonder 
what the intention is here. Would it be good enough to return the value of the
display_x11->user_time field, which tracks the timestamp of the last user
interaction related event (in which case we may want to call the function 
gdk_display_get_user_time()), or do you really want the latest timestamp,
including things like propertynotify, etc

Short answer: Yes, use display_x11->user_time.

Longer answer:
The timestamp passed to gnome_desktop and which is used to set
DESKTOP_STARTUP_ID, is supposed to be the exact analog of _NET_WM_USER_TIME
(_NET_WM_USER_TIME is set on new windows to the timestamp of the last user
interaction with the app; however, no such interaction exists for the very first
window, of course, so whoever launches the app has to fill in that missing
info--that's what this gnome_desktop timestamp is).  Therefore,
display_x11->user_time is exactly what we want; including propertynotify events
would probably destroy its usefulness, at least in this context.  I agree that a
different name would probably be better; most app writers may not know what
"user time" means, so something like
gdk_display_get_last_user_interaction_time() may be better.

Doh, correction:
..._NET_WM_USER_TIME is supposed to be set on new windows to the timestamp of
the user interaction which caused the window to appear (which is usuaully the
last user interaction with the app)...

GTK+ 2.8 will have gdk_x11_display_get_last_user_time()

Cool, that will make it easier for some apps to provide a timestamp to these
functions.

Mark: ping?  (see comment 20)

In addition to my questions in comment 20, the following is interesting:

1062 newren@amr:gnome$ find . -name "*.c" | xargs grep -l \
                       "gnome_desktop_item_launch"
./gnome-desktop/libgnome-desktop/gnome-desktop-item.c
./gnome-desktop/libgnome-desktop/test-ditem.c
./gnome-panel/gnome-panel/panel-util.c
./nautilus/libnautilus-private/nautilus-program-choosing.c
./gnome-control-center/capplets/wm-properties/wm-exec.c
./gnome-control-center/control-center/control-center.c
./gnome-control-center/libwindow-settings/wm-exec.c
./gnome-utils/gsearchtool/gsearchtool-support.c

So it only appears to be used by gnome-panel, nautilus, control-center, and
gnome-utils.  That's a small enough group that it may be worth considering
actually adding the timestamp to the argument list (since we both say that would
be nicer if it weren't for the API/ABI thing)--I'll take the responsibility of
patching all these other components if you want to go that route.

Okay, attaching what I'd like to commit. Some notes:

 - Yeah, the cast in ditem_execute() is ugly, but we'll live ... :)

 - If we need "don't focus window on map", it should be an explicit API rather
   than a magic value of zero to set_launch_time(), e.g.

void gnome_desktop_item_set_focus_on_launch (GnomeDesktopItem *item,
                                             gboolean          focus_on_launch);

    but lets just assume we don't need that for now.

 - I've taken out the warning on ditem_execute() - this will give the effect
   of warnings being spewed to users of the app rather than informing the
   developer of the need to re-work the code. If we wanted to warn the 
   developer, the correct way would be to deprecate all the non-timestamp-using
   APIs. But as you pointed out, we know which apps use the APIs, so lets just
   fix them.

 - I'd prefer not to break the ABI, even if it was earlier in the cycle. The
   set_launch_time() API will do fine.

 - I've logged bug #165689 so we remember to use 
   gdk_x11_display_get_last_user_time() when we start using gtk+ 2.8

 - I'll mail the release to ask for approval for the API addition.

 - We need to fix panel, nautilus, control-center and gsearchtool once we get
   this in

Created attachment 36717 [details] [review]
patch

Okay, we're done with libgnome-desktop. Thanks for your patience Elijah

2005-01-30  Mark McLoughlin  <mark@skynet.ie>

        Based on a patch from Elijah Newren in bug #150910
        to add gnome_desktop_item_set_launch_time()

        * gnome-desktop-item.c, libgnome/gnome-desktop-item.h:
        (gnome_desktop_item_set_launch_time): add.
        (gnome_desktop_item_new): explicitly initialize launch_time.
        (gnome_desktop_item_copy): copy launch_time.
        (ditem_execute): if launch_time is set, use that instead
        of gtk_get_current_event_time()