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 112404 - Problem with focus when closing transient window
Problem with focus when closing transient window
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Win32
2.10.x
Other Windows
: Normal normal
: Medium fix
Assigned To: Cody Russell
gtk-bugs
: 107319 107323 124095 157779 321355 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2003-05-06 13:40 UTC by Arnaud Charlet
Modified: 2007-07-13 02:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
work around patch (735 bytes, patch)
2003-05-06 13:42 UTC, Arnaud Charlet
none Details | Review
A test program I am trying to use to reproduce the problem (2.20 KB, text/plain)
2006-12-13 18:19 UTC, Cody Russell
  Details
Test using GtkDialog instead of GtkWindow (2.21 KB, text/plain)
2006-12-13 18:21 UTC, Cody Russell
  Details
mail messages about transient window patch (81.85 KB, text/plain)
2006-12-13 19:25 UTC, Tor Lillqvist
  Details
Altered test program to create several levels of children (2.35 KB, text/plain)
2006-12-26 15:29 UTC, Robert Ögren
  Details
Test program by Bulia Byak. Not exactly about the specific issue in this bug, but related (2.10 KB, text/plain)
2006-12-29 11:43 UTC, Tor Lillqvist
  Details
Bulia Byak's sample program with fix (2.13 KB, text/plain)
2007-01-02 00:46 UTC, Tor Lillqvist
  Details
Suggested patch (5.04 KB, patch)
2007-01-04 01:19 UTC, Tor Lillqvist
needs-work Details | Review
Further improvements to Cody's/Robert's test program (3.43 KB, text/plain)
2007-01-08 03:05 UTC, Tor Lillqvist
  Details
Modified Tor's test program (3.78 KB, text/plain)
2007-01-08 07:14 UTC, Cody Russell
  Details
Proposed updated patch (5.44 KB, patch)
2007-01-08 07:26 UTC, Cody Russell
none Details | Review
Updated patch (5.54 KB, patch)
2007-01-09 00:16 UTC, Cody Russell
none Details | Review
New proposed patch (11.25 KB, patch)
2007-02-28 20:41 UTC, Cody Russell
none Details | Review
Same as previous patch, just cleaned up a bit (10.68 KB, patch)
2007-02-28 20:44 UTC, Cody Russell
none Details | Review
Minor update (11.69 KB, patch)
2007-03-05 17:12 UTC, Cody Russell
none Details | Review
Small update (11.41 KB, patch)
2007-03-05 18:16 UTC, Cody Russell
none Details | Review
Iterate over child transients list during WM_MOUSEACTIVATE event (11.85 KB, patch)
2007-03-08 00:45 UTC, Cody Russell
none Details | Review
Make sure to set return_val = TRUE (11.88 KB, patch)
2007-03-08 00:50 UTC, Cody Russell
none Details | Review
And now for something completely different.. (1.48 KB, patch)
2007-07-02 21:31 UTC, Cody Russell
none Details | Review
New patch (6.55 KB, patch)
2007-07-02 23:00 UTC, Cody Russell
none Details | Review
Update (7.20 KB, patch)
2007-07-03 02:09 UTC, Cody Russell
none Details | Review
Cleanup update (6.72 KB, patch)
2007-07-03 03:12 UTC, Cody Russell
none Details | Review
Update (6.63 KB, patch)
2007-07-04 01:20 UTC, Cody Russell
none Details | Review
More minor fixes :) (6.68 KB, patch)
2007-07-04 01:54 UTC, Cody Russell
none Details | Review
Update (6.33 KB, patch)
2007-07-04 02:28 UTC, Cody Russell
none Details | Review
Simplified (4.48 KB, patch)
2007-07-04 02:35 UTC, Cody Russell
none Details | Review
Cleaned up warnings and removed unused vars (4.29 KB, patch)
2007-07-04 02:40 UTC, Cody Russell
none Details | Review
Updated test program (3.72 KB, text/plain)
2007-07-04 09:00 UTC, Tor Lillqvist
  Details
Updated patch (4.73 KB, patch)
2007-07-10 20:40 UTC, Cody Russell
none Details | Review
Adds check to see if window being closed is the foreground window (5.86 KB, patch)
2007-07-12 19:57 UTC, Cody Russell
accepted-commit_now Details | Review

Description Arnaud Charlet 2003-05-06 13:40:55 UTC
Under Windows, using Gtk+ 2.2.1

