Excel VBA bug/anomaly - ActiveWorkbook.Save changes Workbook_BeforeSave function

Question

In my code I use a Workbook_BeforeSave function that does some text formatting.

When I hit the Save button, it runs and formats the size and font type of some cells.
Here is part of my code that does the job:

Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)

Dim c As Range
Dim rng As Range
Set rng = ActiveSheet.UsedRange.Cells

For Each c In rng
    If ispcname(c.Value) = True Or isip(c.Value) = True Then ActiveSheet.Hyperlinks.Add Anchor:=c, Address:="": c.HorizontalAlignment = xlCenter: c = StrConv(c, vbProperCase): c.Font.Name = "Arial": c.Font.Size = "10"
    If Right(c, 1) = "$" Then 
        y = c.Column: x = c.Row

        Dim i As Integer
        For i = 1 To rng.Rows.Count
            If LCase(Cells(i, y).Value) = "backup" Then
                If  Right(c, 1) = "$" Then Cells(x, y) = Cells(x, y - 2) & "$": ActiveSheet.Hyperlinks.Add Anchor:=c, Address:="": c.Font.Name = "Calibri": c.Font.Size = "10": c.HorizontalAlignment = xlCenter: c.Font.Color = RGB(192, 0, 0)
            End If
        Next i
    End If
Next c
End Sub

I have recently implemented a code that will save the Workbook if it is closed.

Private Sub Workbook_BeforeClose(Cancel As Boolean)
    Application.DisplayAlerts = False
        ActiveWorkbook.Save
    Application.DisplayAlerts = True
End Sub

Then something went wrong that I can't explain. When ActiveWorkbook.Save runs, the cells that should change into Calibri, change into Arial instead, size remains unmodified, color is working normally. However when I manually hit the save button it works as it should. (changes the cells back to Calibri)

There is no other code interfering because when I commented out the part that changes the font type to Calibri, the ActiveWorkbook.Save also stopped changing it into Arial as well.

My questions are:

Why is this happening? Is this a bug?
Is there any workaround?

I am using Excel 2007.

Comintern · Accepted Answer

Not sure why this is happening at all, but one workaround seems to be calling Workbook_BeforeSave manually and then disabling it for the ActiveWorkbook.Save call:

Private Sub Workbook_BeforeClose(Cancel As Boolean)
    Application.DisplayAlerts = False
    Application.EnableEvents = False
    Workbook_BeforeSave False, False    'Manual call.
    Me.Save                             'Save without the event firing again.
    Application.EnableEvents = True
    Application.DisplayAlerts = True
End Sub

That said, you also have some curious logic in the Workbook_BeforeSave handler. First, I don't think that For i = 1 To rng.Rows.Count is doing what you think it's doing. It isn't necessarily going to loop over the entire column, because UsedRange.Cells doesn't have to start at row 1. If your used range is something like $A$4:$Z$100, rng.Rows.Count will be 97 and all of your references to Cells(i, y) would be off by 3.

It also isn't clear if ispcname(c.Value) = True Or isip(c.Value) = True and Right(c, 1) = "$" are mutually exclusive. If they are, If Right(c, 1) = "$" should actually be an ElseIf.

Couple other things:

Executing 5 different statements in a one line If statement is incredibly hard to read and that makes it error prone. Use actual If...End If blocks unless the action is something trivial like Exit Sub.
The second If Right(c, 1) = "$" Then is always true. It can be removed completely.
After actually formatting your code in a readable way, it's clear that you are using properties of c all over the place inside the For Each c In rng loop. I'd put it in a With block.
You only need to use ActiveSheet once. After that, you can either get it from rng.Parent or (much better) get a reference to it.
Get in the habit of using String returning functions instead of Variant returning functions when you need a String. I.e. Right$ instead of Right - the latter performs an implicit cast.
Fully qualify all of your references to Cells.
Avoid implicitly using the default properties of objects, i.e. Range.Value.
Use Long for row counters, not Integer to avoid the possibility for overflows.
Use vbNullString instead of the literal "".
Font.Size is measured in points. It should be a number, not a string.

It should look more like this:

Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)
    Dim c As Range
    Dim sh As Worksheet
    Set sh = ActiveSheet    'Tip 4
    Dim rng As Range
    Set rng = sh.UsedRange.Cells

    For Each c In rng
        With c  'Tip 3
            If ispcname(.Value) Or isip(.Value) Then  'Tip 1
                sh.Hyperlinks.Add Anchor:=c, Address:=vbNullString    'Tips 4 and 9
                .HorizontalAlignment = xlCenter
                .Value = StrConv(.Value, vbProperCase)  'Tip 7
                .Font.Name = "Arial"
                .Font.Size = 10     'Tip 10
            End If  'Pretty sure this should be an ElseIf structure here.
            If Right$(.Value, 1) = "$" Then  'Tips 5 and 7.
                y = .Column
                x = .Row
                Dim i As Long   'Tip 8
                For i = 1 To rng.Rows.Count     'This is most likely wrong.
                    'Tip 2 used to be here.
                    If LCase$(sh.Cells(i, y).Value) = "backup" Then     'Tips 1, 5, and 6
                        .Value = sh.Cells(x, y - 2).Value & "$"   'Tips 4, 6, and 7
                        sh.Hyperlinks.Add Anchor:=c, Address:=vbNullString    'Tips 4 and 9
                        .Font.Name = "Calibri"
                        .Font.Size = 10     'Tip 10
                        .HorizontalAlignment = xlCenter
                        .Font.Color = RGB(192, 0, 0)
                    End If
                Next i
            End If
        End With
    Next c
End Sub

Excel VBA bug/anomaly - ActiveWorkbook.Save changes Workbook_BeforeSave function

Answers (2)

Related Questions