Fix dialog OK button event issues on OSX.

Move the OSX dialog fix ups later so they come after some wxWindow
deferred processing.

Also provides a facility for doing a selectAll in each text field
so that tabbing between text fields behaves correctly.

Fixes: lp:1599157
* https://bugs.launchpad.net/kicad/+bug/1599157

common/dialog_shim.cpp

@@ -58,6 +58,7 @@ DIALOG_SHIM::DIALOG_SHIM( wxWindow* aParent, wxWindowID id, const wxString& titl
         const wxPoint& pos, const wxSize& size, long style, const wxString& name ) :
     wxDialog( aParent, id, title, pos, size, style, name ),
     KIWAY_HOLDER( 0 ),
+    m_fixupsRun( false ),
     m_qmodal_loop( 0 ),
     m_qmodal_showing( false ),
     m_qmodal_parent_disabler( 0 )
@@ -85,9 +86,7 @@ DIALOG_SHIM::DIALOG_SHIM( wxWindow* aParent, wxWindowID id, const wxString& titl
         Pgm().App().SetTopWindow( parent_kiwayplayer );
 #endif
 
-#if DLGSHIM_USE_SETFOCUS
+    Connect( wxEVT_PAINT, wxPaintEventHandler( DIALOG_SHIM::OnPaint ) );
-    Connect( wxEVT_INIT_DIALOG, wxInitDialogEventHandler( DIALOG_SHIM::onInit ) );
-#endif
 }
 
 
@@ -114,20 +113,6 @@ void DIALOG_SHIM::FinishDialogSettings()
     Center();
 }
 
-void DIALOG_SHIM::FixOSXCancelButtonIssue()
-{
-#ifdef  __WXMAC__
-    // A ugly hack to fix an issue on OSX: ctrl+c closes the dialog instead of
-    // copying a text if a  button with wxID_CANCEL is used in a wxStdDialogButtonSizer
-    // created by wxFormBuilder: the label is &Cancel, and this accelerator key has priority
-    // to copy text standard accelerator, and the dlg is closed when trying to copy text
-    wxButton* button = dynamic_cast< wxButton* > ( wxWindow::FindWindowById( wxID_CANCEL, this ) );
-
-    if( button )
-        button->SetLabel( _( "Cancel" ) );
-#endif
-}
-
 
 // our hashtable is an implementation secret, don't need or want it in a header file
 #include <hashtables.h>
@@ -175,8 +160,6 @@ bool DIALOG_SHIM::Show( bool show )
         ret = wxDialog::Show( show );
     }
 
-    FixOSXCancelButtonIssue();
-
     return ret;
 }
 
@@ -194,58 +177,93 @@ bool DIALOG_SHIM::Enable( bool enable )
 }
 
 
-#if DLGSHIM_USE_SETFOCUS
+// Traverse all items in the dialog.  If selectTextInTextCtrls, do a SelectAll()
-
+// in each so that tab followed by typing will replace the existing value.
-static bool findWindowRecursively( const wxWindowList& children, const wxWindow* wanted )
+// Also collects the firstTextCtrl and the item with focus (if any).
+static void recursiveDescent( wxWindowList& children, const bool selectTextInTextCtrls,
+                              wxWindow* & firstTextCtrl, wxWindow* & windowWithFocus )
 {
-    for( wxWindowList::const_iterator it = children.begin();  it != children.end();  ++it )
+    for( wxWindowList::iterator it = children.begin();  it != children.end();  ++it )
     {
-        const wxWindow* child = *it;
+        wxWindow* child = *it;
 
-        if( wanted == child )
+        if( child->HasFocus() )
-            return true;
+            windowWithFocus = child;
-        else
+
+        wxTextCtrl* childTextCtrl = dynamic_cast<wxTextCtrl*>( child );
+        if( childTextCtrl )
         {
-            if( findWindowRecursively( child->GetChildren(), wanted ) )
+            if( !firstTextCtrl && childTextCtrl->IsEnabled() && childTextCtrl->IsEditable() )
-                return true;
+                firstTextCtrl = childTextCtrl;
+
+            if( selectTextInTextCtrls )
+            {
+                wxTextEntry* asTextEntry = dynamic_cast<wxTextEntry*>( childTextCtrl );
+                // Respect an existing selection
+                if( asTextEntry->GetStringSelection().IsEmpty() )
+                    asTextEntry->SelectAll();
+            }
         }
-    }
 
-    return false;
+        recursiveDescent( child->GetChildren(), selectTextInTextCtrls, firstTextCtrl,
+                          windowWithFocus );
+    }
 }
 
