After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 161361 - cross-desktop window selection focus problem
cross-desktop window selection focus problem
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: EWMH specification
trunk
Other Linux
: High critical
: 2.10.x
Assigned To: Metacity maintainers list
Metacity maintainers list
: 165875 166716 167222 167983 168041 (view as bug list)
Depends on: 128380 167545
Blocks: 155450
 
 
Reported: 2004-12-15 11:58 UTC by Gustavo Carneiro
Modified: 2005-07-24 01:19 UTC
See Also:
GNOME target: ---
GNOME version: 2.9/2.10


Attachments
Do all the libwnck stuff (19.36 KB, patch)
2005-02-16 09:21 UTC, Elijah Newren
accepted-commit_now Details | Review
Metacity patch to support _NET_CURRENT_DESKTOP messages with a timestamp (1.61 KB, patch)
2005-02-16 09:29 UTC, Elijah Newren
accepted-commit_now Details | Review

Description Gustavo Carneiro 2004-12-15 11:58:02 UTC
Need at least two desktops.

1. open two windows on one destkop
2. give focus to one of the windows
3. switch to a different desktop
4. use the Window Selector applet to select the window that _doesn't_ have focus
in the first desktop

Expected result:
  Selected window is raised and given focus

Actual result:
  Window is raised but not given focus

This behaviour surprised me to accidentally close Evolution instead of a chat
window I was trying to select.

PS: I'm using metacity and click-to-focus.  Applet version is 2.8.1.  Metacity
version 2.8.6.  Fedora Core 3.
Comment 1 Vincent Untz 2004-12-18 12:18:57 UTC
This is a libwnck (or a metacity) problem since we call wnck_window_activate ().
Maybe a problem with timestamps...
Comment 2 Elijah Newren 2004-12-18 18:55:03 UTC
Yup, a timestamp problem.  And one which will require a change to the EWMH to
fix (and, once that is fixed, will require modifying Metacity, libwnck, and all
apps that call wnck_workspace_activate--though the changes will all be nearly
trivial):

When you select a window from the Window Selector applet which is not on the
current workspace, the applet (via libwnck) sends a _NET_CURRENT_DESKTOP message
(to switch workspaces) followed by a _NET_ACTIVE_WINDOW message (to activate the
correct window) to the window manager.  The former is sent without a timestamp
(an EWMH problem), while the latter is sent with one...

Now, when Metacity gets the _NET_CURRENT_DESKTOP message it of course calls its
routines to switch workspaces.  Switching workspaces always involves picking a
new focus window.  Whenever we focus a window, we need to provide a timestamp. 
Since one doesn't come with the _NET_CURRENT_DESKTOP message, metacity obtains
one by requesting the current Xserver time *as soon as* it has received the
_NET_CURRENT_DESKTOP message.

Now, the problem is that the _NET_ACTIVE_WINDOW message does come with a
timestamp and it is naturally less than the timestamp used for switching
workspaces (because the timestamp for switching workspaces isn't obtained until
after metacity *receives* the _NET_CURRENT_DESKTOP message, which is naturally
later than when the applet is clicked on).
Comment 3 Havoc Pennington 2004-12-19 03:03:14 UTC
Add one to the "all messages, requests, and events should just have a timestamp
since you will need it eventually" file.
Comment 4 Elijah Newren 2004-12-19 04:02:53 UTC
Will
  all_events_need_a_timestamp_file++;
be enough or should we start worrying about overflow?  ;-)
Comment 5 Elijah Newren 2005-02-01 00:16:38 UTC
*** Bug 165875 has been marked as a duplicate of this bug. ***
Comment 6 Elijah Newren 2005-02-03 18:38:26 UTC
Putting this on the 2.10.x milestone so I don't forget it...
Comment 7 Elijah Newren 2005-02-08 22:27:27 UTC
*** Bug 166716 has been marked as a duplicate of this bug. ***
Comment 8 Elijah Newren 2005-02-14 19:14:20 UTC
*** Bug 167222 has been marked as a duplicate of this bug. ***
Comment 9 Elijah Newren 2005-02-16 09:12:41 UTC
Oh, what a tangled web we weave...

