Recursion in subroutine Workbook_BeforeSave

jeffmb

Board Regular
Joined
Jul 18, 2008
Messages
187
I have a recursion problem in a workbook that comes up only when a second workbook is open at the same time and I click close on the first workbook, at which time Microsoft asks if I want to save the workbook. If I say yes it will come back and ask me again. This will repeat until I click on Cancel. I find that when I finally click cancel the workbook is actually saved. Then when I click on close the workbook will close without incident.
In the following code the subroutine FSO_File_Backup is not requested (it does not update the workbook in any case).
If I save the file workbook first and then click on close the problem does not arise.

I hope my description of the problem is clear.

VBA Code:
Private Sub Workbook_BeforeClose(Cancel As Boolean)
   
    On_Close = False
    If ThisWorkbook.Saved = True Then
        Resp = MsgBox("Do you wish to backup this workbook?", vbYesNo, "Backup?")
        If Resp = vbYes Then
            FSO_File_Backup
        End If
    Else
        On_Close = True
    End If
   
End Sub
________________________________________________________________________________________________________________________
Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)
   
'   This will cancel the user-requested save event and then hide the business sheets,
'   show the setup (instruction) sheet, save workbook via this macro,
'   and then set the business screens back again.
   
    uSheet = ActiveSheet.Name
    Cancel = True
    Application.ScreenUpdating = False
   
   
    '   Set up sheets for close configuration
    On Error GoTo Screwed_up

    Sheets("Startup").Visible = xlSheetVisible
    For Each ws In Worksheets
        If Not ws.Name = "Startup" Then
            ws.Visible = xlSheetVeryHidden
        End If
    Next
   
    On Error GoTo 0
    Application.EnableEvents = False        ' Prevent recursion
    ActiveWorkbook.Save
    Application.EnableEvents = True
   
    '   Put everything back the way it was
    If Not On_Close Then
        If Not uSheet = "Reconcile" Then
            For Each ws In Worksheets
                If Not ws.Name = "Startup" And _
                   Not ws.Name = "Reconcile" Then
                    ws.Visible = xlSheetVisible
                End If
            Next
        Else
            Sheets(uSheet).Visible = xlSheetVisible
        End If
        Sheets("Startup").Visible = xlSheetVeryHidden
        Sheets(uSheet).Select
    End If
           
    Application.ScreenUpdating = True
    ActiveWorkbook.Saved = True     ' Make excel think workbook is saved
   
   If On_Close Then
        Resp = MsgBox("Do you wish to backup this workbook?", vbYesNo, "Backup?")
        If Resp = vbYes Then
            FSO_File_Backup
        End If
    End If

GoTo Skip_it
   
Screwed_up:
    MsgBox "Error: " & Err.Number & "/" & Err.Description
Skip_it:
End Sub
 

Excel Facts

Create a Pivot Table on a Map
If your data has zip codes, postal codes, or city names, select the data and use Insert, 3D Map. (Found to right of chart icons).
I think the problem is here:

You have a Workbook.Save statement inside the Workbook_BeforeSave procedure.

So when you trigger the WorkBook_BeforeSave, it runs till WorkBook_Save, where it again triggers the Workbook_BeforeSave.
 
Upvote 0
This problem arises only when there is a second workbook open.
 
Last edited:
Upvote 0
And the save statement is surrounded:
VBA Code:
This problem arises only when there is a second workbook open. And the save statement is surrounded: [CODE=vba]
   Application.EnableEvents = False        ' Prevent recursion
    ActiveWorkbook.Save
    Application.EnableEvents = True
 
Upvote 0
not a lot of time to spend on this but see if the changes I have made are of any help

VBA Code:
Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)
   
'   This will cancel the user-requested save event and then hide the business sheets,
'   show the setup (instruction) sheet, save workbook via this macro,
'   and then set the business screens back again.

    On Error GoTo myerror
   
    uSheet = ActiveSheet.Name
    Cancel = True
   
'Prevent recursion
    With Application
        .ScreenUpdating = False: .EnableEvents = False
    End With
   
   
'Set up sheets for close configuration
   
    Sheets("Startup").Visible = xlSheetVisible
    For Each ws In Worksheets
        If Not ws.Name = "Startup" Then
            ws.Visible = xlSheetVeryHidden
        End If
    Next
     
    ActiveWorkbook.Save

