From 6bbbf5faa981d26d79b23805b788fb28ae7c288f Mon Sep 17 00:00:00 2001 From: "v.audacity" Date: Sat, 26 Mar 2011 01:41:22 +0000 Subject: [PATCH] Bug 137 (P3) - Unreliable project re-opening with orphaned and missing blockfile errors At bottom of http://bugzilla.audacityteam.org/show_bug.cgi?id=137#c17: > I noticed that although when we open the same project a second time with File > > Open or File > Recent Files we show an error "is already open in another > window", there is no block on executing the .aup from your file manager and > opening as many copies of the project as you like. I had not done that when > Audacity moved the files around in error the first time, but can we block this > anyway? This is a different bug from Bug 137, repeatable, and I think lower priority. But I went ahead and fixed it with this commit. Also fixed previously unnoted bug where AudacityApp::MRUOpen() returned true when it actually failed to open the file. Also removed AudacityApp::OnMRUProject() cruft. Unfortunately, it still creates a new, empty project window on Win Explorer open whereas Open and Recent Files commands do not. I think that constitutes a new, separate P5 bug. Overall, this is another aspect of what I was talking about in http://bugzilla.audacityteam.org/show_bug.cgi?id=322#c26. Opening files vs projects got conflated for convenience, but this code is hacked in some regards, rather than being a good design, and that's why this type of bug shows up. --- src/AudacityApp.cpp | 58 +++++++++++++-------------------------------- src/Project.cpp | 55 +++++++++++++++++++++++++++--------------- src/Project.h | 1 + 3 files changed, 54 insertions(+), 60 deletions(-) diff --git a/src/AudacityApp.cpp b/src/AudacityApp.cpp index 10b976f66..6dca05fda 100644 --- a/src/AudacityApp.cpp +++ b/src/AudacityApp.cpp @@ -701,7 +701,7 @@ void AudacityApp::OnMacOpenFile(wxCommandEvent & event) while (ofqueue.GetCount()) { wxString name(ofqueue[0]); ofqueue.RemoveAt(0); - MRUOpen(name); + MRUOpen(name); // FIX-ME: Check the return result? } } #endif //__WXMAC__ @@ -735,7 +735,7 @@ BEGIN_EVENT_TABLE(AudacityApp, wxApp) EVT_APP_COMMAND(wxID_ANY, AudacityApp::OnReceiveCommand) END_EVENT_TABLE() -// Backend for OnMRUFile and OnMRUProject +// backend for OnMRUFile bool AudacityApp::MRUOpen(wxString fullPathStr) { // Most of the checks below are copied from AudacityProject::OpenFiles. // - some rationalisation might be possible. @@ -749,18 +749,12 @@ bool AudacityApp::MRUOpen(wxString fullPathStr) { { gPrefs->Write(wxT("/DefaultOpenPath"), wxPathOnly(fullPathStr)); - // Make sure it isn't already open - wxFileName newFileName(fullPathStr); - size_t numProjects = gAudacityProjects.Count(); - for (size_t i = 0; i < numProjects; i++) { - if (newFileName.SameAs(gAudacityProjects[i]->GetFileName())) { - wxMessageBox(wxString::Format(_("%s is already open in another window."), - newFileName.GetName().c_str()), - _("Error opening project"), - wxOK | wxCENTRE); - return(true); - } - } + // Make sure it isn't already open. + // Test here even though AudacityProject::OpenFile() also now checks, because + // that method does not return the bad result. + // That itself may be a FIX-ME. + if (AudacityProject::IsAlreadyOpen(fullPathStr)) + return false; // DMM: If the project is dirty, that means it's been touched at // all, and it's not safe to open a new project directly in its @@ -795,40 +789,22 @@ void AudacityApp::OnMRUClear(wxCommandEvent& event) mRecentFiles->Clear(); } +//vvv Basically, anything from Recent Files is treated as a .aup, until proven otherwise, +// then it tries to Import(). Very questionable handling, imo. +// Better, for example, to check the file type early on. void AudacityApp::OnMRUFile(wxCommandEvent& event) { int n = event.GetId() - ID_RECENT_FIRST; wxString fullPathStr = mRecentFiles->GetHistoryFile(n); - bool opened = MRUOpen(fullPathStr); - if(!opened) { + // Try to open only if not already open. + // Test IsAlreadyOpen() here even though AudacityProject::MRUOpen() also now checks, + // because we don't want to RemoveFileFromHistory() just because it already exists, + // and AudacityApp::OnMacOpenFile() calls MRUOpen() directly. + // that method does not return the bad result. + if (!AudacityProject::IsAlreadyOpen(fullPathStr) && !MRUOpen(fullPathStr)) mRecentFiles->RemoveFileFromHistory(n); - } } -#if 0 -//FIX-ME: Was this OnMRUProject lost in an edit?? Should we have it back? -//vvv I think it was removed on purpose, but I don't know why it's still here. -// Basically, anything from Recent Files is treated as a .aup, until proven otherwise, -// then it tries to Import(). Very questionable handling, imo. -// Better, for example, to check the file type early on. -// -void AudacityApp::OnMRUProject(wxCommandEvent& event) { - AudacityProject *proj = GetActiveProject(); - - int n = event.GetId() - 6050;//FIX-ME: Use correct ID name. - wxString fullPathStr = proj->GetRecentProjects()->GetHistoryFile(n); - - bool opened = MRUOpen(fullPathStr); - if(!opened) { - proj->GetRecentProjects()->RemoveFileFromHistory(n); - gPrefs->SetPath("/RecentProjects"); - proj->GetRecentProjects()->Save(*gPrefs); - gPrefs->SetPath(".."); - } -} -#endif - - void AudacityApp::InitLang( const wxString & lang ) { if( mLocale ) diff --git a/src/Project.cpp b/src/Project.cpp index 75f81fdb4..91dc207b1 100644 --- a/src/Project.cpp +++ b/src/Project.cpp @@ -2160,6 +2160,26 @@ wxArrayString AudacityProject::ShowOpenDialog(wxString extraformat, wxString ext return selected; } +// static method, can be called outside of a project +bool AudacityProject::IsAlreadyOpen(const wxString projPathName) +{ + wxFileName newProjPathName(projPathName); + size_t numProjects = gAudacityProjects.Count(); + for (size_t i = 0; i < numProjects; i++) + { + if (newProjPathName.SameAs(gAudacityProjects[i]->mFileName)) + { + wxString errMsg = + wxString::Format(_("%s is already open in another window."), + newProjPathName.GetName().c_str()); + wxLogError(errMsg); + wxMessageBox(errMsg, _("Error opening project"), wxOK | wxCENTRE); + return true; + } + } + return false; +} + // static method, can be called outside of a project void AudacityProject::OpenFiles(AudacityProject *proj) { @@ -2182,25 +2202,10 @@ void AudacityProject::OpenFiles(AudacityProject *proj) for (size_t ff = 0; ff < selectedFiles.GetCount(); ff++) { wxString fileName = selectedFiles[ff]; - wxFileName newFileName(fileName); - bool skip = false; - // Make sure it isn't already open - size_t numProjects = gAudacityProjects.Count(); - for (size_t i = 0; i < numProjects; i++) { - if (newFileName.SameAs(gAudacityProjects[i]->mFileName)) { - wxMessageBox(wxString::Format(_("%s is already open in another window."), - newFileName.GetName().c_str()), - _("Error opening project"), - wxOK | wxCENTRE); - skip = true; - break; - } - } - - if (skip) { - continue; - } + // Make sure it isn't already open. + if (AudacityProject::IsAlreadyOpen(fileName)) + continue; // Skip ones that are already open. gPrefs->Write(wxT("/DefaultOpenPath"), wxPathOnly(fileName)); @@ -2250,7 +2255,8 @@ bool AudacityProject::WarnOfLegacyFile( ) } - +// FIX-ME? This should return a result that is checked. +// See comment in AudacityApp::MRUOpen(). void AudacityProject::OpenFile(wxString fileName, bool addtohistory) { // On Win32, we may be given a short (DOS-compatible) file name on rare @@ -2259,6 +2265,17 @@ void AudacityProject::OpenFile(wxString fileName, bool addtohistory) fileName = PlatformCompatibility::ConvertSlashInFileName( PlatformCompatibility::GetLongFileName(fileName)); + // Make sure it isn't already open. + // Vaughan, 2011-03-25: This was done previously in AudacityProject::OpenFiles() + // and AudacityApp::MRUOpen(), but if you open an aup file by double-clicking it + // from, e.g., Win Explorer, it would bypass those, get to here with no check, + // then open a new project from the same data with no warning. + // This was reported in http://bugzilla.audacityteam.org/show_bug.cgi?id=137#c17, + // but is not really part of that bug. Anyway, prevent it! + if (AudacityProject::IsAlreadyOpen(fileName)) + return; + + // Data loss may occur if users mistakenly try to open ".aup.bak" files // left over from an unsuccessful save or by previous versions of Audacity. // So we always refuse to open such files. diff --git a/src/Project.h b/src/Project.h index 6326e5013..299280ce8 100644 --- a/src/Project.h +++ b/src/Project.h @@ -189,6 +189,7 @@ class AUDACITY_DLL_API AudacityProject: public wxFrame, */ static wxArrayString ShowOpenDialog(wxString extraformat = wxEmptyString, wxString extrafilter = wxEmptyString); + static bool IsAlreadyOpen(const wxString projPathName); static void OpenFiles(AudacityProject *proj); void OpenFile(wxString fileName, bool addtohistory = true); bool WarnOfLegacyFile( );