hardcodet / wpf-notifyicon

NotifyIcon (aka system tray icon or taskbar icon) for the WPF platform
Other
810 stars 125 forks source link

ContextMenu make MessageBox close #74

Closed MisakaCirno closed 2 years ago

MisakaCirno commented 2 years ago

I'm try to create a window less application, so I add ContextMenu for my icon and add click event to MenuItem: `

` The event code is very simple: `MessageBox.Show("Hello, world!");` Button when I click the MenuItem, the MessageBox appeared and disappeared immediately. After few test, I change the code to: `await Task.Delay(200); MessageBox.Show("Hello, world!");` the MessageBox is displaying normally. The MessageBox can show normally after ContextMenu close, or close with ContextMenu at same time. But create new Window was normal too: `MainWindow mainWindow = new(); mainWindow.Show();` How can I slove this problem?
MisakaCirno commented 2 years ago

I may have figured out the cause of this problem:

The MessageBox use a WindowAPI.

Reference: http://pinvoke.net/default.aspx/user32/MessageBox.html

[DllImport("user32.dll", SetLastError = true, CharSet= CharSet.Auto)]
public static extern int MessageBox(IntPtr hWnd, String text, String caption, uint type);

The parameters include the hWnd of type IntPtr.

When MessageBox.Show() is called in WPF, the ShowCore() method is called inside the MessageBox class, which internally handles this parameter.

Reference: https://referencesource.microsoft.com/#PresentationFramework/src/Framework/System/Windows/MessageBox.cs at line 483

...

if ( (options & (MessageBoxOptions.ServiceNotification | MessageBoxOptions.DefaultDesktopOnly)) ! = 0) 
{
    // Demand UnmangedCode permissions if using ServiceNotification/DefaultDesktopOnly.
    // Details in DevDiv 163043.
    SecurityHelper.DemandUnmanagedCode()
    if (owner ! = IntPtr.Zero)
    {
        throw new ArgumentException(SR.Get(SRID.CantShowMBServiceWithOwner));
    }                
}
else
{                                    
    if (owner == IntPtr.Zero)
    {
        owner = UnsafeNativeMethods.GetActiveWindow();
    }                
}

...

I think, when using MessageBox in WPF, the currently active window is automatically selected as the hWnd, and since there is no window, the ContextMenu is used as the window.

We can prove this using the API:

[DllImport("user32.dll")]
static extern IntPtr GetActiveWindow();

And use it in click event:

private void MenuItem_Click(object sender, RoutedEventArgs e)
{
    var t = GetActiveWindow();
}

Then find the t value in spy++, is the ContextMenu.

When the MenuItem on the ContextMenu is clicked, the ContextMenu will be closed, at that time the owner of MessageBox disappears and MessageBox is forced to be closed.

Based on the above research, I got the following solutions.

1.Delay the appearance of the MessageBox so that it is displayed after the ContextMenu is closed.

await Task.Delay(200); 
MessageBox.Show("Hello, world!");

2.Call the API itself and set hWnd to IntPtr.Zero.

[DllImport("user32.dll", SetLastError = true, CharSet = CharSet.Auto)]
public static extern int MessageBox(IntPtr hWnd, String text, String caption, uint type);

and

private void MenuItem_Click(object sender, RoutedEventArgs e)
{
    MessageBox(IntPtr.Zero, "Hello, world!", "Test", 0);
}

3.Modify the value of MessageBoxOptions according to the source code of MessageBox in WPF.

MessageBox.Show(
    "Hello, world!",
    "Test",
    MessageBoxButton.OK,
    MessageBoxImage.Error,
    MessageBoxResult.OK,
    MessageBoxOptions.ServiceNotification | MessageBoxOptions.DefaultDesktopOnly);
Lakritzator commented 2 years ago

I think this is good research, and I agree with your finding. 👍 You already answered your own question.

From my point of view, this is not an issue with this project, but something generic with WPF. It's good to have it in the list of issues though, for when other people have the same (or similar) issue. So I really thank you for the answer.

To be clear, I would NOT use option 1, that is a horrible hack which might seem to work for this case but also let's people believe they can do more of that. The delay might be enough for your setup & use-case, it might not be enough on every PC, and it's an issue waiting for surface. Also the await forces the event handler to be async, and you might get issues with where any exceptions arrive or with having out of order code (something which usually waited for the messagebox to finish). It's also possible you come to issues with the continuation context and it will not run on the UI thread (not in this case). I love Tasks (async await) but it's a big gun which people need to understand.

When you use option 2, you are no longer "inside" WPF, and going the Win32 route directly, nothing against it but do consider that it means another way of getting complexity inside your app (maintaining pinvokes).

For me this means that option 3, if that works, is the best by far.

Some other options to consider, if the 3 above doesn't fit your need:

Lakritzator commented 2 years ago

I'm closing this issue, and I think the question was answered by the author of the issue.

MisakaCirno commented 2 years ago

I think this is good research, and I agree with your finding. 👍 You already answered your own question.

From my point of view, this is not an issue with this project, but something generic with WPF. It's good to have it in the list of issues though, for when other people have the same (or similar) issue. So I really thank you for the answer.

To be clear, I would NOT use option 1, that is a horrible hack which might seem to work for this case but also let's people believe they can do more of that. The delay might be enough for your setup & use-case, it might not be enough on every PC, and it's an issue waiting for surface. Also the await forces the event handler to be async, and you might get issues with where any exceptions arrive or with having out of order code (something which usually waited for the messagebox to finish). It's also possible you come to issues with the continuation context and it will not run on the UI thread (not in this case). I love Tasks (async await) but it's a big gun which people need to understand.

When you use option 2, you are no longer "inside" WPF, and going the Win32 route directly, nothing against it but do consider that it means another way of getting complexity inside your app (maintaining pinvokes).

For me this means that option 3, if that works, is the best by far.

Some other options to consider, if the 3 above doesn't fit your need:

  • Supply a null as the window, although not tested, this might work MessageBox.Show(null, "Hello, world!") although maybe it needs a cast to make the method matching possible.
  • If that doesn't work, try to use the WinForms MessageBox with a null for the window.

Thanks for your reply, and just now I found a new way of thinking to solve this problem. Add a variable to App.xaml.cs:

bool isNeedShowMessageBox = false;

When the MenuItem is pressed:

private void MenuItem_Click(object sender, RoutedEventArgs e)
{
    isNeedShowMessageBox = true;
}

Add event for ContextMenu:

<ContextMenu Closed="ContextMenu_Closed">

Then:

private void ContextMenu_Closed(object sender, RoutedEventArgs e)
{
    if (isNeedShowMessageBox)
    {
        MessageBox.Show();
        isNeedShowMessageBox=false;
    }
}

This seems to be working just fine.

In subsequent attempts I found that not only MessageBox, but also OpenFileDialog or other similar windows would have the same forced closure, but if handled in this way, the problem should be avoided.

fgimian commented 1 year ago

Another method I found which worked well was to use the overload of MessageBox.Show which takes an owner argument and pass your application window as that argument. The advantage of using this approach is that you don't lose your visual styles in the dialog box.

These are the overloads I'm referring to:

        public static MessageBoxResult Show(Window owner, string messageBoxText);
        public static MessageBoxResult Show(Window owner, string messageBoxText, string caption);
        public static MessageBoxResult Show(Window owner, string messageBoxText, string caption, MessageBoxButton button);
        public static MessageBoxResult Show(Window owner, string messageBoxText, string caption, MessageBoxButton button, MessageBoxImage icon);
        public static MessageBoxResult Show(Window owner, string messageBoxText, string caption, MessageBoxButton button, MessageBoxImage icon, MessageBoxResult defaultResult);
        public static MessageBoxResult Show(Window owner, string messageBoxText, string caption, MessageBoxButton button, MessageBoxImage icon, MessageBoxResult defaultResult, MessageBoxOptions options);

Hope this helps someone 😄 Fotis