Fix subdialogs of the ISO props dialog... sort of #2509
Conversation
On OS X, if you close a subdialog of the ISO Properties dialog, such as the one to add a new AR code, the main Dolphin window would magically get raised above ISO Properties. This is confusing, to say the least; when I encountered this the other day, I thought the dialog was actually getting closed. I *think* the diagnosis looks like this: Cocoa expects NSPanel (not to be confused with wxPanel) to be for things like find dialogs, font dialogs with the little title bars, sheets, etc. See: https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/WinPanel/Concepts/ChangingMainKeyWindow.html Therefore, NSPanels return NO for canBecomeMainWindow, which is documented; and when [NSWindow orderOut:] is called to hide a window, Cocoa seems to want to make the window it focuses in its place a main window, which, as far as I can tell, is not. So if the next highest window is a panel, it gets skipped over. (I tested this by overriding wxNSPanel's canBecomeMainWindow to return YES, in which case the right window gets focused, but this isn't a correct fix.) The ISO Properties dialog does have grounds to be a dialog/panel - the close button, whose positioning is provided by the wxDialog class. This is arguably simply a roundabout discovery that our UI sucks for an OS X app and that to be consistent with other nonmodal preferences dialogs, it shouldn't have such a button on OS X (though ESC to close is still kosher). However, I'm not willing to make that change right now, so... Hack around the problem by calling Raise (on this) after each call to ShowModal in CISOProperties. The resulting behavior is slightly glitchy, and I'd like to revisit it, but for now it fixes the issue.
|
If both solutions are rather hacky, why not choose the shorter canBecomeMainWindow one? Or does that come with other issues that |
|
The obvious way to do it would require modifying wx. Can be done in a more hacky way from elsewhere, but I'd rather wait until I can make it stop being a dialog altogether - in the meantime, the symptoms of this issue are pretty bad for the 3 users on OS X, so... |
|
The bomb is now set with a fuse of 3 days. If it reaches 0, I merge regardless of review. Therefore, please review! |
|
Given that I didn't realize this PR existed (it wasn't mentioned in https://code.google.com/p/dolphin-emu/issues/detail?id=8572, which @JMC47 prodded me to fix), I wrote #2556 which is basically exactly the same patch, give or take some minutiae, and had a very similar analysis of the bug. This gets my +1, although a wxMac bug report should probably be filed regardless. (Also while I was in the code I fixed a few minor issues, but those shouldn't present an issue if they don't get merged.) |
Fix subdialogs of the ISO props dialog... sort of
On OS X, if you close a subdialog of the ISO Properties dialog, such as
the one to add a new AR code, the main Dolphin window would magically
get raised above ISO Properties. This is confusing, to say the least;
when I encountered this the other day, I thought the dialog was actually
getting closed.
I think the diagnosis looks like this:
Cocoa expects NSPanel (not to be confused with wxPanel) to be for things
like find dialogs, font dialogs with the little title bars, sheets, etc.
See:
https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/WinPanel/Concepts/ChangingMainKeyWindow.html
Therefore, NSPanels return NO for canBecomeMainWindow, which is
documented; and when [NSWindow orderOut:] is called to hide a window,
Cocoa seems to want to make the window it focuses in its place a main
window, which, as far as I can tell, is not. So if the next highest
window is a panel, it gets skipped over. (I tested this by overriding
wxNSPanel's canBecomeMainWindow to return YES, in which case the right
window gets focused, but this isn't a correct fix.)
The ISO Properties dialog does have grounds to be a dialog/panel - the
close button, whose positioning is provided by the wxDialog class. This
is arguably simply a roundabout discovery that our UI sucks for an OS X
app and that to be consistent with other nonmodal preferences dialogs,
it shouldn't have such a button on OS X (though ESC to close is still
kosher). However, I'm not willing to make that change right now, so...
Hack around the problem by calling Raise (on this) after each call to
ShowModal in CISOProperties. The resulting behavior is slightly
glitchy, and I'd like to revisit it, but for now it fixes the issue.