Now that modal windows are really modal (thanks to the change in
gdk_window_set_transient_for()) wrt to their parent, there is a new
problem appearing: in most cases, when closing a transient window,
the focus is given back to another application's window, instead of
going back to the parent's window.

This behavior can get very annoying soon when using these dialogs.

The patch I'll attach works around the problem and works reasonably well,
although this is really a temporary measure until the underlying problem
is understood.

Arno
Comment 1 Arnaud Charlet 2003-05-06 13:42:00 UTC
Created attachment 16305 [details] [review]
work around patch
Comment 2 Owen Taylor 2003-06-05 18:00:12 UTC

*** This bug has been marked as a duplicate of 107319 ***
Comment 3 Tor Lillqvist 2004-01-28 02:59:09 UTC
Does this (focus going back to the transient windows's "owner" (or 
whatever one should call it) when the transient window closes match 
what X11 window managers do?
Comment 4 Tor Lillqvist 2004-02-02 22:36:13 UTC
Are you sure that GWL_HWNDPARENT really gets the parent? Doesn't it 
get the window's owner? (See other use of GWL_HWNDPARENT in gdkwindow-
win32.c.) Or are the modal windows in question in fact also 
transient_for windows, and thus their GWL_HWNDPARENT has been set?

(Reopening so that this bug will show up in queries for open bugs.)
Comment 5 James Preston 2004-04-28 04:31:13 UTC
Hi,

Sorry if I get any of the details wrong.  Just wanted to point out that
I'm seeing something very similar to this, but on Linux, Fedora Core 1
(2.4.0) and usually with Opera web browser (Qt impl.).  Very similar repro:
eg.
  1. Start several applications.  It doesn't matter if they are minimised
     or maximised.
  2. With Opera full screen, File->Open.  (Dialog shows on panel with focus.)
  3. Cancel the dialog.
  4. Opera doesn't regain focus, in fact no window has focus.

Opera still accepts mouse events, but ignore key events.  Highlighting text
with the mouse highlights in the 'low light' color, if that makes. You
have to click on the window panel twice (once to minimize, once to re-max)
to get the keyboard working agagin.

Hope this helps,

james.
Comment 6 Michael Urban 2005-01-18 23:59:52 UTC
Basically, focus goes back to whatever window was last active. In other words,
if you have several applications open and you switch to another application
while the modal dialog is still open, then switch back to the GTK application
and close the modal dialog, focus will be given to the other application again
rather than to the parent of the modal dialog.
Comment 7 Arnaud Charlet 2005-02-16 16:54:07 UTC
Any news on this issue ?

Note that the patch I provided to work around the problem only works
when 1 modal dialog is used. It does not work when two modal dialogs
are open on top of each other.

Any hint on how to fix this bug for real would be appreciated.

Arno
Comment 8 Vadim Berezniker 2005-03-08 03:54:42 UTC
Is there any workaround for developers when creating dialogs? 
It's really annoying.
Comment 9 Arnaud Charlet 2005-03-24 13:19:57 UTC
<<
Are you sure that GWL_HWNDPARENT really gets the parent? Doesn't it 
get the window's owner? (See other use of GWL_HWNDPARENT in gdkwindow-
win32.c.) Or are the modal windows in question in fact also 
transient_for windows, and thus their GWL_HWNDPARENT has been set?
>>

Sorry for not responding earlier, I missed this message somehow.

modal windows in question are indeed also transient_for_windows, so that's why
GWL_HWNDPARENT has been set.

Arno
Comment 10 Michael Urban 2005-06-10 23:49:44 UTC
I'd also be interested in knowing if any work has been done on this. It's still
not fixed in Gtk 2.6 for Win32 and it is over two years old. I agree it is
really annoying when focus goes to the wrong window which is not even part of
the same application.

Any chance this could be bumped up to a higher priority? It is one of the most
annoying bugs that Gtk currently has on the Win32 platform in my opinion.
Comment 11 Michael Urban 2005-06-11 16:11:53 UTC
The problem James is having with Linux might be related to this, so I am no
longer sure this is a Win32 specific issue.

Basically, in Win32, if all other windows are minimized except the ones
belonging to the Gtk application, then when the dialog is closed, no window will
have focus, similar to what James is seeing on Linux.

In Win32, it seems the underlying Window manager is simply setting focus back to
whatever window had focus before the Gtk window lost focus, whereas in Linux, it
is simply leaving no window in the application with focus.

