Symbol Library Editor: fix crash after renaming a symbol

Crash was caused by removal of the selected item from the
wxDataViewModel, which was later accessed in
COMPONENT_TREE::GetSelectedLibId(). To avoid the problem, the selection
is validated before regenerating the tree widget.

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

eeschema/libedit.cpp

@@ -378,10 +378,6 @@ void LIB_EDIT_FRAME::OnRemovePart( wxCommandEvent& aEvent )
     if( isCurrentPart( libId ) )
         emptyScreen();
 
-    // Keeping a removed item selected may lead to a crash
-    if( m_treePane->GetCmpTree()->GetSelectedLibId( nullptr ) == libId )
-        m_treePane->GetCmpTree()->SelectLibId( LIB_ID( libId.GetLibNickname(), "" ) );
-
     m_libMgr->RemovePart( libId.GetLibItemName(), libId.GetLibNickname() );
 }
 
@@ -389,7 +385,8 @@ void LIB_EDIT_FRAME::OnRemovePart( wxCommandEvent& aEvent )
 void LIB_EDIT_FRAME::OnCopyCutPart( wxCommandEvent& aEvent )
 {
     int unit = 0;
-    LIB_ID partId = m_treePane->GetCmpTree()->GetSelectedLibId( &unit );
+    auto cmpTree = m_treePane->GetCmpTree();
+    LIB_ID partId = cmpTree->GetSelectedLibId( &unit );
     LIB_PART* part = m_libMgr->GetBufferedPart( partId.GetLibItemName(), partId.GetLibNickname() );
 
     if( !part )
@@ -403,10 +400,6 @@ void LIB_EDIT_FRAME::OnCopyCutPart( wxCommandEvent& aEvent )
         if( isCurrentPart( libId ) )
             emptyScreen();
 
-        // Keeping a removed item selected may lead to a crash
-        if( m_treePane->GetCmpTree()->GetSelectedLibId( nullptr ) == libId )
-            m_treePane->GetCmpTree()->SelectLibId( LIB_ID( libId.GetLibNickname(), "" ) );
-
         m_libMgr->RemovePart( libId.GetLibItemName(), libId.GetLibNickname() );
     }
 }

eeschema/libeditframe.cpp

@@ -1635,6 +1635,11 @@ wxString LIB_EDIT_FRAME::getTargetLib() const
 
 void LIB_EDIT_FRAME::SyncLibraries( bool aProgress )
 {
+    LIB_ID selected;
+
+    if( m_treePane )
+        selected = m_treePane->GetCmpTree()->GetSelectedLibId();
+
     if( aProgress )
     {
         wxProgressDialog progressDlg( _( "Loading Symbol Libraries" ),
@@ -1650,7 +1655,31 @@ void LIB_EDIT_FRAME::SyncLibraries( bool aProgress )
     }
 
     if( m_treePane )
+    {
+        wxDataViewItem found;
+
+        if( selected.IsValid() )
+        {
+            // Check if the previously selected item is still valid,
+            // if not - it has to be unselected to prevent crash
+            found = m_libMgr->GetAdapter()->FindItem( selected );
+
+            if( !found )
+                m_treePane->GetCmpTree()->Unselect();
+        }
+
         m_treePane->Regenerate();
+
+        // Try to select the parent library, in case the part is not found
+        if( !found && selected.IsValid() )
+        {
+            selected.SetLibItemName( "" );
+            found = m_libMgr->GetAdapter()->FindItem( selected );
+
+            if( found )
+                m_treePane->GetCmpTree()->SelectLibId( selected );
+        }
+    }
 }
 
 

eeschema/libfield.cpp

@@ -35,7 +35,10 @@
 #include <symbol_lib_table.h>
 #include <template_fieldnames.h>
 #include <dialog_edit_one_field.h>
+
 #include <lib_manager.h>
+#include <widgets/cmp_tree_pane.h>
+#include <component_tree.h>
 
 
 void LIB_EDIT_FRAME::EditField( LIB_FIELD* aField )
@@ -166,10 +169,11 @@ void LIB_EDIT_FRAME::EditField( LIB_FIELD* aField )
         if( !parent->HasAlias( m_aliasName ) )
             m_aliasName = newFieldValue;
 
-        if( newFieldValue != fieldText )
+        m_libMgr->RemovePart( fieldText, lib );
-            m_libMgr->RemovePart( fieldText, lib );
-
         m_libMgr->UpdatePart( parent, lib );
+
+        // Reselect the renamed part
+        m_treePane->GetCmpTree()->SelectLibId( LIB_ID( lib, newFieldValue ) );
     }
 
 

eeschema/widgets/component_tree.cpp

@@ -146,6 +146,12 @@ void COMPONENT_TREE::SelectLibId( const LIB_ID& aLibId )
 }
 
 
+void COMPONENT_TREE::Unselect()
+{
+    m_tree_ctrl->UnselectAll();
+}
+
+
 void COMPONENT_TREE::Regenerate()
 {
     STATE current;
@@ -232,7 +238,7 @@ void COMPONENT_TREE::setState( const STATE& aState )
     // command is issued, otherwise it selects a random item (Windows)
     m_tree_ctrl->Thaw();
 
-    if( aState.selection.IsValid() )
+    if( !aState.selection.GetLibItemName().empty() || !aState.selection.GetLibNickname().empty() )
         SelectLibId( aState.selection );
 }
 

eeschema/widgets/component_tree.h

@@ -77,6 +77,11 @@ public:
      */
     void SelectLibId( const LIB_ID& aLibId );
 
+    /**
+     * Unselect currently selected item in wxDataViewCtrl
+     */
+    void Unselect();
+
     /**
      * Associates a right click context menu for a specific node type.
      * @param aType is the node type to have a menu associated.