To correctly fix this problem, bug 128380 must first be fixed.  (Without that,
we'd need an additional EWMH extension because sending both a
_NET_CURRENT_DESKTOP and a _NET_ACTIVE_WINDOW message with the same timestamp
still results in a race.  But there's no point in an additional EWMH extension
when there's already something that's supposed to work...)

Also, to correctly fix this for mouse and sloppy focus modes, bug 167545 will
also need to be fixed (we need a clean-as-possible way to allow the invariants
to be broken... *sigh*).

Also, libwnck is inherently buggy in regards to timestamps because it uses
gtk_get_current_event_time() instead of taking a timestamp parameter in the
relevant functions.  We have real-world examples already where annoying and
somewhat difficult to find bugs resulted from using gtk_get_current_event_time()
(see bug 166379), so we really need to fix this too.

And, of course, we still need to add a timestamp to _NET_CURRENT_DESKTOP
messages like this bug says...
Comment 10 Elijah Newren 2005-02-16 09:21:14 UTC
Created attachment 37533 [details] [review]
Do all the libwnck stuff

This patch does all the libwnck stuff mentioned above (handle the changes to
_NET_ACTIVE_WINDOW from bug 128380, make relevant functions take a timestamp
parameter, add a timestamp to the _NET_CURRENT_DESKTOP message--I need to email
wm-spec-list about this last change).  I didn't split it into separate patches
because they would have conflicted...
Comment 11 Elijah Newren 2005-02-16 09:29:35 UTC
Created attachment 37534 [details] [review]
Metacity patch to support _NET_CURRENT_DESKTOP messages with a timestamp

As with the last part of the libwnck patch, I need to email wm-spec-list about
this one...
Comment 12 Havoc Pennington 2005-02-20 20:43:47 UTC
Comment on attachment 37533 [details] [review]
Do all the libwnck stuff

This looks fine but it does change API and require the panel etc. to get
rebuilt (and possibly modified), so be sure it's in line with the release
policies and so forth. I'm not sure what the rule is here.
Comment 13 Havoc Pennington 2005-02-20 20:44:21 UTC
Comment on attachment 37534 [details] [review]
Metacity patch to support _NET_CURRENT_DESKTOP messages with a timestamp

thanks!
Comment 14 Elijah Newren 2005-02-20 21:09:26 UTC
*** Bug 167983 has been marked as a duplicate of this bug. ***
Comment 15 Elijah Newren 2005-02-20 23:41:09 UTC
Committed both patches, with incrementing LIBWNCK_CURRENT in
libwnck/configure.in because of the API changes.
Comment 16 Elijah Newren 2005-02-21 15:59:07 UTC
*** Bug 168041 has been marked as a duplicate of this bug. ***
Comment 17 Gustavo Carneiro 2005-02-22 11:16:59 UTC
Thanks a lot! :-)
Comment 18 Elijah Newren 2005-07-24 01:19:42 UTC
The portion of the libwnck patch due to the behavioral change of
_NET_ACTIVE_WINDOW has been reverted on both the gnome-2-10 branch and HEAD,
because the behavioral change of _NET_ACTIVE_WINDOW in metacity was reverted (on
both gnome-2-10 and HEAD).  I don't think this should affect this bug since
_NET_CURRENT_DESKTOP messages now come with timestamps (if it does, I can't
trigger the race and others seem to claim there isn't one; regardless we still
do need a way of replacing _NET_CURRENT_DESKTOP + _NET_ACTIVE_WINDOW with a
single message because otherwise the panel is trying to force a particular
policy instead of letting the WM decide it), but I thought it was worth
mentioning.  See bug 128380 for more details about this.