Maybe this problem should be reassigned? It doesn't seem like it is a Win32
specific issue anymore. I think the bug James is seeing is the same bug we are
seeing on Win32. It's just showing up in different ways because the window
managers handle lost focus in different ways.
Comment 12 Owen Taylor 2005-06-11 16:14:42 UTC
No this is definitely Win32 specific. if there are X issues, than you
have a separate bug (and most likely a window manager bug)
Comment 13 Michael Urban 2005-06-11 19:43:12 UTC
For those who were asking about a workaround, I did find one that doesn't
require you to patch the Gtk source code itself. You can use the present()
method to explicitly set the focus back to the parent window when the child
dialog closes. Here is a Python example which is the close handler for some
child dialog (should work in C too):

def onDestroy(self):
   self.dialog.destroy()
   self.parent.present()

This will explicitly set the focus back to the parent window of the child dialog
and prevent the loss of focus / random application switch problem.

Hope this helps for those who are experiencing this problem.
Comment 14 Michael Urban 2005-06-11 20:02:12 UTC
Actually, it works better if you reverse the two statements. First set the focus
back to the parent, then destroy the dialog:

def onDestroy(self, *args):
   self.parent.present()
   self.dialog.destroy()

If you destroy the dialog before setting focus back to the parent, you will get
an annoying flash since the other application will get focus for a split second
before the parent window grabs it back. If you set focus to the parent first and
then destroy the dialog, there is no flash and the parent retains focus after
the child is destroyed.

This seems to be a good workaround that you can implement from the application
level (rather than hacking Gtk source code), and so far it doesn't seem to have
any unintended side effects.
Comment 15 Cody Russell 2006-12-13 18:17:53 UTC
I wanted to take a stab at working on this, but so far I have been unable to reproduce it.  Can anyone confirm if this problem still exists, and if so can you please post code for a simple test program to exploit the problem?

I'm using GTK 2.10.6 on Windows and my test program creates a toplevel window with a button; when the button is clicked it creates a child modal window that is transient for the parent.  If I close the child window, focus returns to the parent.  If I click a different window (for another program running) and then close the child window, the focus goes to the previously focused window rather than to the parent window.  This is the same behavior I get in Linux.

I will attach my test program.  Please point out what I'm doing wrong, if anything.
Comment 16 Cody Russell 2006-12-13 18:19:38 UTC
Created attachment 78306 [details]
A test program I am trying to use to reproduce the problem
Comment 17 Cody Russell 2006-12-13 18:21:19 UTC
Created attachment 78307 [details]
Test using GtkDialog instead of GtkWindow

Brad Taylor suggested I also try testing it using GtkDialog.  This is a modified version of the above test, but using GtkDialog instead of GtkWindow.  The results are the same for me using GTK 2.10.6.
Comment 18 Robert Ögren 2006-12-13 18:56:24 UTC
If I remember correctly the problem only occured when you had two levels of child windows, e.g. a main window opened a dialog which then opened another dialog, but it was a while ago (about 2 years probably).

I might have looked into this problem or a similar one and if I remember correctly I came to the conclusion that it didn't work perfectly to change the owner of a window after it was created - you had to pass the owner to CreateWindow. I don't remember if I reported it anywhere. Anyway, that just vague memories, I haven't verified this recently.

See also bug #107319 which seems to talk about two levels of dialogs. 
Comment 19 Tor Lillqvist 2006-12-13 19:24:31 UTC
Hmm, is this related to the old friend, the "transient window" problem? See bug #371036.

I'll include here some pointers to mail messages about a related patch that has been exchanged in the past:

http://lists-archives.org/gtk-devel/01038-transient-windows-on-win32.html
http://mail.gnome.org/archives/gtk-devel-list/2005-October/msg00034.html

Careful googling for keywords like "reilly patch win32 transient" might find more.

Actually much of the mail exchange seems to have been off-list. I'll attach some the mail exchanges I can find as an attachment to this bug.

It has been very hard to determine whether applying the "Reilly patch" is OK or not. I think the latest consensus between me and the Inkscape guys, at least, was that more work is needed and that Inkscape has transient window behaviour problems even in X11.

And then there is the issue of what to do with GIMP's transient window use: GIMP wants to set transient-for even between windows created in different processes. And chain transient-for.

