Modular-Kicad milestone B), minor portions:

*) KIWAY_PLAYER::IsModal() is now a retained state, controlled by SetModal()

*) Fully re-work the KIWAY_PLAYER::ShowModal() to use a nested event loop.

*) Add support to DIALOG_SHIM for a "quasi-modal" dialog presentation and mode.
   See top of dialog_shim.cpp about that for benefits and need.

*) You can now pick footprint from the schematic component field dialog, although
   if you do this before you open the BOARD, you will only get the global footprint
   libraries, not also the project specific ones.  Opening the BOARD first avoids this
   problem.

This is the first example of cross KIFACE invocation, it is also the first
instance of using a TOP_FRAME other than FRAME_PCB as the first thing. It works,
but it's missing support for opening the project specific table because
historically the FRAME_PCB did that. This is now starting to expose all the near
term needs for KIWAY_PLAYER <-> PROJECT interaction, independence and out of
sequence usage.

A fix for this will be coming in a few days.

However it mostly starts to show why the KIWAY is terribly useful and important.

common/basicframe.cpp

@@ -151,6 +151,17 @@ bool EDA_BASE_FRAME::ProcessEvent( wxEvent& aEvent )
 }
 
 
+bool EDA_BASE_FRAME::Enable( bool enable )
+{
+#if defined(DEBUG)
+    const char* type_id = typeid( *this ).name();
+    printf( "wxFrame %s: %s\n", type_id, enable ? "enabled" : "disabled" );
+#endif
+
+    return wxFrame::Enable( enable );
+}
+
+
 void EDA_BASE_FRAME::onAutoSaveTimer( wxTimerEvent& aEvent )
 {
     if( !doAutoSave() )

common/dialog_shim.cpp

@@ -25,17 +25,45 @@
 
 #include <dialog_shim.h>
 #include <kiway_player.h>
+#include <wx/evtloop.h>
+
+/*
+    Quasi-Modal Mode Explained:
+
+    The gtk calls in wxDialog::ShowModal() cause event routing problems if that
+    modal dialog then tries to use KIWAY_PLAYER::ShowModal().  The latter shows up
+    and mostly works but does not respond to the window decoration close button.
+    There is no way to get around this without reversing the gtk calls temporarily.
+
+    Quasi-Modal mode is our own almost modal mode which disables only the parent
+    of the DIALOG_SHIM, leaving other frames operable and while staying captured in the
+    nested event loop.  This avoids the gtk calls and leaves event routing pure
+    and sufficient to operate the KIWAY_PLAYER::ShowModal() properly.  When using
+    ShowQuasiModal() you have to use EndQuasiModal() in your dialogs and not
+    EndModal().  There is also IsQuasiModal() but its value can only be true
+    when the nested event loop is active.  Do not mix the modal and quasi-modal
+    functions.  Use one set or the other.
+
+    You might find this behavior preferable over a pure modal mode, and it was said
+    that only the Mac has this natively, but now other platforms have something
+    similar.  You CAN use it anywhere for any dialog.  But you MUST use it when
+    you want to use KIWAY_PLAYER::ShowModal() from a dialog event.
+*/
+
+
 
 DIALOG_SHIM::DIALOG_SHIM( wxWindow* aParent, wxWindowID id, const wxString& title,
         const wxPoint& pos, const wxSize& size, long style, const wxString& name ) :
     wxDialog( aParent, id, title, pos, size, style, name ),
-    KIWAY_HOLDER( 0 )
+    KIWAY_HOLDER( 0 ),
+    m_qmodal_loop( 0 ),
+    m_qmodal_showing( false )
 {
     // pray that aParent is either a KIWAY_PLAYER or DIALOG_SHIM derivation.
     KIWAY_HOLDER* h = dynamic_cast<KIWAY_HOLDER*>( aParent );
 
     wxASSERT_MSG( h,
-        wxT( "DIALOG_SHIM's parent not derived from KIWAY_PLAYER nor DIALOG_SHIM" ) );
+        wxT( "DIALOG_SHIM's parent is NULL or not derived from KIWAY_PLAYER nor DIALOG_SHIM" ) );
 
     if( h )
         SetKiway( this, &h->Kiway() );
@@ -46,6 +74,14 @@ DIALOG_SHIM::DIALOG_SHIM( wxWindow* aParent, wxWindowID id, const wxString& titl
 }
 
 
+DIALOG_SHIM::~DIALOG_SHIM()
+{
+    // if the dialog is quasi-modal, this will end its event loop
+    if( IsQuasiModal() )
+        EndQuasiModal( wxID_CANCEL );
+}
+
+
 // our hashtable is an implementation secret, don't need or want it in a header file
 #include <hashtables.h>
 #include <base_struct.h>        // EDA_RECT
@@ -92,6 +128,90 @@ bool DIALOG_SHIM::Show( bool show )
 }
 
 
+bool DIALOG_SHIM::Enable( bool enable )
+{
+#if defined(DEBUG)
+    const char* type_id = typeid( *this ).name();
+    printf( "wxDialog %s: %s\n", type_id, enable ? "enabled" : "disabled" );
+#endif
+
+    return wxDialog::Enable( enable );
+}
+
+
+int DIALOG_SHIM::ShowQuasiModal()
+{
+    // toggle a window's "enable" status to disabled, then enabled on exit.
+    // exception safe.
+    struct ENABLE_DISABLE
+    {
+        wxWindow* m_win;
+        ENABLE_DISABLE( wxWindow* aWindow ) : m_win( aWindow ) { if( m_win ) m_win->Disable(); }
+        ~ENABLE_DISABLE() { if( m_win ) m_win->Enable(); }
+    };
+
+    // This is an exception safe way to zero a pointer before returning.
+    // Yes, even though DismissModal() clears this first normally, this is
+    // here in case there's an exception before the dialog is dismissed.
+    struct NULLER
+    {
+        void*&  m_what;
+        NULLER( void*& aPtr ) : m_what( aPtr ) {}
+        ~NULLER() { m_what = 0; }   // indeed, set it to NULL on destruction
+    } clear_this( (void*&) m_qmodal_loop );
+
+
+    // release the mouse if it's currently captured as the window having it
+    // will be disabled when this dialog is shown -- but will still keep the
+    // capture making it impossible to do anything in the modal dialog itself
+    wxWindow* win = wxWindow::GetCapture();
+    if( win )
+        win->ReleaseMouse();
+
+    wxWindow* parent = GetParentForModalDialog();
+
+    ENABLE_DISABLE  toggle( parent );
+
+    Show( true );
+
+    m_qmodal_showing = true;
+
+    wxGUIEventLoop          event_loop;
+    wxEventLoopActivator    event_loop_stacker( &event_loop );
+
+    m_qmodal_loop = &event_loop;
+
+    event_loop.Run();
+
+    if( toggle.m_win )      // let's focus back on the parent window
+        toggle.m_win->SetFocus();
+
+    return GetReturnCode();
+}
+
+
+void DIALOG_SHIM::EndQuasiModal( int retCode )
+{
+    SetReturnCode( retCode );
+
+    if( !IsQuasiModal() )
+    {
+        wxFAIL_MSG( "either DIALOG_SHIM::EndQuasiModal called twice or ShowQuasiModal wasn't called" );
+        return;
+    }
+
+    m_qmodal_showing = false;
+
+    if( m_qmodal_loop )
+    {
+        m_qmodal_loop->Exit();
+        m_qmodal_loop = NULL;
+    }
+
+    Show( false );
+}
+
+
 #if DLGSHIM_USE_SETFOCUS
 
 static bool findWindowRecursively( const wxWindowList& children, const wxWindow* wanted )
@@ -113,7 +233,7 @@ static bool findWindowRecursively( const wxWindowList& children, const wxWindow*
 }
 
 