-
+#ifdef  __WXMAC__
-static bool findWindowRecursively( const wxWindow* topmost, const wxWindow* wanted )
+static void fixOSXCancelButtonIssue( wxWindow *aWindow )
 {
-    // wanted may be NULL and that is ok.
+    // A ugly hack to fix an issue on OSX: cmd+c closes the dialog instead of
+    // copying the text if a button with wxID_CANCEL is used in a
+    // wxStdDialogButtonSizer created by wxFormBuilder: the label is &Cancel, and
+    // this accelerator key has priority over the standard copy accelerator.
+    wxButton* button = dynamic_cast<wxButton*>( wxWindow::FindWindowById( wxID_CANCEL, aWindow ) );
 
-    if( wanted == topmost )
+    if( button )
-        return true;
+        button->SetLabel( _( "Cancel" ) );
-
-    return findWindowRecursively( topmost->GetChildren(), wanted );
-}
-
-
-/// Set the focus if it is not already set in a derived constructor to a specific control.
-void DIALOG_SHIM::onInit( wxInitDialogEvent& aEvent )
-{
-    wxWindow* focusWnd = wxWindow::FindFocus();
-
-    // If focusWnd is not already this window or a child of it, then SetFocus().
-    // Otherwise the derived class's constructor SetFocus() already to a specific
-    // child control.
-
-    if( !findWindowRecursively( this, focusWnd ) )
-    {
-        // Linux wxGTK needs this to allow the ESCAPE key to close a wxDialog window.
-        SetFocus();
-    }
-
-    aEvent.Skip();     // derived class's handler should be called too
 }
 #endif
 
 
+void DIALOG_SHIM::OnPaint( wxPaintEvent &event )
+{
+    if( !m_fixupsRun )
+    {
+#if DLGSHIM_SELECT_ALL_IN_TEXT_CONTROLS
+        const bool selectAllInTextCtrls = true;
+#else
+        const bool selectAllInTextCtrls = false;
+#endif
+        wxWindow* firstTextCtrl = NULL;
+        wxWindow* windowWithFocus = NULL;
+
+        recursiveDescent( GetChildren(), selectAllInTextCtrls, firstTextCtrl,
+                          windowWithFocus );
+
+#if DLGSHIM_USE_SETFOCUS
+        // While it would be nice to honour any focus already set (which was
+        // recorded in windowWithFocus), the reality is that it's currently wrong
+        // far more often than it's right.
+        // So just focus on the first text control if we have one; otherwise the
+        // focus on the dialog itself, which will at least allow esc, return, etc.
+        // to function.
+        if( firstTextCtrl )
+            firstTextCtrl->SetFocus();
+        else
+            SetFocus();
+#endif
+
+#ifdef  __WXMAC__
+        fixOSXCancelButtonIssue( this );
+#endif
+
+        m_fixupsRun = true;
+    }
+
+    event.Skip();
+}
+
+
 /*
     Quasi-Modal Mode Explained:
 

include/dialog_shim.h

@@ -38,6 +38,16 @@
 #define DLGSHIM_USE_SETFOCUS      0
 #endif
 
+#ifdef __WXMAC__
+/**
+ * MACOS requires this option so that tabbing between text controls will
+ * arrive with the text selected.
+ **/
+#define DLGSHIM_SELECT_ALL_IN_TEXT_CONTROLS     1
+#else
+#define DLGSHIM_SELECT_ALL_IN_TEXT_CONTROLS     0
+#endif
+
 class WDO_ENABLE_DISABLE;
 class EVENT_LOOP;
 
@@ -96,6 +106,8 @@ public:
 
     bool Enable( bool enable ) override;
 
+    void OnPaint( wxPaintEvent &event );
+
 protected:
 
     /**
@@ -115,15 +127,7 @@ protected:
      */
     void FinishDialogSettings();
 
-    /** A ugly hack to fix an issue on OSX:
+    bool m_fixupsRun;
-     * when typing ctrl+c in a wxTextCtrl inside a dialog, it is closed instead of
-     * copying a text if a button with wxID_CANCEL is used in a wxStdDialogButtonSizer,
-     * when the dlg is created by wxFormBuilder:
-     * the label is &Cancel, and this accelerator key has priority
-     * to copy text standard accelerator, and the dlg is closed when trying to copy text
-     * this function do nothing on other platforms
-     */
-    void FixOSXCancelButtonIssue();
 
     std::string m_hash_key;     // alternate for class_map when classname re-used.
 
@@ -131,11 +135,6 @@ protected:
     EVENT_LOOP*         m_qmodal_loop;      // points to nested event_loop, NULL means not qmodal and dismissed
     bool                m_qmodal_showing;
     WDO_ENABLE_DISABLE* m_qmodal_parent_disabler;
-
-#if DLGSHIM_USE_SETFOCUS
-private:
-    void    onInit( wxInitDialogEvent& aEvent );
-#endif
 };
 
 #endif  // DIALOG_SHIM_