Bug 336485

Comment 1 Scott Kovatch 2011-02-07 12:24:15 EST

(In reply to comment #0)
> HEAD Cocoa
> 
> - Open a child shell with PRIMARY_MODAL (e.g. in ControlExample)
> - Via Exposé, select the parent shell
> => Parent shell gets activated, but keyboard interaction doesn't work
> => Expected: Child shell should be active
> 
> I would also expect that the child shell is shown as a separate window in
> Exposé, at least if it's MODELESS.

This is because we used NSWindow#addChildWindow to implement parent/child relationships. Setting a parent/child relationship groups the windows so they always appear together in Expose. Cocoa doesn't have the same window modality modes that Carbon does. I agree this could be better than it is right now.

Having said that, we should be able to at least select the child modal shell like we do in Carbon.

Comment 2 Scott Kovatch 2011-02-07 12:33:14 EST

(In reply to comment #0)
> HEAD Cocoa
> 
> - Open a child shell with PRIMARY_MODAL (e.g. in ControlExample)
> - Via Exposé, select the parent shell
> => Parent shell gets activated, but keyboard interaction doesn't work
> => Expected: Child shell should be active
> 
> I would also expect that the child shell is shown as a separate window in
> Exposé, at least if it's MODELESS.

After more thought on this, I think this is only half-correct. Yes, the child shell should be shown as a separate window, but the parent shell should NOT be selectable via Expose, since it technically can't be selected. 

That's arguably the better behavior in Carbon, too, and would be the better fix for the problem because then we aren't trying to activate another window in the middle of handling an activate event.

The entire concept of 'document modal' is not that common on the Mac -- I remember we had to implement it ourselves in the AWT because Cocoa doesn't really support it.

Comment 3 Markus Keller 2011-02-07 13:12:13 EST

I don't know how Cocoa-native the "Open" dialogs from TextEdit and from SWT are, but they are both modal and show the same behavior in Expose: Only the main window is visible in Expose, but when I click it, the "Open" dialog is activated.

In contrast, TextEdit's modeless Find dialog is a separate window in Expose.

BTW: If I have two open TextEdit documents and then open the "Open" dialog, I can change the z-order of the documents via Expose, but the documents still don't take focus.

Comment 4 Scott Kovatch 2011-02-07 17:04:03 EST

(In reply to comment #3)
> I don't know how Cocoa-native the "Open" dialogs from TextEdit and from SWT
> are, but they are both modal and show the same behavior in Expose: Only the
> main window is visible in Expose, but when I click it, the "Open" dialog is
> activated.

Interesting... I see this too, but I find that surprising. The file dialog is modal, so it is pushed into a higher window level to ensure it is always above the document windows.

But, that's the least of the problems right now, because....

> BTW: If I have two open TextEdit documents and then open the "Open" dialog, I
> can change the z-order of the documents via Expose, but the documents still
> don't take focus.

In Cocoa, APPLICATION_MODAL (and probably SYSTEM_MODAL) are thoroughly wrong right now! If I have a collection of modeless Shells and then open a APPLICATION_MODAL shell on top of it I can still click on the modeless shell and bring it forward. It doesn't activate, but that shouldn't be possible.

Comment 5 Silenio Quarti 2011-04-27 11:37:13 EDT

I will investigate this for RC1.

Comment 6 Silenio Quarti 2011-05-09 14:16:22 EDT

Created attachment 195117 [details]
fix

This patch fixes the main problem (Parent shell gets activated, but keyboard interaction doesn't work).

Comment 7 Silenio Quarti 2011-05-09 16:20:09 EDT

>I would also expect that the child shell is shown as a separate window in
>Exposé, at least if it's MODELESS.

I do not think this is possible in Cocoa when there is a child/parent relationship. TextEdit's modeless find dialog works because it does have a parent window.

Releasing this patch to 3.7 RC1 and marking as fixed, since the main problem is addressed.

Comment 8 Silenio Quarti 2011-05-09 16:31:49 EDT

Created attachment 195142 [details]
snippet

Comment 9 Markus Keller 2011-05-10 06:49:57 EDT

Test failures in HEAD look like the

		modal.window.becomeKeyWindow();

should also be surrounded by the focus checking code used in the normal case

	Display display = this.display;
	display.keyWindow = view.window();
	super.becomeKeyWindow(id, sel);
	display.checkFocus();
	display.keyWindow = null;

Comment 10 Silenio Quarti 2011-05-10 10:22:26 EDT

Is this the test failures you are talking about?

http://fullmoon/downloads/drops/I20110509-1200/testresults/html/org.eclipse.swt.tests_macosx.cocoa.x86_5.0.html

Comment 11 Markus Keller 2011-05-10 10:52:01 EDT

Yes. Here's the public link:
http://download.eclipse.org/eclipse/downloads/drops/I20110509-1200/testresults/html/org.eclipse.swt.tests_macosx.cocoa.x86_5.0.html

display.checkFocus(); also sends out events and stores the currentFocusControl, so my guess was that this is missing in the new code.

Comment 12 Silenio Quarti 2011-05-10 11:20:43 EDT

(In reply to comment #11)
> Yes. Here's the public link:
> http://download.eclipse.org/eclipse/downloads/drops/I20110509-1200/testresults/html/org.eclipse.swt.tests_macosx.cocoa.x86_5.0.html
> 
> display.checkFocus(); also sends out events and stores the currentFocusControl,
> so my guess was that this is missing in the new code.

I do not think this is the problem. The call to modal.window.becomeKeyWindow(); will come back to becomeKeyWindow (int /*long*/ id, int /*long*/ sel) for a different window (the modal dialog) and checkFocus() will run properly.

I cannot reproduce the test failures on my machine (10.6). 

Kim, are the tests run on 10.6 or 10.5 now?

Comment 13 Kim Moir 2011-05-10 11:52:45 EDT

Silenio, the tests are run on 10.6 now.

Comment 14 Markus Keller 2011-05-10 12:19:33 EDT

An interesting point could be that the tests are run on a 64bit machine but with -d32 and the 32bit SWT.
Or maybe it was just a Software Update dialog or a Screensaver that was stealing focus, or something else a young and wild machine likes to do...

Comment 15 Kim Moir 2011-05-10 16:58:07 EDT

The SWT tests on the mac all passed in build I20110510-2000.

Comment 16 Markus Keller 2011-05-20 05:35:11 EDT

Verified in I20110519-1138.