I.e. a window opened by gimp.exe is transient for a window opened by a plug-in (GIMP plug-ins are separate processes), which is transient for a window opened by gimp.exe. Blindly not checking for foreign windows in the Reilly code (and I think also in the current GTK+ code) is known to cause deadlocks.

One more testcase: I think I have noticed that if I run Gaim against a GTK+ with the Reilly patch included, now and then Gaim gets stuck in a TOPMOST state... which is very irritating to say the least (i.e., one then has to minimize it to be able to use the windows it covers), just switching focus doesn't bring them on top. Unfortunately I haven't been able to find out exactly what sequence of events / input causes this to happen...

So, all in all, this is a very complex issue. bratsche's approach, using purpose-written minimal test programs seems like the way one definitely needs to go to be able to get to the bottom of this. Trying to debug or look through GDK debug output of some large application to find out what happens is not fun.
Comment 20 Tor Lillqvist 2006-12-13 19:25:26 UTC
Created attachment 78312 [details]
mail messages about transient window patch
Comment 21 Robert Ögren 2006-12-26 15:29:52 UTC
Created attachment 78908 [details]
Altered test program to create several levels of children

Here's an altered version of Cody's test program in comment #16 that reproduces the problem for me. I do the following:

1. start the program from a rxvt terminal
2. click the create child button
3. click the create child button in the newly created window from step 2
4. close the topmost window
5. close the topmost window

After step 5, the rxvt window is focused instead of the main window of the test program.

I tested with gtk 2.10.6 binaries as well as a really old compilation of the HEAD branch (from 2005-07 or so...)
Comment 22 Tor Lillqvist 2006-12-29 11:35:17 UTC
Thanks Robert for the test program. What do you think, should we use this bug for only the specific problem nicely exhibited by that program, or should we use this for all problems related to modal or non-modal dialogs, transient-for relationships, minimizing oddities for dialogs/parents, etc? They are presumably all related anyway, and fixing one class of problems in one way presumably will affect the others, positively or negatively...

Other related bugs are bug #107323, bug #124095, bug #132501 (?), bug #157679,
bug #157779, bug #164537, bug #169965 at least, maybe even more.
Comment 23 Tor Lillqvist 2006-12-29 11:43:26 UTC
Created attachment 79027 [details]
Test program by Bulia Byak. Not exactly about the specific issue in this bug, but related

Mail from Bulia Byak:

====== begin quote

  Sorry for a long delay. I'm back on this and eager to see this resolved!

  As I wrote earlier, the problems you listed really aren't Inkscape
  problems. However, I understand your reluctance to touch Inkscape code
  - it's huge and messy. So here I attach the simplest possible,
  absolutely straightforward example that demonstrates a transient
  window. Run it and press "Transient dialog" button to open the dialog.
  Please test this example with your patch on Windows and check the
  following aspects of behavior:

  1. The dialog stays on top of the main window, always; but they both
  can be covered by some other window.

  2. The dialog has no separate button in the system task bar and
  application switcher.

  3. The dialog does not block main window; you can still press buttons
  in the main window under the dialog. You can switch focus from main
  window to the dialog and back.

  4. If you open several dialogs by pressing the "Transient dialog"
  button multiple times, ALL these dialogs are on top of the main
  window, but their z-order relative to each other can be switched
  arbitrarily by clicking on them.

  5. Dialogs don't have Minimize window buttons (only Maximize and Close).

  6. If you press Minimize on the main window, both it and ALL its
  dialogs get minimized. When you restore it, EVERYTHING is restored
  exactly as it was before, including dialog positions and z-order.

  7. When you press Close on the main window, everything is destroyed,
  including all visible dialogs.

  As you can see, there's absolutely no hackery or trickery in the
  source - just standard GTK calls. Everything listed above is verified
  and working on Linux with KDE 3.2. Please test this with your patch on
  Windows and let me know if it works. If this example works but
  Inkscape still does not, please let me know where exactly they diverge
  and I'll investigate that.

  Thanks, I really look forward to seeing this fixed soon

====== end quote

What's interesting is that with metacity (2.12.2) on SLED 10, this test program definitely does *not* behave like Bulia says it should. The "Transient dialog" windows do not stay on top of the main window. Sigh. What would be a definitive X11 reference window manager known to implement all this stuff correctly?
Comment 24 Robert Ögren 2006-12-30 02:20:55 UTC
Re Comment #22:

I'm not sure. I don't know enough about all the problems with the current implementation and how it really should work. If you think that the solution that would fix the problem demonstrated by the test case also will solve other issues then that would be great.

In general I personally like the "one report per issue" principle - i.e. I wouldn't want to gradually morph this bug report that is about a specific problem into one that covers all the other ones. It makes the report hard to read and hard to know what is really fixed and what is remaining. 

But I guess that the most important thing is that the job gets done, not exactly in which bug report it is discussed :)


Anyway, to me it looks like the following reports are about exactly the same problem, the behavior I described in comment #21 (approximately: when several levels of modal windows are closed, focus is given to the wrong window):

Bug 112404 (this report) – Problem with focus when closing modal window

Bug 107319 – Main application window set to bottom of Z order when Servers Dialog closed

Bug 107323 – Wrong window ownership for dialogs.

Bug 124095 – Wrong window receives focus after closing a spawned window

If you agree, they should immediately be combined into one to reduce clutter, perhaps marking the last three as duplicate of 112404 and making sure the description of this bug is good. (As I said in an earlier comment I think this problem is caused by the undocumented use of SetWindowLong to change the owner of the window after it has been created, but I can't find any old notes or test programs that prove this. The best way to make sure would probably be to write a minimal sample program using only the Windows API to create modal windows in both ways.)
Comment 25 Tor Lillqvist 2007-01-02 00:46:08 UTC
Created attachment 79158 [details]
Bulia Byak's sample program with fix

If I change the sample program to create the "Transient dialog" window as actually being marked as a dialog (with gtk_dialog_new()), it behaves more closely as Bulia says on GNOME (metacity), too.
Comment 26 Tor Lillqvist 2007-01-04 01:19:15 UTC
Created attachment 79348 [details] [review]
Suggested patch

This is actually rather close to Arno's first patch, I guess. This seems to fix the actual problem this bug is about, but not the other problems related to window transience and modality. (The patch also adds some more verbose logging in a few places, and removes an #if 0 block, sorry.)
Comment 27 Tor Lillqvist 2007-01-04 01:25:06 UTC
The above patch also removes the code in gdk_window_set_modal_hint() that sets modal mapped windows to TOPMOST. Surely that can't be right? Please tell if you can figure out what I (or whoever wrote that code) had been thinking... could it really be necessary?
Comment 28 Cody Russell 2007-01-05 00:23:27 UTC
This patch seems to behave very well in the testing I've done so far.

I think you're right about removing TOPMOST from the modal hint.  It seems like they didn't really intend for there to be very many topmost Windows on screen at a time (taskbar and application menus mostly).  With your patch it looks like the only way to get TOPMOST is either with a GDK_WINDOW_TEMP (e.g., menu) or with GDK_WINDOW_STATE_ABOVE.

I have another situation I want to test with this patch, but I need to go digging through the bug tracker at work to find the issue.  I'll post a follow up soon.
Comment 29 Cody Russell 2007-01-05 01:43:43 UTC
Nevermind, I can't test that other situation right now after all due to issues in my build process.
Comment 30 Tor Lillqvist 2007-01-08 03:05:04 UTC
Created attachment 79702 [details]
Further improvements to Cody's/Robert's test program

Added more descriptive titles to the windows and labels to buttons. Titles now indicate relationships. By defining MODAL and/or TRANSIENT you can compile several versions, that set or don't set the modal hint, and set or don't set transient-for.

I also add buttons to the mail window to destroy children programmatically, to verify that a fix for the focus problem isn't "too effective". If a transient window is closed when it does not have focus, focus should of course stay where it is, not suddenly jump to the transient's owner, like in my patch above.

So the patch is not usable as such. Unfortunately just straightforwardly checking with GetFocus() whether the window that is being destroyed has the focus before explicitly setting focus to the transient owner doesn't work either, as by the time we get to WM_DESTROY, focus has already jumped somwhere else.

"modal" and "transient-for" are separate concepts, so changing the Summary. It's only when closing several levels windows that are transient-for (have a owner, i.e. GWL_HWNDPARENT) that focus unexpectedly jumps. Closing windows that are just modal oesn't do anything unexpected (obviously, as setting the modal hint doesn't do anything currently in gdk/win32...).
Comment 31 Tor Lillqvist 2007-01-08 03:12:35 UTC
*** Bug 107323 has been marked as a duplicate of this bug. ***
Comment 32 Tor Lillqvist 2007-01-08 03:16:13 UTC
*** Bug 124095 has been marked as a duplicate of this bug. ***
Comment 33 Tor Lillqvist 2007-01-08 03:17:35 UTC
Note: See also patch in #124095, for yet another idea.
Comment 34 Tor Lillqvist 2007-01-08 03:18:25 UTC
*** Bug 107319 has been marked as a duplicate of this bug. ***
Comment 35 Tor Lillqvist 2007-01-08 03:28:40 UTC
Btw, a related problem is that I think even if gtk's destroy-with-parent is *not* set, transient, i.e. owned windows, will still be destroyed by Windows when their owner is destroyed. MSDN says "The system automatically destroys an owned window when its owner is destroyed." 

So how can we make gdk/win32 know whether the gtk-level destroy-with-parent is on or not? Argh.
Comment 36 Cody Russell 2007-01-08 07:12:35 UTC
With this patch, if the transient dialog being destroyed does not have focus then it will still focus its parent when its destroy event occurs.  I have modified Tor's version of the test program to help exploit this problem.  It does not set the transient window for the mainwindow.. and if you compile with MODAL undefined you can see test this problem:

1/ From "Parent Window", click "Create Transient Child"
2/ From "Transient Child Window /1" click "Create Transient Child"
3/ Focus back to "Parent Window"
4/ Click "Delayed Kill Child /1/0"

You should see the remaining child focus now.

I'm attaching the modified test program below, and I'll attach an updated patch that fixes this issue.
Comment 37 Cody Russell 2007-01-08 07:14:56 UTC
Created attachment 79712 [details]
Modified Tor's test program

This just disables setting the transient dialog on the mainwindow.
Comment 38 Cody Russell 2007-01-08 07:26:33 UTC
Created attachment 79714 [details] [review]
Proposed updated patch

This should solve the above described problem.  This patch is made against the current svn of GTK.
Comment 39 Tor Lillqvist 2007-01-08 12:58:00 UTC
Doesn't the test for current_window that you add work only if it is the top-level transient-for window that is current_window? Can't current_window as well be some child window inside that toplevel?
Comment 40 Cody Russell 2007-01-09 00:16:23 UTC
Created attachment 79800 [details] [review]
Updated patch

Okay, I see what you mean.. I think that testing gdk_window_is_ancestor() would fix this, right?
Comment 41 Tor Lillqvist 2007-01-09 10:39:03 UTC
Actually, sorry I didn't think of this already yesterday, but looking at current_window is not the right thing to do. current_window tracks the window the pointer is over, mainly so that gdk/win32 can synthesize proper enter/leave GDK events. It is not related to keyboard focus at all.
Comment 42 Cody Russell 2007-02-28 20:41:43 UTC
Created attachment 83583 [details] [review]
New proposed patch

I've come up with a new patch that I hope will solve this issue for everyone.  Could any interested parties apply this and torture test it?
Comment 43 Cody Russell 2007-02-28 20:44:46 UTC
Created attachment 83584 [details] [review]
Same as previous patch, just cleaned up a bit

Sorry about that.  Just cleaned out a couple things from the code that I had commented out.  Functionally the same patch.
Comment 44 Cody Russell 2007-03-05 17:12:54 UTC
Created attachment 83980 [details] [review]
Minor update
Comment 45 Cody Russell 2007-03-05 18:16:26 UTC
Created attachment 83991 [details] [review]
Small update
Comment 46 Cody Russell 2007-03-08 00:45:32 UTC
Created attachment 84203 [details] [review]
Iterate over child transients list during WM_MOUSEACTIVATE event

Thanks Tor.  This corrects the issue of only acting transient for the first transient child.
Comment 47 Cody Russell 2007-03-08 00:50:18 UTC
Created attachment 84205 [details] [review]
Make sure to set return_val = TRUE

Sets return_val = TRUE
Comment 48 Cody Russell 2007-07-02 21:31:14 UTC
Created attachment 91062 [details] [review]
And now for something completely different..

Here's another patch using a totally different, much simpler approach.
Comment 49 Cody Russell 2007-07-02 23:00:53 UTC
Created attachment 91065 [details] [review]
New patch

Sorry about the last one, I was totally on crack.

Anyway, this is a new patch that seems to actually make improvements.  I'm not doing away with the GWL_HWNDPARENT flag in this patch in the way that I was with attachments 84205 and its predecessors.  This patch keeps GWL_HWNDPARENT, but also sets up the transient_owner and transient_children relationships.  What I've added now is a new a handler for WM_CLOSE that checks for transient_owner and calls SetForegroundWindow(transient_owner) if it exists.
Comment 50 Cody Russell 2007-07-03 02:09:57 UTC
Created attachment 91072 [details] [review]
Update
Comment 51 Cody Russell 2007-07-03 02:27:09 UTC
*** Bug 321355 has been marked as a duplicate of this bug. ***
Comment 52 Cody Russell 2007-07-03 03:12:47 UTC
Created attachment 91076 [details] [review]
Cleanup update
Comment 53 Cody Russell 2007-07-04 01:20:06 UTC
Created attachment 91150 [details] [review]
Update
Comment 54 Cody Russell 2007-07-04 01:54:52 UTC
Created attachment 91153 [details] [review]
More minor fixes :)
Comment 55 Cody Russell 2007-07-04 02:28:17 UTC
Created attachment 91154 [details] [review]
Update
Comment 56 Cody Russell 2007-07-04 02:35:35 UTC
Created attachment 91155 [details] [review]
Simplified

We don't really need to track transient_children anymore, so removing that.  The WM_ACTIVATE stuff was a relic from the previous patch effort and was causing another undesired effect.
Comment 57 Cody Russell 2007-07-04 02:40:34 UTC
Created attachment 91156 [details] [review]
Cleaned up warnings and removed unused vars

Sorry for all the Bugzilla spam.  This is a cleanup patch.
Comment 58 Tor Lillqvist 2007-07-04 09:00:44 UTC
Created attachment 91164 [details]
Updated test program

Corrected the test program so that the titles and button texts actually reflect the modality and/or transiency correctly. (I hope.)

To save us from confusion, and to ensure we can without risk of confusion talk about the different configurations about the test program, save test program as "112404.c" and compile four versions with something like:

gcc                     -o 112404-0.exe `pkg-config --cflags gtk+-2.0` 112404.c `pkg-config --libs gtk+-2.0`


gcc -DMODAL -DTRANSIENT -o 112404-1.exe `pkg-config --cflags gtk+-2.0` 112404.c `pkg-config --libs gtk+-2.0`

gcc         -DTRANSIENT -o 112404-2.exe `pkg-config --cflags gtk+-2.0` 112404.c `pkg-config --libs gtk+-2.0`

gcc -DMODAL             -o 112404-3.exe `pkg-config --cflags gtk+-2.0` 112404.c `pkg-config --libs gtk+-2.0`
Comment 59 Tor Lillqvist 2007-07-04 09:32:44 UTC
The 112404-*.exe test programs seem to behave sanely and as one can expect with regards to focus/activeness movement when windows are closed. Both if closed explicitly with Alt-F4 or clicking the close button on the title bar, or if closed programmatically by clicking one of the "Delayed Kill" buttons on the main window (and after that making some other window active).

(Hmmm, maybe we should have a totally separate window with the delayed kill buttons as in the Modal cases it's impossible to click on the "Parent Window's" buttons.)

One odd thing: run 112404-1, press spacebar four times, so that you have "Parent Window", "Modal Child Window /0", "Modal Transient Child Window /0/0", "Modal Transient Child Window /0/0/0" and "Modal Transient Child Window /0/0/0/0". Then click on the "Parent Window" title bar. The window raises and can be moved on top of the "Child" windows (is this correct?), and its title bar indicates it is the active window. Then press spacebar. Whoops, the button on the "Modal Transient Child Window /0/0/0/0" gets pressed. IMHO this is against reasonable expectation. Either it should not be possible to make a window that has a modal "child" active, or if it is made active, keyboard events should be directed to it (and ignored, like mouse clicks on it are). Or what?

Another unclear issue is the presence of minimize and maximize buttons. At least Bulia Byak's test program in comment #23 does not behave at all as he says in the quoted mail that it should in that regard.

Should it be possible to minimize or maximize windows that have transient and/or modal children open?

(Hmm, is it confusing to talk about "children", as we are not talking about child windows in the GDK_WINDOW_CHILD sense...? Is there a better term?)
Comment 60 Cody Russell 2007-07-04 12:13:25 UTC
"One odd thing: run 112404-1, press spacebar four times, so that you have
"Parent Window", "Modal Child Window /0", "Modal Transient Child Window /0/0",
"Modal Transient Child Window /0/0/0" and "Modal Transient Child Window
/0/0/0/0". Then click on the "Parent Window" title bar. The window raises and
can be moved on top of the "Child" windows (is this correct?), and its title
bar indicates it is the active window. Then press spacebar. Whoops, the button
on the "Modal Transient Child Window /0/0/0/0" gets pressed. IMHO this is
against reasonable expectation. Either it should not be possible to make a
window that has a modal "child" active, or if it is made active, keyboard
events should be directed to it (and ignored, like mouse clicks on it are). Or
what?"

The keyboard input definitely needs to be addressed, and I'll look at that later this afternoon.

However, as far as making the parent window active.. I think it is doing exactly what we told it to do.  The parent window has "allow_transient" set to FALSE, which is what the test program uses to determine whether to call set_transient_for().  Then when we create a new transient child, it becomes the top of the chain.  There is kind of a "popping" effect then because in order to show the new transient child it needs to raise its chain of owners.  I think it would be incorrect to have a z-order of:
   Modal Transient Child /0/0/0/0/0
   Parent Window
   Modal Transient Child /0/0/0/0
   Modal Transient Child /0/0/0
   Modal Transient Child /0/0
   Modal Transient Child /0

What do you think?
Comment 61 Cody Russell 2007-07-04 21:39:19 UTC
"Should it be possible to minimize or maximize windows that have transient
and/or modal children open?"

Yes, I think so.  At least for non-modal transient windows, I would think so.  For modal windows, it's less clear.

Another related question is, if you minimize a window that owns one or more other windows then should those owned windows also be minimized?  Right now they're not, but the owner is being iconified so that when it is restored it returns behind its owned windows as expected.  On my Linux box with Metacity, minimizing a window that has a transient/owned child results in that child window also being minimized and then restored with the parent when the user restores it.  I would like to find an example of a native Windows program that is using modal windows and test to see how this functions.  If it turns out we want to mimic the same behavior as Metacity, then I'm inclined to ask if I can open a different bug# for that because otherwise I feel like the current patch is a vast improvement over what's going on in GTK right now.
Comment 62 Cody Russell 2007-07-10 17:48:11 UTC
I verified that the keyboard input bug in modal windows is not related to my patch, I was able to reproduce it using stock GTK.  I'd like to fix it as well, but I'd rather keep the patches separate.  Created bug #455627.
Comment 63 Cody Russell 2007-07-10 20:40:26 UTC
Created attachment 91576 [details] [review]
Updated patch

Tor gave me the green light to commit, but here is one final update to the patch before I commit.  This patch introduces SetForeGroundWindow() under WM_SHOWWINDOW message if event->any.type == GDK_UNMAP.  The reason for this is because our test programs worked fine when you click the [X] to close them, which sends WM_CLOSE, but if you have a dialog with an "Ok" button and hide the dialog when it responds then it will not send WM_CLOSE.
Comment 64 Tor Lillqvist 2007-07-12 19:46:13 UTC
Looks good. One question though: Should the SetForegroundWindow() perhaps be done only if the window that is being unmapped (or closed) at that moment *is* the foreground window? (In both cases where you call it.) Otherwise if the user is working in some different app, and in the background the GTK+ app is doing some work and closes a transient progress window or something, won't focus then unexpectedly jump to the GTK+ app's window that was the owner of the transient window?
Comment 65 Cody Russell 2007-07-12 19:57:09 UTC
Created attachment 91694 [details] [review]
Adds check to see if window being closed is the foreground window

Yeah, that makes sense.  This patch checks if the window being closed is the foreground window.
Comment 66 Tor Lillqvist 2007-07-12 23:24:55 UTC
Looks good, please commit.
Comment 67 Cody Russell 2007-07-12 23:41:53 UTC
2007-07-12  Cody Russell  <bratsche@gnome.org>

        * gdk/win32/gdkevents-win32.c
        * gdk/win32/gdkwindow-win32.[ch]: Fix transient windows on Win32 
        so that when a transient child window is closed (particularly when 
        there are 3 or more levels of transient windows), the correct window
        receives focus rather than a seemingly random window. (#112404) 
Comment 68 Cody Russell 2007-07-13 02:09:01 UTC
*** Bug 157779 has been marked as a duplicate of this bug. ***