HI Wayne, Yes, here is a separate patch for the 5.0 series.
Cheers, John On Fri, Sep 28, 2018 at 3:11 PM Wayne Stambaugh <stambau...@gmail.com> wrote: > > Hey John, > > Nice work! I merged your patch into the development branch. I do have > one favor to ask if you have the time. Patch 6 does not apply to the > 5.0 branch which also suffers from the crash bug. If you could merge it > into the 5.0 branch and send me a separate patch I would appreciate it. > If you don't have the time please let me know because this needs to be > fixed for the 5.0.1 release. > > Cheers, > > Wayne > > On 09/28/2018 07:53 AM, John Beard wrote: > > Hi, > > > > Here is a patch set for adding a filter control to the hotkey editor > > dialog. Preview video: https://sendvid.com/je0cyg87 > > > > Most of the work in the first commit is separating out the hotkey data > > from the UI widget code. > > > > This also fixes a couple of other bugs (one crashing, and one able to > > get the HK configs into a mess with conflicts) in that dialog, and > > adds some mock objects to the common QA tests, which will make it > > easier to get more of libcommon under test. > > > > A 5.0 series fix for the crashing bug will follow. > > > > This is also useful as groundwork for > > https://bugs.launchpad.net/kicad/+bug/1778374 (a 5.1 milestone), but > > that will need a further decision to choose between: > > > > * Use a read-only variant of the hotkey editor TreeListView for the > > hotkey list dialog (needs more code: a new dialog and adding a > > read-only mode to the current widget), or > > * Remove the hotkey list dialog altogether and bind the "show hotkey > > list" command to simply opening the prefs dialog to the hotkey panel > > > > Cheers, > > > > John > > > > > > > > _______________________________________________ > > Mailing list: https://launchpad.net/~kicad-developers > > Post to : kicad-developers@lists.launchpad.net > > Unsubscribe : https://launchpad.net/~kicad-developers > > More help : https://help.launchpad.net/ListHelp > > > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp
From 7b4692552f6bc03dff62d036ad0543a76dd11309 Mon Sep 17 00:00:00 2001 From: John Beard <john.j.be...@gmail.com> Date: Thu, 27 Sep 2018 14:44:03 +0100 Subject: [PATCH] Prevent segfault when undoing or resetting non-hotkey rows This is caused by: * Not checking the hotkey data is not null when performing a hotkey action * Allowing hotkey actions on non-hotkey rows. This fixes both of these, and adds an assert to warn if someone does manage to fire a hotkey action on a non-hotkey row (but it won't crash). This is a ported version of commit 704615721 on the master branch. Fixes: lp:1794756 * https://bugs.launchpad.net/kicad/+bug/1794756 --- common/widgets/widget_hotkey_list.cpp | 47 ++++++++++++++++++++------- include/widgets/widget_hotkey_list.h | 7 ++++ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/common/widgets/widget_hotkey_list.cpp b/common/widgets/widget_hotkey_list.cpp index 66b38ad04..74c3f59b7 100644 --- a/common/widgets/widget_hotkey_list.cpp +++ b/common/widgets/widget_hotkey_list.cpp @@ -253,6 +253,18 @@ WIDGET_HOTKEY_CLIENT_DATA* WIDGET_HOTKEY_LIST::GetSelHKClientData() } +WIDGET_HOTKEY_CLIENT_DATA* WIDGET_HOTKEY_LIST::getExpectedHkClientData( wxTreeListItem aItem ) +{ + const auto hkdata = GetHKClientData( aItem ); + + // This probably means a hotkey-only action is being attempted on + // a row that is not a hotkey (like a section heading) + wxASSERT_MSG( hkdata != nullptr, "No hotkey data found for list item" ); + + return hkdata; +} + + void WIDGET_HOTKEY_LIST::UpdateFromClientData() { for( wxTreeListItem i = GetFirstItem(); i.IsOk(); i = GetNextItem( i ) ) @@ -285,13 +297,10 @@ void WIDGET_HOTKEY_LIST::LoadSection( EDA_HOTKEY_CONFIG* aSection ) void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem ) { - WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem ); + WIDGET_HOTKEY_CLIENT_DATA* hkdata = getExpectedHkClientData( aItem ); if( !hkdata ) - { - // Activated item was not a hotkey row return; - } wxString name = GetItemText( aItem, 0 ); wxString current_key = GetItemText( aItem, 1 ); @@ -299,7 +308,7 @@ void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem ) wxKeyEvent key_event = HK_PROMPT_DIALOG::PromptForKey( GetParent(), name, current_key ); long key = MapKeypressToKeycode( key_event ); - if( hkdata && key ) + if( key ) { // See if this key code is handled in hotkeys names list bool exists; @@ -327,7 +336,11 @@ void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem ) void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem ) { - WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem ); + WIDGET_HOTKEY_CLIENT_DATA* hkdata = getExpectedHkClientData( aItem ); + + if( !hkdata ) + return; + EDA_HOTKEY* hk = &hkdata->GetHotkey(); for( size_t sec_index = 0; sec_index < m_sections.size(); ++sec_index ) @@ -356,9 +369,14 @@ void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem ) void WIDGET_HOTKEY_LIST::ResetItemToDefault( wxTreeListItem aItem ) { - WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem ); + WIDGET_HOTKEY_CLIENT_DATA* hkdata = getExpectedHkClientData( aItem ); + + if( !hkdata ) + return; + EDA_HOTKEY* hk = &hkdata->GetHotkey(); hk->ResetKeyCodeToDefault(); + UpdateFromClientData(); } @@ -376,10 +394,17 @@ void WIDGET_HOTKEY_LIST::OnContextMenu( wxTreeListEvent& aEvent ) wxMenu menu; - menu.Append( ID_EDIT, _( "Edit..." ) ); - menu.Append( ID_RESET, _( "Undo Changes" ) ); - menu.Append( ID_DEFAULT, _( "Restore Default" ) ); - menu.Append( wxID_SEPARATOR ); + WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( m_context_menu_item ); + + // Some actions only apply if the row is hotkey data + if( hkdata ) + { + menu.Append( ID_EDIT, _( "Edit..." ) ); + menu.Append( ID_RESET, _( "Undo Changes" ) ); + menu.Append( ID_DEFAULT, _( "Restore Default" ) ); + menu.Append( wxID_SEPARATOR ); + } + menu.Append( ID_RESET_ALL, _( "Undo All Changes" ) ); menu.Append( ID_DEFAULT_ALL, _( "Restore All to Default" ) ); diff --git a/include/widgets/widget_hotkey_list.h b/include/widgets/widget_hotkey_list.h index dff73f03e..fe8eb087f 100644 --- a/include/widgets/widget_hotkey_list.h +++ b/include/widgets/widget_hotkey_list.h @@ -72,6 +72,13 @@ class WIDGET_HOTKEY_LIST : public TWO_COLUMN_TREE_LIST */ WIDGET_HOTKEY_CLIENT_DATA* GetSelHKClientData(); + /** + * Get the WIDGET_HOTKEY_CLIENT_DATA form an item and assert if it isn't + * found. This is for use when the data not being present indicates an + * error. + */ + WIDGET_HOTKEY_CLIENT_DATA* getExpectedHkClientData( wxTreeListItem aItem ); + /** * Method UpdateFromClientData * Refresh the visible text on the widget from the rows' client data objects. -- 2.19.0
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp