Patch #2894 2009-12-30 23:46
techy
Set parent window of cbMessageBox where possible- Download
- 2894-Set_parent_win.patch (41.5 KB)
Index: src/plugins/scriptedwizard/wizpage.cpp
===================================================================
--- src/plugins/scriptedwizard/wizpage.cpp (revision 5987)
+++ src/plugins/scriptedwizard/wizpage.cpp (working copy)
@@ -273,7 +273,7 @@
if (m_Filename.IsEmpty() || !wxDirExists(wxPathOnly(m_Filename)))
{
- cbMessageBox(_("Please select a filename with full path for your new file..."), _("Error"), wxICON_ERROR);
+ cbMessageBox(_("Please select a filename with full path for your new file..."), _("Error"), wxICON_ERROR, GetParent());
event.Veto();
return;
}
@@ -347,19 +347,19 @@
wxString title = m_pProjectPathPanel->GetTitle();
// if (!wxDirExists(dir))
// {
-// cbMessageBox(_("Please select a valid path to create your project..."), _("Error"), wxICON_ERROR);
+// cbMessageBox(_("Please select a valid path to create your project..."), _("Error"), wxICON_ERROR, GetParent());
// event.Veto();
// return;
// }
if (title.IsEmpty())
{
- cbMessageBox(_("Please select a title for your project..."), _("Error"), wxICON_ERROR);
+ cbMessageBox(_("Please select a title for your project..."), _("Error"), wxICON_ERROR, GetParent());
event.Veto();
return;
}
if (name.IsEmpty())
{
- cbMessageBox(_("Please select a name for your project..."), _("Error"), wxICON_ERROR);
+ cbMessageBox(_("Please select a name for your project..."), _("Error"), wxICON_ERROR, GetParent());
event.Veto();
return;
}
@@ -368,10 +368,10 @@
if (cbMessageBox(_("A project with the same name already exists in the project folder.\n"
"Are you sure you want to use this directory (files may be OVERWRITTEN)?"),
_("Confirmation"),
- wxICON_QUESTION | wxYES_NO) != wxID_YES)
+ wxICON_QUESTION | wxYES_NO, GetParent()) != wxID_YES)
{
// cbMessageBox(_("A project with the same name already exists in the project folder.\n"
-// "Please select a different project name..."), _("Warning"), wxICON_WARNING);
+// "Please select a different project name..."), _("Warning"), wxICON_WARNING, GetParent());
event.Veto();
return;
}
@@ -436,7 +436,7 @@
wxString dir = Manager::Get()->GetMacrosManager()->ReplaceMacros(m_pGenericSelectPath->txtFolder->GetValue());
if (!wxDirExists(dir))
{
- cbMessageBox(_("Please select a valid location..."), _("Error"), wxICON_ERROR);
+ cbMessageBox(_("Please select a valid location..."), _("Error"), wxICON_ERROR, GetParent());
event.Veto();
return;
}
@@ -458,7 +458,7 @@
: WizPageBase(_T("CompilerPage"), parent, bitmap),
m_AllowConfigChange(allowConfigChange)
{
- m_pCompilerPanel = new CompilerPanel(this);
+ m_pCompilerPanel = new CompilerPanel(this, GetParent());
wxArrayString valids = GetArrayFromString(validCompilerIDs, _T(";"), true);
wxString def = compilerID;
@@ -568,13 +568,13 @@
{
if (GetCompilerID().IsEmpty())
{
- wxMessageBox(_("You must select a compiler for your project..."), _("Error"), wxICON_ERROR);
+ cbMessageBox(_("You must select a compiler for your project..."), _("Error"), wxICON_ERROR, GetParent());
event.Veto();
return;
}
if (m_AllowConfigChange && !GetWantDebug() && !GetWantRelease())
{
- wxMessageBox(_("You must select at least one configuration..."), _("Error"), wxICON_ERROR);
+ cbMessageBox(_("You must select at least one configuration..."), _("Error"), wxICON_ERROR, GetParent());
event.Veto();
return;
}
@@ -688,7 +688,7 @@
{
if (m_pBuildTargetPanel->GetCompilerCombo()->IsShown() && GetCompilerID().IsEmpty())
{
- wxMessageBox(_("You must select a compiler for your build target..."), _("Error"), wxICON_ERROR);
+ cbMessageBox(_("You must select a compiler for your build target..."), _("Error"), wxICON_ERROR, GetParent());
event.Veto();
return;
}
@@ -696,7 +696,7 @@
cbProject* theproject = Manager::Get()->GetProjectManager()->GetActiveProject(); // can't fail; if no project, the wizard didn't even run
if (theproject->GetBuildTarget(m_pBuildTargetPanel->GetTargetName()))
{
- wxMessageBox(_("A build target with that name already exists in the active project..."), _("Error"), wxICON_ERROR);
+ cbMessageBox(_("A build target with that name already exists in the active project..."), _("Error"),
download for full patch...
History
(About the opposite of the previous patch - huge, boring, a lot work to do and not visibly useful)
There are two main reasons that lead me to writing this patch:
1. With old linuxes the message box without parent ***ISN'T*** (the stars are there to make it look cool :-) modal - it can be hidden by the main application window, which is quite unpleasant - several times I ended with "totally unresponsive" codeblocks and then just realizing that there was some dialog in the background.
2. Even with recent linuxes when the message box is displayed from within a dialog, you can click on the dialog window and hide the message box - which could lead to similar problem as above. So message boxes should have dialogs as their parents whenever possible.
So what I did was:
1. When cbMessageBox is invoked from a dialog, the dialog should be its parent (about 90pct of the patch is adding "this" as a parent of the message box)
2. In cbMessageBox itself I set the parent to Manager::Get()->GetAppWindow() if no explicit parent is provided so if the main window is available, it should be a parent of the window so nothing should hide behind it. (I had to move the implementation from the header because there is now circular dependency between manager.h and globals.h)
3. I added a kind of hack into cbConfigurationPanel - I added a cbMessageBox method with identical interface as the one in globlas.h. It is used to fool the plugins that implement a configuration panel so when they invoke cbMessageBox from within a class inherited from cbConfigurationPanel, it actually calls the cbMessageBox method of this class and uses the config dialog as its parent. I added a getter method as well so the plugins could (and should) use it to obtain the dialog itself for instance when not calling cbMessageBox from within the class.
Of course, not all cases are fixed by this patch - for instance when a plugin calls a sdk method that displays a message box from within another dialog, the dialog cannot be the parent of the message box. However, still it fixes the problem in quite many cases and the message box should not hide behind the main window any more.
I'd like to do something similar with other wxDialog based windows that codeblocks uses - I'd just like to know your opinion in order not to do useless things.
C::B does not compile anymore after applying this patch. Reason (configurationpanel.h):
In file included from D:\Devel\CodeBlocks\src\sdk\configurationpanel.cpp:10:
include/configurationpanel.h: In member function 'int cbConfigurationPanel::cbMessageBox(const wxString&, const wxString&, int, wxWindow*, int, int)':
include/configurationpanel.h:48: error: '::cbMessageBox' has not been declared
include/configurationpanel.h:50: error: '::cbMessageBox' has not been declared
Try a full rebuild (delete all PCH's before) to reproduce.
Ok - the compilation error was easy to fix (missing #include "globals.h" only ;-)).
About the wxDialog stuff: We are testing a patch that replaces all wxDialog with wxScrollingDialog (you can try yourself using the scintilla branch). So I'd rather propose to wait until tis is applied. Otherwise it would result in a serious conflict not so easy to solve.Probably it's obsolete then anyways...
(I'm usually compiling codeblocks from commandline make so maybe the configure scripts create different makefiles than codeblocks [like usage of precompiled headers], which would explain why I was able to compile it.)
I've just tested the scintilla branch and I agree that the settings dialogs should be scrollable somehow (I can't reach the OK and Cancel buttons under Linux on my 1280x800 resolution notebook display). But I don't think that the whole dialog should be scrollable - the listbook icons are already scrollable so it's quite strange to have them scrollable twice (together with the whole dialog). Wouldn't it be better to have a scrollbar for cbConfigurationPanel only?
Anyway, this thing looks like pretty unrelated to what I was planning to do. By "wxDialog based windows" I meant mainly the "small" dialogs like wxProgressDialog, wxTextEntryDialog, wxSingleChoiceDialog and so on and the only thing I wanted to do was to specify parent wondows for them (if they are invoked from a dialog, it would be the dialog, otherwise Manager::Get()->GetAppWindow()). This prevents them from being hidden by the parent window.
Any progress on this patch?
The MessageBox behind the main window is pretty annoying problem on linux...
Unfortunately the patch doesn't apply in trunk any more :(
It's still working in my local copy, hence I am unsure about the modifications needed for plugins. Not only the once of our repo but also external once. I am careful not to break all of them...
But probably it's really time for a commit...