Skip to content

Commit 2d3e174

Browse files
committed
Make CommandBarButtonEventArgs safe by copying properties of button rather than marshaling the button across event boundaries.
1 parent d4b45fa commit 2d3e174

File tree

5 files changed

+27
-18
lines changed

5 files changed

+27
-18
lines changed

Rubberduck.Core/UI/Command/MenuItems/CommandBars/AppCommandBarBase.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ private void child_Click(object sender, CommandBarButtonClickEventArgs e)
224224
ICommandMenuItem item;
225225
try
226226
{
227-
item = _items.Select(kvp => kvp.Key).SingleOrDefault(menu => menu.GetType().FullName == e.Control.Tag);
227+
item = _items.Select(kvp => kvp.Key).SingleOrDefault(menu => menu.GetType().FullName == e.Tag);
228228
}
229229
catch (COMException exception)
230230
{
@@ -236,7 +236,7 @@ private void child_Click(object sender, CommandBarButtonClickEventArgs e)
236236
return;
237237
}
238238

239-
Logger.Debug("({0}) Executing click handler for commandbar item '{1}', hash code {2}", GetHashCode(), e.Control.Caption, e.Control.Target.GetHashCode());
239+
Logger.Debug("({0}) Executing click handler for commandbar item '{1}', hash code {2}", GetHashCode(), e.Caption, e.TargetHashCode);
240240
item.Command.Execute(null);
241241
}
242242
}

Rubberduck.Core/UI/Command/MenuItems/ParentMenus/ParentMenuItemBase.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,13 +181,13 @@ private ICommandBarControl InitializeChildControl(ICommandMenuItem item)
181181

182182
private void child_Click(object sender, CommandBarButtonClickEventArgs e)
183183
{
184-
var item = _items.Select(kvp => kvp.Key).SingleOrDefault(menu => e.Control.Tag.EndsWith(menu.GetType().Name)) as ICommandMenuItem;
184+
var item = _items.Select(kvp => kvp.Key).SingleOrDefault(menu => e.Tag.EndsWith(menu.GetType().Name)) as ICommandMenuItem;
185185
if (item == null)
186186
{
187187
return;
188188
}
189189

190-
Logger.Debug("({0}) Executing click handler for menu item '{1}', hash code {2}", GetHashCode(), e.Control.Caption, e.Control.Target.GetHashCode());
190+
Logger.Debug("({0}) Executing click handler for menu item '{1}', hash code {2}", GetHashCode(), e.Caption, e.TargetHashCode);
191191
item.Command.Execute(null);
192192
}
193193
}

Rubberduck.VBEEditor/SafeComWrappers/Office/CommandBarButtonClickEventArgs.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@ public class CommandBarButtonClickEventArgs : EventArgs
77
{
88
public CommandBarButtonClickEventArgs(ICommandBarButton control)
99
{
10-
Control = control;
10+
Tag = control.Tag;
11+
Caption = control.Caption;
12+
TargetHashCode = control.Target.GetHashCode();
1113
}
1214

13-
public ICommandBarButton Control { get; }
15+
public string Tag { get; }
16+
public string Caption { get; }
17+
public int TargetHashCode { get; }
1418

1519
public bool Cancel { get; set; }
1620
public bool Handled { get; set; } // Only used in VB6

Rubberduck.VBEditor.VB6/SafeComWrappers/Office/CommandBarButton.cs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -277,14 +277,17 @@ void VB._dispCommandBarControlEvents.Click(object Ctrl, ref bool Handled, ref bo
277277
{
278278
return;
279279
}
280-
var button = new CommandBarButton((MSO.CommandBarButton)Ctrl, _vbe);
281280

282-
System.Diagnostics.Debug.Assert(handler.GetInvocationList().Length == 1, "Multicast delegate is registered more than once.");
281+
using (var button = new CommandBarButton((MSO.CommandBarButton) Ctrl, _vbe))
282+
{
283+
System.Diagnostics.Debug.Assert(handler.GetInvocationList().Length == 1,
284+
"Multicast delegate is registered more than once.");
283285

284-
var args = new CommandBarButtonClickEventArgs(button);
285-
handler.Invoke(this, args);
286-
CancelDefault = args.Cancel;
287-
Handled = args.Handled;
286+
var args = new CommandBarButtonClickEventArgs(button);
287+
handler.Invoke(this, args);
288+
CancelDefault = args.Cancel;
289+
Handled = args.Handled;
290+
}
288291
}
289292

290293
public event EventHandler Disposing;

Rubberduck.VBEditor.VBA/SafeComWrappers/Office/CommandBarButton.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -314,19 +314,21 @@ public event EventHandler<CommandBarButtonClickEventArgs> Click
314314

315315
void MSO._CommandBarButtonEvents.Click(MSO.CommandBarButton Ctrl, ref bool CancelDefault)
316316
{
317-
var button = new CommandBarButton(Ctrl);
318317
var handler = _click;
319318
if (handler == null || IsWrappingNullReference)
320319
{
321-
button.Dispose();
322320
return;
323321
}
324322

325-
System.Diagnostics.Debug.Assert(handler.GetInvocationList().Length == 1, "Multicast delegate is registered more than once.");
323+
using (var button = new CommandBarButton(Ctrl))
324+
{
325+
System.Diagnostics.Debug.Assert(handler.GetInvocationList().Length == 1,
326+
"Multicast delegate is registered more than once.");
326327

327-
var args = new CommandBarButtonClickEventArgs(button);
328-
handler.Invoke(this, args);
329-
CancelDefault = args.Cancel;
328+
var args = new CommandBarButtonClickEventArgs(button);
329+
handler.Invoke(this, args);
330+
CancelDefault = args.Cancel;
331+
}
330332
}
331333

332334
public event EventHandler Disposing;

0 commit comments

Comments
 (0)