'   Put everything back the way it was
    If Not On_Close Then
        If Not uSheet = "Reconcile" Then
            For Each ws In Worksheets
                If Not ws.Name = "Startup" And _
                Not ws.Name = "Reconcile" Then
                    ws.Visible = xlSheetVisible
                End If
            Next
        Else
            Sheets(uSheet).Visible = xlSheetVisible
        End If
        Sheets("Startup").Visible = xlSheetVeryHidden
        Sheets(uSheet).Select
    End If

    ActiveWorkbook.Saved = True     ' Make excel think workbook is saved

    If On_Close Then
        Resp = MsgBox("Do you wish to backup this workbook?", vbYesNo, "Backup?")
        If Resp = vbYes Then
            FSO_File_Backup
        End If
    End If

myerror:
    With Application
        .ScreenUpdating = True: .EnableEvents = True
    End With
    If Err <> 0 Then MsgBox "Error: " & Err.Number & "/" & Err.Description
End Sub

You have not shown where you variables are declared but assume at top of code page?
Note: for later versions of excel you can use AfterSave event to re-instate your worksheets.

Hope Helpful

Dave
 
Last edited:
Upvote 0
Thanks for your input, dmt32. I tried your code but, unfortunately, it had the same results, which I expected. However, your advice about AfterSave sounds promising. I will give that a try a little later and let you know how it works out.

Your assumption about the variables is correct:
VBA Code:
Option Explicit
Dim lastrow As Integer
Dim uSheet As String
Dim ws As Worksheet
Dim Resp
 
Upvote 0
Thanks for your input, dmt32. I tried your code but, unfortunately, it had the same results, which I expected. However, your advice about AfterSave sounds promising. I will give that a try a little later and let you know how it works out.

Its largely your code just changed about a little but as with all these things, just sometimes bit of a guess if changes will work.
Hopefully, you will find the cause.

Dave
 
Upvote 0
Dave,

Using AfterSave did the trick. It even allowed me to simplify the code a bit. I still have no idea why the original code failed when a second workbook was open but worked fine when it was the only open workbook. Thanks again for your suggestion.
 
Upvote 0
Dave,

Using AfterSave did the trick. It even allowed me to simplify the code a bit. I still have no idea why the original code failed when a second workbook was open but worked fine when it was the only open workbook. Thanks again for your suggestion.

Glad you found solution & suggestion helped - you may want to think about posting what you did to help others in future searching to solve a similar problem

Many thanks for feedback

Dave
 
Upvote 0
The updated code now includes the AfterSave event:

VBA Code:
Dim lastrow As Integer
Dim Rw As Long
Dim ws As Worksheet
Dim Resp
_________________________________________________________________________________________________
Private Sub Workbook_AfterSave(ByVal Success As Boolean)

'       Put screens back the way they were

    If Not On_Close Then
        If Not uSheet = "Reconcile" Then
            For Each ws In Worksheets
                If Not ws.Name = "Startup" And _
                   Not ws.Name = "Reconcile" Then
                    ws.Visible = xlSheetVisible
                End If
            Next
        Else
            Sheets(uSheet).Visible = xlSheetVisible
        End If
        Sheets("Startup").Visible = xlSheetVeryHidden
        Sheets(uSheet).Select
    End If
          
'    ActiveWorkbook.Saved = True     ' Make excel think workbook is saved
    Application.ScreenUpdating = True


End Sub
_________________________________________________________________________________________________
Private Sub Workbook_BeforeClose(Cancel As Boolean)
  
    On_Close = False
    If ThisWorkbook.Saved = True Then
        Resp = MsgBox("Do you wish to backup this workbook?", vbYesNo, "Backup?")
        If Resp = vbYes Then
            FSO_File_Backup
        End If
    Else
        On_Close = True
    End If
  
End Sub
_________________________________________________________________________________________________
Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)
  
'   This will  hide the business sheets,
'   show the setup (instruction) sheet, save workbook via this macro.
'   The business screens will be reset in the AfterSave event.
  
    uSheet = ActiveSheet.Name
    Application.ScreenUpdating = False
  
  
    '   Set up sheets for close configuration
    On Error GoTo Screwed_up

    Sheets("Startup").Visible = xlSheetVisible
    For Each ws In Worksheets
        If Not ws.Name = "Startup" Then
            ws.Visible = xlSheetVeryHidden
        End If
    Next
    On Error GoTo 0
  
  
   If On_Close Then
        Resp = MsgBox("Do you wish to backup this workbook?", vbYesNo, "Backup?")
        If Resp = vbYes Then
            FSO_File_Backup
        End If
    End If

Screwed_up:
    If Err <> 0 Then
        MsgBox "Error: " & Err.Number & "/" & Err.Description
    End If
  
End Sub
 
Upvote 0

Forum statistics

Threads
1,213,496
Messages
6,113,993
Members
448,539
Latest member
alex78

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top