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 158626 - Motif based apps will lost main-window focus when main-menu popups.
Motif based apps will lost main-window focus when main-menu popups.
Status: RESOLVED OBSOLETE
Product: metacity
Classification: Other
Component: Focus
2.8.x
Other Linux
: High normal
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
Depends on:
Blocks: 155450
 
 
Reported: 2004-11-18 16:10 UTC by Huang, Zhangrong
Modified: 2020-11-06 20:07 UTC
See Also:
GNOME target: ---
GNOME version: 2.7/2.8


Attachments
Patch that fixes observed behavior (681 bytes, patch)
2004-12-26 23:41 UTC, Elijah Newren
needs-work Details | Review
Call meta_window_notify_focus when calling XSetInputFocus instead of waiting for FocusIn/FocusOut/UnmapNotify events (13.60 KB, patch)
2005-02-13 01:11 UTC, Elijah Newren
rejected Details | Review

Description Huang, Zhangrong 2004-11-18 16:10:22 UTC
Try some motif based apps with menu-bar, like ddd, acroread, when you click the
menu-bar, the main-menu will popup, but the app main-window will also lost focus.
Comment 1 Elijah Newren 2004-12-26 23:41:35 UTC
Created attachment 35210 [details] [review]
Patch that fixes observed behavior

In meta_window_notify_focus(), we handle a variety of cases.  In particular, we
try to ignore events generated by a grab.  Our method of doing so was to filter
out xfocus.mode's of NotifyGrab and NotifyUngrab.  It turns out that motif apps
like acroread also send events with xfocus.mode of NotifyWhileGrabbed.	Adding
that to the list of things to filter out in this function fixes the observed
problem.  (Tested with acroread only, though)

However, it makes me wonder if we have similar bugs lurking elsewhere due to
not handling NotifyWhileGrabbed.  The only place it appears in the code
currently is the following snippet from display.c, function
meta_event_mode_to_string:
      /* not sure any X implementations are missing this, but
       * it seems to be absent from some docs.
       */
  #ifdef NotifyWhileGrabbed
      case NotifyWhileGrabbed:
	mode = "NotifyWhileGrabbed";
	break;
  #endif

Since I really don't know what NotifyWhileGrabbed is or why or when it would be
generated, I did a quick Google on it.	All I really discovered is that Google
knows about Luminocity code already (Owen appears to be handling things with
this mode throughout his code), and a somewhat obscure reference in Xlib
documentation at http://tronche.com/gui/x/.
Comment 2 Elijah Newren 2004-12-27 01:23:25 UTC
Comment on attachment 35210 [details] [review]
Patch that fixes observed behavior

With this patch, alt-tabbing seems to be unable to focus the newly selected
window (though it is raised).  So something else needs to be done.  Comments
from someone who understands NotifyWhileGrabbed and the various cases that we
need to handle better are appreciated...
Comment 3 Havoc Pennington 2004-12-28 05:11:21 UTC
I don't remember which bug discusses it, but Owen has spent a lot of time for
GTK+ trying to perfect 100% correct tracking of which window has focus. At one
time I was planning to copy that code into metacity from gdk. I imagine he has
similar knowledge embedded in luminocity. 
Comment 4 Elijah Newren 2004-12-28 19:32:28 UTC
Owen: Could you provide any pointers here?
Comment 5 Elijah Newren 2005-02-13 01:10:22 UTC
Okay, so as mentioned in bug 167199 I wrote up a patch that overhauls
meta_window_notify_focus, and this is something that overhaul fixes so I figured
sticking it here was as good a place as any (I could have opened a new bug, but
then I might just be marking this one as a duplicate...).  The basic idea is:

  Call meta_window_notify_focus when calling XSetInputFocus, instead of waiting 
  for FocusIn events.

This is made simple since we have already wrapped XSetInputFocus (bug 154598).

