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 401028 - Unshaded windows not focused
Unshaded windows not focused
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
2.16.x
Other Linux
: Normal normal
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
: 524687 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-01-26 15:22 UTC by Shaun McCance
Modified: 2008-12-19 12:47 UTC
See Also:
GNOME target: ---
GNOME version: 2.15/2.16


Attachments
possible fix (378 bytes, patch)
2007-03-16 21:30 UTC, Shaun McCance
reviewed Details | Review
possible fix (387 bytes, patch)
2008-09-10 01:59 UTC, Thomas Thurman
committed Details | Review

Description Shaun McCance 2007-01-26 15:22:10 UTC
If you unshade (un-roll-up?  I never got used to that terminology) a window in Metacity, keyboard focus is not moved inside the window.  The window appears focused (i.e. blue titlebar) and is raised, which is all fine.  But key events don't go inside the window, which is very annoying.

This behavior is new in 2.16 on FC6.  I've been shading and unshading windows in Metacity for years, and this is the first time I've seen this problem.
Comment 1 Shaun McCance 2007-01-26 15:31:56 UTC
Actually, there were some other focus oddities going on.  I played with it a bit more, and I can describe the problem better.  It's not just that unshading doesn't give keyboard focus to the window.  Rather, unshading puts the window into some sort of quasi-sloppy-focus mode.  This persists even after clicking in the newly-unshaded window, and doesn't stop until another window is explicitly focused and the previously unshaded window is focused again.

Basically, unshading puts the window into a mode where it only has keyboard focus when the mouse is over the window.  Putting the mouse anywhere else removes focus, though it doesn't send focus anywhere else.  My original impression that it just didn't get focus was because I unshade by double-clicking the titlebar, and the titlebar is not in the window.
Comment 2 Shaun McCance 2007-01-26 15:33:18 UTC
One more data point: out of curiosity, I added a keybinding to toggle the shaded state.  Oddly enough, this behavior does not happen when using that keybinding.
Comment 3 Havoc Pennington 2007-01-26 16:26:36 UTC
the quasi-sloppy is probably caused by RevertToPointerRoot (hint for anyone debugging this)
Comment 4 Shaun McCance 2007-02-02 17:12:58 UTC
So I'm able to fix this by inserting the following at line 1244 of frames.c, inside the part of meta_frames_button_press_event that handles shade toggling:

meta_core_end_grab_op (gdk_display, event->time);

Based on my observation from comment #2, I just tried to see what was different between double-clicking and using a keybinding, and it pretty much came down to *something* in that function.  I thought the META_GRAB_OP_MOVING grab op might be interfering, so I tried releasing it, and it works.

Note that I don't know all the inracacies of window management, and I have no idea if this is the right fix.  Somebody who knows what he's doing should run with this.
Comment 5 Shaun McCance 2007-03-16 21:30:02 UTC
Created attachment 84741 [details] [review]
possible fix

Attaching a patch per my previous comment.  It's moved because of the patches from Linus that were applied.
Comment 6 Elijah Newren 2007-04-08 03:12:18 UTC
I've spent a long time on this and still can't figure it out.  I have one main question stumping me:

  How is it possible that the focus window is becoming invalid during
  an *unshade* operation?  I assume that is what is happening, since
  we're essentially seeing RevertToPointerRoot behavior.  But the
  revert_to argument of XSetInputFocus shouldn't be used unless the
  focus window becomes unviewable; I can see why that would happen for
  a shade operation but not for an unshade operation.

So, here's some notes I took while trying to figure things out:

1) We have meta_window_focus() calls all over the place.  A double-click on the titlebar results in *six* calls to meta_window_focus().  (The user clicked on the titlebar, so we focus the window.  We're about to grab the pointer/keyboard to do a move/resize/whatever operation, so we focus the window.  We're going to unshade the window, so we focus the window. etc.)

2) gtk+/gdk/x11/gdkwindow-x11.c contains an interesting tidbit; apparently they have a focus_window separate from the rest of their app for focus:
  /* We use an extra X window for toplevel windows that we XSetInputFocus()
   * to in order to avoid getting keyboard events redirected to subwindows
   * that might not even be part of this app
   */
  Window focus_window;