-static bool findWindowRecursively( const wxWindow* topmost, const wxWindow* wanted )
+static bool findWindowReursively( const wxWindow* topmost, const wxWindow* wanted )
 {
     // wanted may be NULL and that is ok.
 

common/kiway_player.cpp

@@ -23,13 +23,14 @@
  */
 
 
-
 #include <kiway_player.h>
 #include <kiway_express.h>
 #include <kiway.h>
 #include <id.h>
+#include <macros.h>
 #include <typeinfo>
 #include <wx/utils.h>
+#include <wx/evtloop.h>
 
 
 BEGIN_EVENT_TABLE( KIWAY_PLAYER, EDA_BASE_FRAME )
@@ -43,10 +44,10 @@ KIWAY_PLAYER::KIWAY_PLAYER( KIWAY* aKiway, wxWindow* aParent, FRAME_T aFrameType
         long aStyle, const wxString& aWdoName ) :
     EDA_BASE_FRAME( aParent, aFrameType, aTitle, aPos, aSize, aStyle, aWdoName ),
     KIWAY_HOLDER( aKiway ),
-    m_modal_dismissed( 0 )
+    m_modal( false ),
+    m_modal_loop( 0 )
 {
-    DBG( printf("KIWAY_EXPRESS::wxEVENT_ID:%d\n", KIWAY_EXPRESS::wxEVENT_ID );)
-    //Connect( KIWAY_EXPRESS::wxEVENT_ID, wxKiwayExressHandler( KIWAY_PLAYER::kiway_express ) );
+    // DBG( printf("KIWAY_EXPRESS::wxEVENT_ID:%d\n", KIWAY_EXPRESS::wxEVENT_ID );)
 }
 
 
@@ -55,10 +56,10 @@ KIWAY_PLAYER::KIWAY_PLAYER( wxWindow* aParent, wxWindowID aId, const wxString& a
         const wxString& aWdoName ) :
     EDA_BASE_FRAME( aParent, (FRAME_T) aId, aTitle, aPos, aSize, aStyle, aWdoName ),
     KIWAY_HOLDER( 0 ),
-    m_modal_dismissed( 0 )
+    m_modal( false ),
+    m_modal_loop( 0 )
 {
-    DBG( printf("KIWAY_EXPRESS::wxEVENT_ID:%d\n", KIWAY_EXPRESS::wxEVENT_ID );)
-    //Connect( KIWAY_EXPRESS::wxEVENT_ID, wxKiwayExressHandler( KIWAY_PLAYER::kiway_express ) );
+    // DBG( printf("KIWAY_EXPRESS::wxEVENT_ID:%d\n", KIWAY_EXPRESS::wxEVENT_ID );)
 }
 
 
@@ -73,49 +74,54 @@ void KIWAY_PLAYER::KiwayMailIn( KIWAY_EXPRESS& aEvent )
 
 bool KIWAY_PLAYER::ShowModal( wxString* aResult )
 {
+    wxASSERT_MSG( IsModal(), "ShowModal() shouldn't be called on non-modal frame" );
+
     /*
-        This function has a nice interface but an unsightly implementation.
-        Now it is encapsulated, making it easier to improve.  I am not sure
-        a wxSemaphore was needed, especially since no blocking happens.  It seems
-        like a volatile bool is sufficient.
+        This function has a nice interface but a necessarily unsightly implementation.
+        Now the implementation is encapsulated, localizing future changes.
 
         It works in tandem with DismissModal().  But only ShowModal() is in the
         vtable and therefore cross-module capable.
    */
 
-    volatile bool   dismissed = false;
+    // This is an exception safe way to zero a pointer before returning.
+    // Yes, even though DismissModal() clears this first normally, this is
+    // here in case there's an exception before the dialog is dismissed.
+    struct NULLER
+    {
+        void*&  m_what;
+        NULLER( void*& aPtr ) : m_what( aPtr ) {}
+        ~NULLER() { m_what = 0; }   // indeed, set it to NULL on destruction
+    } clear_this( (void*&) m_modal_loop );
 
-    // disable all frames except the modal one, re-enable on exit, exception safe.
+    // exception safe way to disable all frames except the modal one,
+    // re-enable on exit
     wxWindowDisabler    toggle( this );
 
-    m_modal_dismissed = &dismissed;
-
     Show( true );
-    Raise();
 
-    // Wait for the one and only active frame to call DismissModal() from
-    // some concluding event.
-    while( !dismissed )
-    {
-        wxYield();
-        wxMilliSleep( 50 );
-    }
+    wxGUIEventLoop          event_loop;
+    wxEventLoopActivator    event_loop_stacker( &event_loop );
 
-    // no longer modal, not to mention that the pointer would be invalid outside this scope.
-    m_modal_dismissed = NULL;
+    m_modal_loop = &event_loop;
+
+    event_loop.Run();
 
     if( aResult )
         *aResult = m_modal_string;
 
+    DBG(printf( "~%s: aResult:'%s'  ret:%d\n",
+            __func__, TO_UTF8( m_modal_string ), m_modal_ret_val );)
+
     return m_modal_ret_val;
 }
 
 
 bool KIWAY_PLAYER::IsDismissed()
 {
-    // if already dismissed, then m_modal_dismissed may be NULL, and if not,
-    // it can still be dismissed if the bool is true.
-    bool ret = !m_modal_dismissed || *m_modal_dismissed;
+    bool ret = !m_modal_loop;
+
+    DBG(printf( "%s: ret:%d\n", __func__, ret );)
 
     return ret;
 }
@@ -126,8 +132,13 @@ void KIWAY_PLAYER::DismissModal( bool aRetVal, const wxString& aResult )
     m_modal_ret_val = aRetVal;
     m_modal_string  = aResult;
 
-    if( m_modal_dismissed )
-        *m_modal_dismissed = true;
+    if( m_modal_loop )
+    {
+        m_modal_loop->Exit();
+        m_modal_loop = 0;      // this marks it as dismissed.
+    }
+
+    Show( false );
 }
 
 