Advantages of doing this:
  1) Makes meta_window_notify_focus much easier to understand, removing the
     guesswork about whether we've covered all the modes and details
  2) Fixes this bug without causing any regressions that I have been able to
     find
  3) Will remove the confusion and worry about window->has_focus,
     display->focus_window, and display->expected_focus_window (this makes it
     so that display->focus_window==display->expected_focus_window and 
     window->has_focus==(window==display->expected_focus_window)), which will
     allow us to simplify a lot of code
  4) I believe this is halfway towards the "separate metacity focus from
     keyboard focus" idea in bug 90382, if we decide that we actually need it
     (we'd just need to add display->keyboard_focus_window that responds to
     FocusIn/FocusOut events).
  5) I think this will make it feasible to retry solving bug 88194

Disadvantages/caveats:
  1) This is a fairly core part of the system to change this drastically; while
     I have tested some and it seems okay, we'd probably want to get some more
     thorough testing of it.
  2) Rogue apps could screw things up by calling XSetInputFocus on some window.
     Granted, that screws things up anyway even for the current code (stealing
     focus like this is a security issue as well as being extremely annoying),
     but it'd be a little bit worse with this change because we would miss the
     notification from it and the wrong titlebar would be highlighted which
     would require the user to click on some window to straighten things out.
     (We may be able to add code to try to detect and workaround this problem
     at least partially; I believe KWin does this just as part of focus
     stealing prevention.)

I'll attach the patch in a minute.
Comment 6 Elijah Newren 2005-02-13 01:11:43 UTC
Created attachment 37422 [details] [review]
Call meta_window_notify_focus when calling XSetInputFocus instead of waiting for FocusIn/FocusOut/UnmapNotify events
Comment 7 Havoc Pennington 2005-02-13 20:41:52 UTC
It's broken to do this in my opinion. We need to handle other apps moving the focus.

The X protocol spec has a bunch of text that could be helpful about what all the
various focus events mean and when they are generated, if we are failing to
correctly track current focus.

To track it correctly we may need to know when focus is on an unmanaged window,
so perhaps focus_window should be an XID instead of or in addition to a
MetaWindow, I don't know.

I think the right approach is this:

 - be sure there are variables in metacity that accurately reflect current 
   state; e.g. focus_window is always the last event received, and 
   expected_focus_window is always our last SetInputFocus

 - if we want to do an abstraction to avoid dealing with focus_window, 
   expected_focus_window, etc. in the code, then write a function that
   abstracts them; but we should still have the accurate information 
   available to use.
Comment 8 Elijah Newren 2005-02-14 06:31:38 UTC
Yeah, unmanaged windows is a case I didn't think of.  This patch might not work
in that case (though it'd be nice to have a testcase to try it out with...)

Could you point me to the document and location in it that you're thinking of in
regards to tracking focus?  I tried looking around for that kind of information
but seem to have come up short-handed; I was probably just looking in the wrong
place, though.  Hopefully there is something like that which will help, because
right now I think this bug (and the more general bug of getting focus tracking
correct) may be extremely difficult to solve without something like this patch.
 Clicking on a menu of a Motif app appears to be indistinguishable from using
Alt-Tab to focus a window; thus the only other fix I can think of right now
would be to check for whether we did a grab op ourselves--but the problem with
that approach is that the grab op ends before any FocusIn/FocusOut event is
received which means that this approach would require some kind of timing and
queueing mechanism which really sounds problematic to me...

However, maybe someone will think of something clever.  I'll just mark the patch
according to your comments to get it off the patch-diligence report.  :)
Comment 9 Havoc Pennington 2005-02-14 14:45:09 UTC
What you want is /usr/share/doc/xorg-x11-doc-6.8.1/XProtocol/proto.PS.gz
on Fedora, Debian has it too but probably in a different place.

You want pages 74 and 75 in my version, they list all the possible focus changes
and which focus events will be generated for which windows in each case.
Comment 10 Thomas Thurman 2008-04-09 03:06:53 UTC
In ddd, this occurs in metacity and kwin; it does not occur in twm or icewm.
Comment 11 Thomas Thurman 2008-04-14 02:41:04 UTC
Havoc: I assume this is the excerpt you referred to?  (It's pp80-81 in my edition.)

Normal and WhileGrabbed events are generated as follows:

When the focus moves from window A to window B, A is an inferior of B, and the pointer is in window P:

* FocusOut with detail Ancestor is generated on A.
* FocusOut with detail Virtual is generated on each window between A and B exclusive (in order).
* FocusIn with detail Inferior is generated on B.
* If P is an inferior of B but P is not A or an inferior of A or an ancestor of A, FocusIn with detail Pointer is generated on each window below B down to and including P (in order).

When the focus moves from window A to window B, B is an inferior of A, and the pointer is in window P:

* If P is an inferior of A but P is not an inferior of B or an ancestor of B, FocusOut with detail Pointer is generated on each window from P up to but not including A (in order).
* FocusOut with detail Inferior is generated on A.
* FocusIn with detail Virtual is generated on each window between A and B exclusive (in order).
* FocusIn with detail Ancestor is generated on B.

When the focus moves from window A to window B, window C is their least common ancestor, and the pointer is in window P:

* If P is an inferior of A, FocusOut with detail Pointer is generated on each window from Pup to but not including A (in order).
* FocusOut with detail Nonlinear is generated on A.
* FocusOut with detail NonlinearVirtual is generated on each window between A and C exclusive (in order).
* FocusIn with detail NonlinearVirtual is generated on each window between C and B exclusive (in order).
* FocusIn with detail Nonlinear is generated on B.
* If P is an inferior of B, FocusIn with detail Pointer is generated on each window below B down to and including P (in order).
* If P is an inferior of A, FocusOut with detail Pointer is generated on each window from P up to but not including A (in order).
* FocusOut with detail Nonlinear is generated on A.
* If A is not a root window, FocusOut with detail NonlinearVirtual is generated on each window above A up to and including its root (in order).
* If B is not a root window, FocusIn with detail NonlinearVirtual is generated on each window from B's root down to but not including B (in order).
* FocusIn with detail Nonlinear is generated on B.
* If P is an inferior of B, FocusIn with detail Pointer is generated on each window below B down to and including P (in order).

When the focus moves from window A to PointerRoot (or None) and the pointer is in window P:

* If P is an inferior of A, FocusOut with detail Pointer is generated on each window from P up to but not including A (in order).
* FocusOut with detail Nonlinear is generated on A.
* If A is not a root window, FocusOut with detail NonlinearVirtual is generated on each window above A up to and including its root (in order).
* FocusIn with detail PointerRoot (or None) is generated on all root windows.
* If the new focus is PointerRoot, FocusIn with detail Pointer is generated on each window from P's root down to and including P (in order).

When the focus moves from PointerRoot (or None) to window A and the pointer is in window P:

* If the old focus is PointerRoot, FocusOut with detail Pointer is generated on each window from P up to and including P's root (in order).
* FocusOut with detail PointerRoot (or None) is generated on all root windows.
* If A is not a root window, FocusIn with detail NonlinearVirtual is generated on each window from A's root down to but not including A (in order).
* FocusIn with detail Nonlinear is generated on A.
* If P is an inferior of A, FocusIn with detail Pointer is generated on each window below A down to and including P (in order).

When the focus moves from PointerRoot to None (or vice versa) and the pointer is in window P:

* If the old focus is PointerRoot, FocusOut with detail Pointer is generated on each window from P up to and including P's root (in order).
* FocusOut with detail PointerRoot (or None) is generated on all root windows.
* FocusIn with detail None (or PointerRoot) is generated on all root windows.
* If the new focus is PointerRoot, FocusIn with detail Pointer is generated on each window from P's root down to and including P (in order).

When a keyboard grab activates (but before generating any actual KeyPress event that activates the grab), G is the grab-window for the grab, and F is the current focus:

* FocusIn and FocusOut events with mode Grab are generated (as for Normal above) as if the focus were to change from F to G.

When a keyboard grab deactivates (but after generating any actual KeyRelease event that deactivates the grab), G is the grab-window for the grab, and F is the current focus:

* FocusIn and FocusOut ev ents with mode Ungrab are generated (as for Normal above) as if the focus were to change from G to F.
Comment 12 Thomas Thurman 2008-04-20 20:19:38 UTC
Moving things to focus component.  Sorry for spam.
Comment 13 André Klapper 2020-11-06 20:07:20 UTC
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports in Bugzilla which have not seen updates for many years.

If you can still reproduce this issue in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/metacity/-/issues/

Thank you for reporting this issue and we are sorry it could not be fixed.