Merge pull request #612 from turtleli/ui-event-handling-fixes

gui: Dialog handling fixes and cleanup

common/include/Utilities/wxGuiTools.h

@@ -512,11 +512,11 @@ public:
     void Init( const pxDialogCreationFlags& cflags );
 	void AddOkCancel( wxSizer& sizer, bool hasApply=false );
 	void AddOkCancel( wxSizer* sizer=NULL, bool hasApply=false );
+	void RememberPosition();
 
 	virtual void SmartCenterFit();
 	virtual int ShowModal();
 	virtual bool Show( bool show=true );
-	virtual bool Destroy();
 
 	// Must return the same thing as GetNameStatic; a name ideal for use in uniquely
 	// identifying dialogs.  (this version is the 'instance' version, which is called
@@ -536,7 +536,6 @@ public:
 protected:
 	void OnDialogCreated( wxCommandEvent& evt );
 	void OnOkCancel(wxCommandEvent& evt);
-	void OnCloseWindow(wxCloseEvent& event);
 
 	bool ShouldPreventAppExit() const { return false; }
 

common/src/Utilities/wxHelpers.cpp

@@ -149,10 +149,16 @@ wxDialogWithHelpers::~wxDialogWithHelpers() throw()
 
 void wxDialogWithHelpers::Init( const pxDialogCreationFlags& cflags )
 {
+	// Note to self: if any comments indicate platform specific behaviour then
+	// ifdef them out to see if they fix the issue. I wasted too much time
+	// figuring out why the close box wouldn't work on wxGTK modal dialogs that
+	// had a minimise button.
+#if _WIN32
 	// This fixes it so that the dialogs show up in the task bar in Vista:
 	// (otherwise they go stupid iconized mode if the user minimizes them)
 	if( cflags.hasMinimizeBox )
 		SetExtraStyle(GetExtraStyle() & ~wxTOPLEVEL_EX_DIALOG);
+#endif
 
 	m_extraButtonSizer	= NULL;
 
@@ -161,16 +167,10 @@ void wxDialogWithHelpers::Init( const pxDialogCreationFlags& cflags )
 		delete wxHelpProvider::Set( new wxSimpleHelpProvider() );
 #endif
 
-	// GTK/Linux Note: currently the Close (X) button doesn't appear to work in dialogs.  Docs
-	// indicate that it should, so I presume the problem is in wxWidgets and that (hopefully!)
-	// an updated version will fix it later.  I tried to fix it using a manual Connect but it
-	// didn't do any good.  (problem could also be my Co-Linux / x-window manager)
-
 	Connect( pxEvt_OnDialogCreated,	wxCommandEventHandler	(wxDialogWithHelpers::OnDialogCreated) );
 
 	Connect( wxID_OK,		wxEVT_COMMAND_BUTTON_CLICKED,	wxCommandEventHandler	(wxDialogWithHelpers::OnOkCancel) );
 	Connect( wxID_CANCEL,	wxEVT_COMMAND_BUTTON_CLICKED,	wxCommandEventHandler	(wxDialogWithHelpers::OnOkCancel) );
-	Connect(				wxEVT_CLOSE_WINDOW,				wxCloseEventHandler		(wxDialogWithHelpers::OnCloseWindow) );
 
 	wxCommandEvent createEvent( pxEvt_OnDialogCreated );
 	createEvent.SetId( GetId() );
@@ -286,7 +286,7 @@ pxStaticText& wxDialogWithHelpers::Heading( const wxString& label )
 	return *new pxStaticHeading( this, label );
 }
 
-bool wxDialogWithHelpers::Destroy()
+void wxDialogWithHelpers::RememberPosition()
 {
 	// Save the dialog position if the dialog is named...
 	// FIXME : This doesn't get called if the app is exited by alt-f4'ing the main app window.
@@ -311,20 +311,18 @@ bool wxDialogWithHelpers::Destroy()
 			saver.Entry( dlgName + L"_Pos", pos, screenRect.GetPosition() );
 		}
 	}
-
-	return _parent::Destroy();
-}
-
-void wxDialogWithHelpers::OnCloseWindow( wxCloseEvent& evt )
-{
-	if( !IsModal() ) Destroy();
-	evt.Skip();
 }
 
 void wxDialogWithHelpers::OnOkCancel( wxCommandEvent& evt )
 {
-	Close();
+	RememberPosition();
-	evt.Skip();
+
+	// Modal dialogs should be destroyed after ShowModal returns, otherwise there
+	// might be double delete problems if the dialog was declared on the stack.
+	if (!IsModal())
+		Destroy();
+	else
+		evt.Skip();
 }
 
 void wxDialogWithHelpers::AddOkCancel( wxSizer &sizer, bool hasApply )