Is something funny happening to us as well when we focus the frame window?

3) How Shaun's patch works around the issue: If any window grab is in effect, meta_window_focus() bails early without bothering to try to focus any window (the assumption seems to be that a different window is grabbed, but no special casing exists for when the grabbed window is the window that was requested to be focused).  With Shaun's patch, the last of the 6 meta_window_focus() calls comes after the grab has ended so the call isn't ignored and the client window is focused.

4) Why this never showed up earlier:  It wouldn't surprise me if we used to have 10 calls to meta_window_focus().  We probably had one or two triggered directly from meta_display_end_grab_op().
Comment 7 Shaun McCance 2007-10-13 16:51:36 UTC
Ping?  I only remember this bug when I install/upgrade a system, and then have to build metacity by hand with my patch.  I imagine there's a few people out there who are just living (annoyed) with this bug.

I understand that there's probably some insidious goings-on that manifest themselves in this bug, and that my patch is a cheap bandage.  But I have been running with my patch for about seven months now.  It does fix the symptom, and seemingly with no other ill effects.
Comment 8 Elijah Newren 2007-10-13 19:00:13 UTC
Um, go ahead and apply the patch for now but please add a FIXME comment above it and point to this bug (and in particular, comment #6).

Thanks Shaun.
Comment 9 Thomas Thurman 2008-04-08 12:18:45 UTC
Was this patch ever applied?
Comment 10 Shaun McCance 2008-04-08 15:56:09 UTC
Nope, sorry.  I always completely forget about it until I upgrade.
Comment 11 Thomas Thurman 2008-09-09 00:33:16 UTC
I don't see this in 2.25.0 (using right-click bound to toggle_shade).  Shaun, do you still see it?
Comment 12 Shaun McCance 2008-09-09 13:31:55 UTC
I haven't tried 2.25 yet.  I can confirm it's still a problem in Fedora 9, but that's only metacity 2.22, I think.

Note that I'm not sure the problem would be triggered by using right-click to unshade.  I know I didn't have any problems using a keybinding to unshade.  The problem was that the left click induced a grab operation, which caused some sort of odd conflict.  I never quite figured out the conflict, but I was able to get around it by ending the grab operation.  I think it went something like this:

click ->
  start grab op
  double-click ->
    double-click handler ->
      unshade
      focus
      end grab op

And the "focus" step failed because there was a grab operation.  My quick fix was to insert a step as follows:

click ->
  start grab op
  double-click ->
    end grab op ** NEW **
    double-click handler ->
      unshade
      focus
      end grab op

If right clicks don't induce a grab operation, they wouldn't trigger this problem.

That said, I took a glance at the code to see if my reasoning would stand up, and it's all changed.  I'm not even sure how to apply this patch anymore.
Comment 13 Thomas Thurman 2008-09-10 01:59:20 UTC
Created attachment 118407 [details] [review]
possible fix

Well, here's a version of the same thing which applies now.  Let us know whether this gives you ideas.
Comment 14 Shaun McCance 2008-10-05 22:47:39 UTC
I still get the bug with SVN trunk (rev 3938).  Your patch fixes it.
Comment 15 Shaun McCance 2008-10-15 05:09:02 UTC
I want to point out that I've found a consistent crasher in the build of metacity I'm running.  I need to check whether it's caused by this patch.  If so, this patch shouldn't be applied as is.  Otherwise, I'll file a separate bug.
Comment 16 Christian Persch 2008-10-21 23:36:16 UTC
*** Bug 524687 has been marked as a duplicate of this bug. ***
Comment 17 Shaun McCance 2008-12-19 04:02:47 UTC
OK, I don't know what was up with that crasher I was experiencing before, but I can't reproduce it with SVN trunk either with or without the patch.

This patch fixes the problem for me.  Thanks for updating it.

Comment 18 Thomas Thurman 2008-12-19 12:47:53 UTC
Committed, then.  Thanks.