From 4f6e7ef22e0f2a47542d64c1314127938aecf055 Mon Sep 17 00:00:00 2001
From: Maciej Suminski <maciej.suminski@cern.ch>
Date: Tue, 9 Jan 2018 13:48:00 +0100
Subject: [PATCH] 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                | 11 ++---------
 eeschema/libeditframe.cpp           | 29 +++++++++++++++++++++++++++++
 eeschema/libfield.cpp               | 10 +++++++---
 eeschema/widgets/component_tree.cpp |  8 +++++++-
 eeschema/widgets/component_tree.h   |  5 +++++
 5 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/eeschema/libedit.cpp b/eeschema/libedit.cpp
index 75e44d92a..807c4117b 100644
--- a/eeschema/libedit.cpp
+++ b/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() );
     }
 }
diff --git a/eeschema/libeditframe.cpp b/eeschema/libeditframe.cpp
index c29cb6607..78c5f2d0f 100644
--- a/eeschema/libeditframe.cpp
+++ b/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 );
+        }
+    }
 }
 
 
diff --git a/eeschema/libfield.cpp b/eeschema/libfield.cpp
index 383957257..1a0f657cd 100644
--- a/eeschema/libfield.cpp
+++ b/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 ) );
     }
 
 
diff --git a/eeschema/widgets/component_tree.cpp b/eeschema/widgets/component_tree.cpp
index bc0cb14a2..6e0a1c86e 100644
--- a/eeschema/widgets/component_tree.cpp
+++ b/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 );
 }
 
diff --git a/eeschema/widgets/component_tree.h b/eeschema/widgets/component_tree.h
index 34b5d7380..15bf9c60f 100644
--- a/eeschema/widgets/component_tree.h
+++ b/eeschema/widgets/component_tree.h
@@ -78,6 +78,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.
      * @param aMenu is the associated menu.
-- 
2.13.3