pcsx2/gui/Dialogs/BaseConfigurationDialog.cpp

@@ -144,8 +144,6 @@ Dialogs::BaseConfigurationDialog::BaseConfigurationDialog( wxWindow* parent, con
 	Connect( wxID_APPLY,	wxEVT_COMMAND_BUTTON_CLICKED,	wxCommandEventHandler( BaseConfigurationDialog::OnApply_Click ) );
 	Connect( wxID_SAVE,		wxEVT_COMMAND_BUTTON_CLICKED,	wxCommandEventHandler( BaseConfigurationDialog::OnScreenshot_Click ) );
 
-	Connect(				wxEVT_CLOSE_WINDOW,				wxCloseEventHandler(BaseConfigurationDialog::OnCloseWindow) );
-
 	Connect( pxEvt_SetSettingsPage, wxCommandEventHandler( BaseConfigurationDialog::OnSetSettingsPage ) );
 
 	// ----------------------------------------------------------------------------
@@ -230,14 +228,6 @@ void Dialogs::BaseConfigurationDialog::OnSomethingChanged( wxCommandEvent& evt )
 		SomethingChanged();
 }
 
-
-void Dialogs::BaseConfigurationDialog::OnCloseWindow( wxCloseEvent& evt )
-{
-	if( !IsModal() ) Destroy();
-	evt.Skip();
-}
-
-
 void Dialogs::BaseConfigurationDialog::AllowApplyActivation( bool allow )
 {
 	m_allowApplyActivation = allow;
@@ -280,7 +270,6 @@ void Dialogs::BaseConfigurationDialog::OnApply_Click( wxCommandEvent& evt )
 	SomethingChanged_StateModified_IsChanged();
 }
 
-//avih: FIXME: ? for some reason, this OnCancel_Click is called twice when clicking cancel or closing the dialog (Jake's code?).
 void Dialogs::BaseConfigurationDialog::OnCancel_Click( wxCommandEvent& evt )
 {
 	//same as for Ok/Apply: let SysConfigDialog clean-up the presets and derivatives (menu system) if needed.

pcsx2/gui/Dialogs/ConfigurationDialog.h

@@ -76,7 +76,6 @@ namespace Dialogs
 		void OnCancel_Click( wxCommandEvent& evt );
 		void OnApply_Click( wxCommandEvent& evt );
 		void OnScreenshot_Click( wxCommandEvent& evt );
-		void OnCloseWindow( wxCloseEvent& evt );
 
 		void OnSetSettingsPage( wxCommandEvent& evt );
 		void OnSomethingChanged( wxCommandEvent& evt );

pcsx2/gui/Dialogs/ConfirmationDialogs.cpp

@@ -298,10 +298,11 @@ ModalButtonPanel::ModalButtonPanel( wxWindow* parent, const MsgButtons& buttons
 
 void ModalButtonPanel::OnActionButtonClicked( wxCommandEvent& evt )
 {
-	evt.Skip();
 	wxWindow* toplevel = wxGetTopLevelParent( this );
 	if( wxDialog* dialog = wxDynamicCast( toplevel, wxDialog ) )
 		dialog->EndModal( evt.GetId() );
+	// If the dialog doesn't close, and you're using it for a modeless dialog - hint:
+	// read the name of the class. ;)
 }
 
 void ModalButtonPanel::AddCustomButton( wxWindowID id, const wxString& label )

pcsx2/gui/MessageBoxes.cpp

@@ -181,30 +181,12 @@ namespace Msgbox
 	// true if OK, false if cancel.
 	bool OkCancel( const wxString& text, const wxString& caption, int icon )
 	{
-		MsgButtons buttons( MsgButtons().OKCancel() );
+		return ShowModal(caption, text, MsgButtons().OKCancel()) == wxID_OK;
-
-		if( wxThread::IsMain() )
-		{
-			return wxID_OK == pxMessageDialog( caption, text, buttons );
-		}
-		else
-		{
-			return wxID_OK == ShowModal( caption, text, buttons );
-		}
 	}
 
 	bool YesNo( const wxString& text, const wxString& caption, int icon )
 	{
-		MsgButtons buttons( MsgButtons().YesNo() );
+		return ShowModal(caption, text, MsgButtons().YesNo()) == wxID_YES;
-
-		if( wxThread::IsMain() )
-		{
-			return wxID_YES == pxMessageDialog( caption, text, buttons );
-		}
-		else
-		{
-			return wxID_YES == ShowModal( caption, text, buttons );
-		}
 	}
 
 	int Assertion( const wxString& text, const wxString& stacktrace )