Can't use EndModal() to cancel a modal dialog that hasn't been shown yet (KICAD-6YE).

(This particular case happens when the user cancels
the progress dialog while loading symbols before the
symbol picker has been displayed.  3.5K people have
done this 13K times on MSW.)

eeschema/dialogs/dialog_symbol_chooser.cpp

@@ -42,17 +42,20 @@ DIALOG_SYMBOL_CHOOSER::DIALOG_SYMBOL_CHOOSER( SCH_BASE_FRAME* aParent, const LIB
                                               const SYMBOL_LIBRARY_FILTER* aFilter,
                                               std::vector<PICKED_SYMBOL>&  aHistoryList,
                                               std::vector<PICKED_SYMBOL>&  aAlreadyPlaced,
-                                              bool aAllowFieldEdits, bool aShowFootprints ) :
+                                              bool aAllowFieldEdits, bool aShowFootprints,
+                                              bool& aCancelled ) :
         DIALOG_SHIM( aParent, wxID_ANY, _( "Choose Symbol" ), wxDefaultPosition, wxDefaultSize,
                      wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER )
 {
     wxBoxSizer* sizer = new wxBoxSizer( wxVERTICAL );
     m_chooserPanel = new PANEL_SYMBOL_CHOOSER( aParent, this, aFilter, aHistoryList, aAlreadyPlaced,
-                                               aAllowFieldEdits, aShowFootprints,
+                                               aAllowFieldEdits, aShowFootprints, aCancelled,
+                                               // Accept handler
                                                [this]()
                                                {
                                                    EndModal( wxID_OK );
                                                },
+                                               // Escape handler
                                                [this]()
                                                {
                                                    EndModal( wxID_CANCEL );

eeschema/dialogs/dialog_symbol_chooser.h

@@ -53,7 +53,7 @@ public:
                            const SYMBOL_LIBRARY_FILTER* aFilter,
                            std::vector<PICKED_SYMBOL>& aHistoryList,
                            std::vector<PICKED_SYMBOL>& aAlreadyPlaced, bool aAllowFieldEdits,
-                           bool aShowFootprints );
+                           bool aShowFootprints, bool& aCancelled );
 
     ~DIALOG_SYMBOL_CHOOSER();
 

eeschema/dialogs/panel_sym_lib_table.cpp

@@ -422,6 +422,7 @@ bool PANEL_SYM_LIB_TABLE::allowAutomaticPluginTypeSelection( wxString& aLibraryP
 bool PANEL_SYM_LIB_TABLE::verifyTables()
 {
     wxString                      msg;
+    int                           cursorCol;
     std::unique_ptr<wxBusyCursor> wait;
     wait.reset( new wxBusyCursor );
 
@@ -430,7 +431,7 @@ bool PANEL_SYM_LIB_TABLE::verifyTables()
         if( !model )
             continue;
 
-        for( int r = 0; r < model->GetNumberRows(); )
+        for( int r = 0; r < model->GetNumberRows(); ++r )
         {
             wxString nick = model->GetValue( r, COL_NICKNAME ).Trim( false ).Trim();
             wxString uri  = model->GetValue( r, COL_URI ).Trim( false ).Trim();
@@ -439,33 +440,35 @@ bool PANEL_SYM_LIB_TABLE::verifyTables()
             if( !nick || !uri )
             {
                 if( !nick && !uri )
-                    msg = _( "A library table row nickname and path cells are empty." );
+                {
+                    msg = _( "Nickname and path cannot be empty." );
+                    cursorCol = COL_NICKNAME;
+                }
                 else if( !nick )
-                    msg = _( "A library table row nickname cell is empty." );
+                {
+                    msg = _( "Nickname cannot be empty." );
+                    cursorCol = COL_NICKNAME;
+                }
                 else
-                    msg = _( "A library table row path cell is empty." );
-
-                wxWindow* topLevelParent = wxGetTopLevelParent( this );
+                {
+                    msg = _( "Path cannot be empty." );
+                    cursorCol = COL_URI;
+                }
 
-                wxMessageDialog badCellDlg( topLevelParent, msg, _( "Invalid Row Definition" ),
-                                            wxYES_NO | wxCENTER | wxICON_QUESTION | wxYES_DEFAULT );
-                badCellDlg.SetExtendedMessage( _( "Empty cells will result in all rows that are "
-                                                  "invalid to be removed from the table." ) );
-                badCellDlg.SetYesNoLabels( wxMessageDialog::ButtonLabel( _( "Remove Invalid Cells" ) ),
-                                           wxMessageDialog::ButtonLabel( _( "Cancel Table Update" ) ) );
+                // show the tabbed panel holding the grid we have flunked:
+                if( model != cur_model() )
+                    m_notebook->SetSelection( model == global_model() ? 0 : 1 );
 
-                wait.reset();
+                m_cur_grid->MakeCellVisible( r, 0 );
+                m_cur_grid->SetGridCursor( r, cursorCol );
 
-                if( badCellDlg.ShowModal() == wxID_NO )
-                    return false;
+                wxWindow* topLevelParent = wxGetTopLevelParent( this );
 
-                wait.reset( new wxBusyCursor );
+                wxMessageDialog errdlg( topLevelParent, msg, _( "Library Table Error" ) );
 
-                // Delete the "empty" row, where empty means missing nick or uri.
-                // This also updates the UI which could be slow, but there should only be a few
-                // rows to delete, unless the user fell asleep on the Add Row
-                // button.
-                model->DeleteRows( r, 1 );
+                wait.reset();
+                errdlg.ShowModal();
+                return false;
             }
             else if( ( illegalCh = LIB_ID::FindIllegalLibraryNameChar( nick ) ) )
             {
@@ -478,7 +481,7 @@ bool PANEL_SYM_LIB_TABLE::verifyTables()
                     m_notebook->SetSelection( model == global_model() ? 0 : 1 );
 
                 m_cur_grid->MakeCellVisible( r, 0 );
-                m_cur_grid->SetGridCursor( r, 1 );
+                m_cur_grid->SetGridCursor( r, COL_NICKNAME );
 
                 wxWindow* topLevelParent = wxGetTopLevelParent( this );
 
@@ -499,12 +502,10 @@ bool PANEL_SYM_LIB_TABLE::verifyTables()
                 }
                 else
                 {
-                    wxString ltype  = model->GetValue( r, COL_TYPE );
+                    wxString ltype = model->GetValue( r, COL_TYPE );
                     model->LIB_TABLE_GRID::SetValue( r, COL_URI, uri );
                     model->SetValue( r, COL_TYPE, ltype );
                 }
-
-                ++r;        // this row was OK.
             }
         }
     }
@@ -517,11 +518,11 @@ bool PANEL_SYM_LIB_TABLE::verifyTables()
 
         for( int r1 = 0; r1 < model->GetNumberRows() - 1; ++r1 )
         {
-            wxString    nick1 = model->GetValue( r1, COL_NICKNAME );
+            wxString nick1 = model->GetValue( r1, COL_NICKNAME );
 
             for( int r2 = r1 + 1; r2 < model->GetNumberRows(); ++r2 )
             {
-                wxString    nick2 = model->GetValue( r2, COL_NICKNAME );
+                wxString nick2 = model->GetValue( r2, COL_NICKNAME );
 
                 if( nick1 == nick2 )
                 {
@@ -534,7 +535,7 @@ bool PANEL_SYM_LIB_TABLE::verifyTables()
 
                     // go to the lower of the two rows, it is technically the duplicate:
                     m_cur_grid->MakeCellVisible( r2, 0 );
-                    m_cur_grid->SetGridCursor( r2, 1 );
+                    m_cur_grid->SetGridCursor( r2, COL_NICKNAME );
 
                     wxWindow* topLevelParent = wxGetTopLevelParent( this );
 

eeschema/eeschema.cpp

@@ -204,7 +204,18 @@ static struct IFACE : public KIFACE_BASE, public UNITS_PROVIDER
             return new SYMBOL_VIEWER_FRAME( aKiway, aParent );
 
         case FRAME_SYMBOL_CHOOSER:
-            return new SYMBOL_CHOOSER_FRAME( aKiway, aParent );
+        {
+            bool cancelled = false;
+            SYMBOL_CHOOSER_FRAME* chooser = new SYMBOL_CHOOSER_FRAME( aKiway, aParent, cancelled );
+
+            if( cancelled )
+            {
+                chooser->Destroy();
+                return nullptr;
+            }
+
+            return chooser;
+        }
 
         case DIALOG_SCH_LIBRARY_TABLE:
             InvokeSchEditSymbolLibTable( aKiway, aParent );

eeschema/picksymbol.cpp

@@ -60,10 +60,12 @@ PICKED_SYMBOL SCH_BASE_FRAME::PickSymbolFromLibrary( const SYMBOL_LIBRARY_FILTER
     if( !dialogLock.try_lock() )
         return PICKED_SYMBOL();
 
+    bool aCancelled = false;
+
     DIALOG_SYMBOL_CHOOSER dlg( this, aHighlight, aFilter, aHistoryList, aAlreadyPlaced,
-                               aAllowFields, aShowFootprints );
+                               aAllowFields, aShowFootprints, aCancelled );
 
-    if( dlg.ShowModal() == wxID_CANCEL )
+    if( aCancelled || dlg.ShowModal() == wxID_CANCEL )
         return PICKED_SYMBOL();
 
     PICKED_SYMBOL sel;

eeschema/symbol_chooser_frame.cpp

@@ -65,7 +65,7 @@ END_EVENT_TABLE()
 #define MODAL_FRAME ( wxRESIZE_BORDER | wxSYSTEM_MENU | wxCAPTION | wxCLOSE_BOX | wxCLIP_CHILDREN \
                       | wxWANTS_CHARS | wxFRAME_NO_TASKBAR | wxSTAY_ON_TOP )
 
-SYMBOL_CHOOSER_FRAME::SYMBOL_CHOOSER_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
+SYMBOL_CHOOSER_FRAME::SYMBOL_CHOOSER_FRAME( KIWAY* aKiway, wxWindow* aParent, bool& aCancelled ) :
         SCH_BASE_FRAME( aKiway, aParent, FRAME_SYMBOL_CHOOSER, _( "Symbol Chooser" ),
                         wxDefaultPosition, wxDefaultSize, MODAL_FRAME, SYMBOL_CHOOSER_FRAME_NAME )
 {
@@ -78,12 +78,14 @@ SYMBOL_CHOOSER_FRAME::SYMBOL_CHOOSER_FRAME( KIWAY* aKiway, wxWindow* aParent ) :
     std::vector<PICKED_SYMBOL> dummyAlreadyPlaced;
     m_chooserPanel = new PANEL_SYMBOL_CHOOSER( this, this, nullptr /* no filter */,
                                                s_SymbolHistoryList,
-                                               dummyAlreadyPlaced, false, false,
+                                               dummyAlreadyPlaced, false, false, aCancelled,
+                                               // Accept handler
                                                [this]()
                                                {
                                                    wxCommandEvent dummy;
                                                    OnOK( dummy );
                                                },
+                                               // Escape handler
                                                [this]()
                                                {
                                                    DismissModal( false );

eeschema/symbol_chooser_frame.h

@@ -42,7 +42,7 @@ public:
      * @param aFrameType must be either #FRAME_SCH_LIB_VIEWER or #FRAME_SCH_LIB_VIEWER_MODAL.
      * @param aLibrary is the library to open when starting (default = NULL).
      */
-    SYMBOL_CHOOSER_FRAME( KIWAY* aKiway, wxWindow* aParent );
+    SYMBOL_CHOOSER_FRAME( KIWAY* aKiway, wxWindow* aParent, bool& aCancelled );
 
     ~SYMBOL_CHOOSER_FRAME() {};
 

eeschema/widgets/panel_symbol_chooser.cpp

@@ -56,7 +56,7 @@ PANEL_SYMBOL_CHOOSER::PANEL_SYMBOL_CHOOSER( SCH_BASE_FRAME* aFrame, wxWindow* aP
                                             const SYMBOL_LIBRARY_FILTER* aFilter,
                                             std::vector<PICKED_SYMBOL>&  aHistoryList,
                                             std::vector<PICKED_SYMBOL>&  aAlreadyPlaced,
-                                            bool aAllowFieldEdits, bool aShowFootprints,
+                                            bool aAllowFieldEdits, bool aShowFootprints, bool& aCancelled,
                                             std::function<void()> aAcceptHandler,
                                             std::function<void()> aEscapeHandler ) :
         wxPanel( aParent, wxID_ANY, wxDefaultPosition, wxDefaultSize ),
@@ -188,7 +188,7 @@ PANEL_SYMBOL_CHOOSER::PANEL_SYMBOL_CHOOSER( SCH_BASE_FRAME* aFrame, wxWindow* aP
         if( !adapter->AddLibraries( libNicknames, m_frame ) )
         {
             // loading cancelled by user
-            m_acceptHandler();
+            aCancelled = true;
         }
     }
 

eeschema/widgets/panel_symbol_chooser.h

@@ -50,10 +50,12 @@ public:
      *
      * @param aFrame  the parent frame (usually a SCH_EDIT_FRAME or SYMBOL_CHOOSER_FRAME)
      * @param aParent the parent window (usually a DIALOG_SHIM or SYMBOL_CHOOSER_FRAME)
-     * @param aAllowFieldEdits  if false, all functions that allow the user to edit fields
-     *                          (currently just footprint selection) will not be available.
-     * @param aShowFootprints   if false, all footprint preview and selection features are
-     *                          disabled. This forces aAllowFieldEdits false too.
+     * @param aAllowFieldEdits  if false, all functions that allow the user to edit fields (currently just
+     *                          footprint selection) will not be available.
+     * @param aShowFootprints   if false, all footprint preview and selection features are disabled. This
+     *                          forces aAllowFieldEdits false too.
+     * @param aCancelled [out] value indicating the user has cancelled the loading symbols progress dialog
+     *                   before we even get to showing the symbol chooser dialog.
      * @param aAcceptHandler a handler to be called on double-click of a footprint
      * @param aEscapeHandler a handler to be called on <ESC>
      */
@@ -61,7 +63,7 @@ public:
                           const SYMBOL_LIBRARY_FILTER* aFilter,
                           std::vector<PICKED_SYMBOL>& aHistoryList,
                           std::vector<PICKED_SYMBOL>& aAlreadyPlaced,
-                          bool aAllowFieldEdits, bool aShowFootprints,
+                          bool aAllowFieldEdits, bool aShowFootprints, bool& aCancelled,
                           std::function<void()> aAcceptHandler,
                           std::function<void()> aEscapeHandler );