eeschema/dialogs/dialog_edit_component_in_schematic.cpp

@@ -148,10 +148,10 @@ void SCH_EDIT_FRAME::EditComponent( SCH_COMPONENT* aComponent )
     // make sure the chipnameTextCtrl is wide enough to hold any unusually long chip names:
     EnsureTextCtrlWidth( dlg->chipnameTextCtrl );
 
-    dlg->ShowModal();
+    dlg->ShowQuasiModal();
 
-    m_canvas->MoveCursorToCrossHair();
     m_canvas->SetIgnoreMouseEvents( false );
+    m_canvas->MoveCursorToCrossHair();
     dlg->Destroy();
 }
 
@@ -214,7 +214,7 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnListItemSelected( wxListEvent& event
 
 void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnCancelButtonClick( wxCommandEvent& event )
 {
-    EndModal( 1 );
+    EndQuasiModal( 1 );
 }
 
 
@@ -379,7 +379,7 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnOKButtonClick( wxCommandEvent& event
     m_Parent->GetScreen()->TestDanglingEnds();
     m_Parent->GetCanvas()->Refresh( true );
 
-    EndModal( 0 );
+    EndQuasiModal( 0 );
 }
 
 
@@ -436,23 +436,21 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::deleteFieldButtonHandler( wxCommandEven
 
 void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::showButtonHandler( wxCommandEvent& event )
 {
-#if 1
+#if 0
    wxString datasheet_uri = fieldValueTextCtrl->GetValue();
    ::wxLaunchDefaultBrowser( datasheet_uri );
 
 #else
-    unsigned fieldNdx = getSelectedFieldNdx();
 
-/*
+    unsigned fieldNdx = getSelectedFieldNdx();
     if( fieldNdx == DATASHEET )
     {
         wxString datasheet_uri = fieldValueTextCtrl->GetValue();
         ::wxLaunchDefaultBrowser( datasheet_uri );
     }
     else if( fieldNdx == FOOTPRINT )
-*/
     {
-        // pick a footprint
+        // pick a footprint using the footprint picker.
         wxString fpid;
 
         KIWAY_PLAYER* frame = Kiway().Player( FRAME_PCB_MODULE_VIEWER_MODAL, true );
@@ -460,11 +458,11 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::showButtonHandler( wxCommandEvent& even
         if( frame->ShowModal( &fpid ) )
         {
             printf( "%s: %s\n", __func__, TO_UTF8( fpid ) );
-        }
+            fieldValueTextCtrl->SetValue( fpid );
 
-        frame->Show( false );       // keep the frame open, but hidden.
+        }
 
-        Raise();
+        frame->Destroy();
     }
 #endif
 
@@ -764,7 +762,16 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::copySelectedFieldToPanel()
 
     fieldValueTextCtrl->SetValue( field.GetText() );
 
-    m_show_datasheet_button->Enable( fieldNdx == DATASHEET );
+    m_show_datasheet_button->Enable( fieldNdx == DATASHEET || fieldNdx == FOOTPRINT );
+
+    if( fieldNdx == DATASHEET )
+        m_show_datasheet_button->SetLabel( _( "Show in Browser" ) );
+    else if( fieldNdx == FOOTPRINT )
+        m_show_datasheet_button->SetLabel( _( "Assign Footprint" ) );
+    else
+        m_show_datasheet_button->SetLabel( wxEmptyString );
+
+    m_show_datasheet_button->Enable( fieldNdx == DATASHEET || fieldNdx == FOOTPRINT );
 
     // For power symbols, the value is NOR editable, because value and pin
     // name must be same and can be edited only in library editor
@@ -1006,5 +1013,5 @@ void DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::SetInitCmp( wxCommandEvent& event )
     m_Parent->OnModify();
 
     m_Cmp->Draw( m_Parent->GetCanvas(), &dc, wxPoint( 0, 0 ), GR_DEFAULT_DRAWMODE );
-    EndModal( 1 );
+    EndQuasiModal( 1 );
 }

eeschema/getpart.cpp

@@ -56,7 +56,7 @@
 wxString SCH_BASE_FRAME::SelectComponentFromLibBrowser( LIB_ALIAS* aPreselectedAlias,
                                                         int* aUnit, int* aConvert )
 {
-    // Close the any current non-modal Lib browser if open, and open a new one, in "modal" mode:
+    // Close any open non-modal Lib browser, and open a new one, in "modal" mode:
     LIB_VIEW_FRAME* viewlibFrame = (LIB_VIEW_FRAME*) Kiway().Player( FRAME_SCH_VIEWER, false );
     if( viewlibFrame )
         viewlibFrame->Destroy();
@@ -79,13 +79,14 @@ wxString SCH_BASE_FRAME::SelectComponentFromLibBrowser( LIB_ALIAS* aPreselectedA
 
     wxString cmpname;
 
-    viewlibFrame->ShowModal( &cmpname );
-
-    if( aUnit )
-        *aUnit = viewlibFrame->GetUnit();
+    if( viewlibFrame->ShowModal( &cmpname ) )
+    {
+        if( aUnit )
+            *aUnit = viewlibFrame->GetUnit();
 
-    if( aConvert )
-        *aConvert = viewlibFrame->GetConvert();
+        if( aConvert )
+            *aConvert = viewlibFrame->GetConvert();
+    }
 
     viewlibFrame->Destroy();
 

eeschema/tool_viewlib.cpp

@@ -116,8 +116,7 @@ void LIB_VIEW_FRAME::ReCreateHToolbar()
                                 _( "View component documents" ) );
         m_mainToolBar->EnableTool( ID_LIBVIEW_VIEWDOC, false );
 
-        // if library browser is modal
-        if( m_Ident == FRAME_SCH_VIEWER_MODAL )
+        if( IsModal() )
         {
             m_mainToolBar->AddSeparator();
             m_mainToolBar->AddTool( ID_LIBVIEW_CMP_EXPORT_TO_SCHEMATIC,
@@ -130,11 +129,11 @@ void LIB_VIEW_FRAME::ReCreateHToolbar()
         m_mainToolBar->Realize();
     }
 
-    if( (m_libraryName != wxEmptyString) && (m_entryName != wxEmptyString) )
+    if( m_libraryName.size() && m_entryName.size() )
     {
         lib = CMP_LIBRARY::FindLibrary( m_libraryName );
 
-        if( lib != NULL )
+        if( lib )
         {
             component = lib->FindComponent( m_entryName );
 
@@ -161,7 +160,6 @@ void LIB_VIEW_FRAME::ReCreateHToolbar()
         m_mainToolBar->ToggleTool( ID_LIBVIEW_DE_MORGAN_CONVERT_BUTT, false );
      }
 
-
     int parts_count = 1;
 
     if( component )
@@ -171,8 +169,7 @@ void LIB_VIEW_FRAME::ReCreateHToolbar()
 
     for( ii = 0; ii < parts_count; ii++ )
     {
-        wxString msg;
-        msg.Printf( _( "Unit %c" ), 'A' + ii );
+        wxString msg = wxString::Format( _( "Unit %c" ), 'A' + ii );
         m_selpartBox->Append( msg );
     }
 

eeschema/viewlib_frame.cpp

@@ -103,6 +103,9 @@ LIB_VIEW_FRAME::LIB_VIEW_FRAME( KIWAY* aKiway, wxWindow* aParent, FRAME_T aFrame
 {
     wxASSERT( aFrameType==FRAME_SCH_VIEWER || aFrameType==FRAME_SCH_VIEWER_MODAL );
 
+    if( aFrameType == FRAME_SCH_VIEWER_MODAL )
+        SetModal( true );
+
     wxAcceleratorTable table( ACCEL_TABLE_CNT, accels );
 
     m_FrameName = GetLibViewerFrameName();

include/dialog_shim.h

@@ -56,12 +56,26 @@ public:
             const   wxString& name = wxDialogNameStr
             );
 
-    bool Show( bool show );     // overload wxDialog::Show
+    ~DIALOG_SHIM();
 
+    int ShowQuasiModal();      // disable only the parent window, otherwise modal.
+
+    void EndQuasiModal( int retCode );  // End quasi-modal mode
+
+    bool IsQuasiModal()         { return m_qmodal_showing; }
+
+    bool Show( bool show );     // override wxDialog::Show
+
+    bool Enable( bool enable ); // override wxDialog::Enable virtual
 
 protected:
     std::string m_hash_key;     // alternate for class_map when classname re-used.
 
+    // variables for quasi-modal behavior support, only used by a few derivatives.
+    wxGUIEventLoop* m_qmodal_loop;      // points to nested event_loop, NULL means not qmodal and dismissed
+    bool            m_qmodal_showing;
+
+
 #if DLGSHIM_USE_SETFOCUS
 private:
     void    onInit( wxInitDialogEvent& aEvent );

include/kiway_player.h

@@ -87,6 +87,7 @@ private:
 
 
 class KIWAY_EXPRESS;
+class wxGUIEventLoop;
 
 /**
  * Class KIWAY_PLAYER
@@ -194,7 +195,8 @@ public:
 
 protected:
 
-    bool IsModal()              { return m_modal_dismissed; }
+    bool IsModal()                      { return m_modal; }
+    void SetModal( bool IsModal )       { m_modal = IsModal; }
 
     /**
      * Function IsDismissed
@@ -216,7 +218,8 @@ protected:
     void language_change( wxCommandEvent& event );
 
     // variables for modal behavior support, only used by a few derivatives.
-    volatile bool*  m_modal_dismissed;  // points to "dismissed state", NULL means not modal
+    bool            m_modal;        // true if frame is intended to be modal, not modeless
+    wxGUIEventLoop* m_modal_loop;   // points to nested event_loop, NULL means not modal and dismissed
     wxString        m_modal_string;
     bool            m_modal_ret_val;    // true if a selection was made
 

include/wxstruct.h

@@ -190,7 +190,9 @@ public:
      * @warning If you override this function in a derived class, make sure you call
      *          down to this or the auto save feature will be disabled.
      */
-    bool ProcessEvent( wxEvent& aEvent );       // overload wxFrame::ProcessEvent()
+    bool ProcessEvent( wxEvent& aEvent );       // override wxFrame::ProcessEvent()
+
+    bool Enable( bool enable );                 // override wxFrame::Enable virtual
 
     void SetAutoSaveInterval( int aInterval ) { m_autoSaveInterval = aInterval; }
 

pcbnew/footprint_wizard_frame.cpp

@@ -125,6 +125,9 @@ FOOTPRINT_WIZARD_FRAME::FOOTPRINT_WIZARD_FRAME( KIWAY* aKiway,
 {
     wxASSERT( aFrameType==FRAME_PCB_FOOTPRINT_WIZARD_MODAL );
 
+    if( aFrameType == FRAME_PCB_FOOTPRINT_WIZARD_MODAL )
+        SetModal( true );
+
     wxAcceleratorTable table( ACCEL_TABLE_CNT, accels );
 
     m_FrameName     = FOOTPRINT_WIZARD_FRAME_NAME;
@@ -641,7 +644,7 @@ void FOOTPRINT_WIZARD_FRAME::ReCreateHToolbar()
         m_mainToolBar->AddTool( ID_ZOOM_PAGE, wxEmptyString,
                                 KiBitmap( zoom_fit_in_page_xpm ), msg );
 
-        if( m_Ident == FRAME_PCB_FOOTPRINT_WIZARD_MODAL )
+        if( IsModal() )
         {
             // The library browser is called from a "load component" command
             m_mainToolBar->AddSeparator();

pcbnew/modview_frame.cpp

@@ -129,6 +129,9 @@ FOOTPRINT_VIEWER_FRAME::FOOTPRINT_VIEWER_FRAME( KIWAY* aKiway, wxWindow* aParent
 {
     wxASSERT( aFrameType==FRAME_PCB_MODULE_VIEWER || aFrameType==FRAME_PCB_MODULE_VIEWER_MODAL );
 
+    if( aFrameType == FRAME_PCB_MODULE_VIEWER_MODAL )
+        SetModal( true );
+
     wxAcceleratorTable table( DIM( accels ), accels );
 
     m_FrameName  = GetFootprintViewerFrameName();
@@ -271,6 +274,7 @@ const wxChar* FOOTPRINT_VIEWER_FRAME::GetFootprintViewerFrameName()
 
 void FOOTPRINT_VIEWER_FRAME::OnCloseWindow( wxCloseEvent& Event )
 {
+    DBG(printf( "%s:\n", __func__ );)
     if( IsModal() )
     {
         // Only dismiss a modal frame once, so that the return values set by
@@ -278,7 +282,7 @@ void FOOTPRINT_VIEWER_FRAME::OnCloseWindow( wxCloseEvent& Event )
         if( !IsDismissed() )
             DismissModal( false );
 
-        // window will be destroyed by the calling function.
+        // window to be destroyed by the caller of KIWAY_PLAYER::ShowModal()
     }
     else
         Destroy();

pcbnew/tool_modview.cpp

@@ -48,8 +48,7 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateHToolbar()
     {
         m_mainToolBar = new wxAuiToolBar( this, wxID_ANY, wxDefaultPosition, wxDefaultSize,
                                           wxAUI_TB_DEFAULT_STYLE
-                                            | wxAUI_TB_OVERFLOW
-        );
+                                            | wxAUI_TB_OVERFLOW );
 
         // Set up toolbar
         m_mainToolBar->AddTool( ID_MODVIEW_SELECT_LIB, wxEmptyString,
@@ -95,8 +94,7 @@ void FOOTPRINT_VIEWER_FRAME::ReCreateHToolbar()
         m_mainToolBar->AddTool( ID_ZOOM_PAGE, wxEmptyString,
                                 KiBitmap( zoom_fit_in_page_xpm ), msg );
 
-        // Enable this tool only if the library browser is intended for modal use.
-        if( m_Ident == FRAME_PCB_MODULE_VIEWER_MODAL )
+        if( IsModal() )
         {
             m_mainToolBar->AddSeparator();
             m_mainToolBar->AddTool( ID_MODVIEW_FOOTPRINT_EXPORT_TO_BOARD, wxEmptyString,