From c24f65a5f78cbe0dd11d926d28ea8d8a198d90a0 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Mon, 16 Nov 2020 22:51:18 +0100 Subject: [PATCH] Fix even more broken cases for closing child/dialog windows on win32. --- .../ControlCatalog/Pages/DialogsPage.xaml.cs | 29 +++++++++++++--- .../Avalonia.Win32/WindowImpl.AppWndProc.cs | 19 ++--------- src/Windows/Avalonia.Win32/WindowImpl.cs | 34 +++++++++++++++++++ 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/samples/ControlCatalog/Pages/DialogsPage.xaml.cs b/samples/ControlCatalog/Pages/DialogsPage.xaml.cs index cf6c771e34..49921fb7f6 100644 --- a/samples/ControlCatalog/Pages/DialogsPage.xaml.cs +++ b/samples/ControlCatalog/Pages/DialogsPage.xaml.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Reflection; using Avalonia.Controls; using Avalonia.Dialogs; +using Avalonia.Layout; using Avalonia.Markup.Xaml; #pragma warning disable 4014 @@ -112,11 +113,29 @@ namespace ControlCatalog.Pages private Window CreateSampleWindow() { - var window = new Window(); - window.Height = 200; - window.Width = 200; - window.Content = new TextBlock { Text = "Hello world!" }; - window.WindowStartupLocation = WindowStartupLocation.CenterOwner; + Button button; + + var window = new Window + { + Height = 200, + Width = 200, + Content = new StackPanel + { + Spacing = 4, + Children = + { + new TextBlock { Text = "Hello world!" }, + (button = new Button + { + HorizontalAlignment = HorizontalAlignment.Center, + Content = "Click to close" + }) + } + }, + WindowStartupLocation = WindowStartupLocation.CenterOwner + }; + + button.Click += (_, __) => window.Close(); return window; } diff --git a/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs b/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs index d770f4b211..78de681403 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.AppWndProc.cs @@ -65,23 +65,10 @@ namespace Avalonia.Win32 return IntPtr.Zero; } - // Based on https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Window.cs#L4270-L4337 - // We need to enable parent window before destroying child window to prevent OS from activating a random window behind us. - // This is described here: https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-enablewindow#remarks - // Our window closed callback will set enabled state to a correct value after child window gets destroyed. - // We need to verify if parent is still alive (perhaps it got destroyed somehow). - if (_parent != null && IsWindow(_parent._hwnd)) - { - var wasActive = GetActiveWindow() == _hwnd; - - _parent.SetEnabled(true); + BeforeCloseCleanup(false); - // We also need to activate our parent window since again OS might try to activate a window behind if it is not set. - if (wasActive) - { - SetActiveWindow(_parent._hwnd); - } - } + // Used to distinguish between programmatic and regular close requests. + _isCloseRequested = true; break; } diff --git a/src/Windows/Avalonia.Win32/WindowImpl.cs b/src/Windows/Avalonia.Win32/WindowImpl.cs index c603128a18..2483356e9a 100644 --- a/src/Windows/Avalonia.Win32/WindowImpl.cs +++ b/src/Windows/Avalonia.Win32/WindowImpl.cs @@ -83,6 +83,7 @@ namespace Avalonia.Win32 private POINT _maxTrackSize; private WindowImpl _parent; private ExtendClientAreaChromeHints _extendChromeHints = ExtendClientAreaChromeHints.Default; + private bool _isCloseRequested; public WindowImpl() { @@ -506,6 +507,13 @@ namespace Avalonia.Win32 if (_hwnd != IntPtr.Zero) { + // Detect if we are being closed programmatically - this would mean that WM_CLOSE was not called + // and we didn't prepare this window for destruction. + if (!_isCloseRequested) + { + BeforeCloseCleanup(true); + } + DestroyWindow(_hwnd); _hwnd = IntPtr.Zero; } @@ -948,6 +956,32 @@ namespace Avalonia.Win32 SetFocus(_hwnd); } } + + private void BeforeCloseCleanup(bool isDisposing) + { + // Based on https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Window.cs#L4270-L4337 + // We need to enable parent window before destroying child window to prevent OS from activating a random window behind us (or last active window). + // This is described here: https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-enablewindow#remarks + // We need to verify if parent is still alive (perhaps it got destroyed somehow). + if (_parent != null && IsWindow(_parent._hwnd)) + { + var wasActive = GetActiveWindow() == _hwnd; + + // We can only set enabled state if we are not disposing - generally Dispose happens after enabled state has been set. + // Ignoring this would cause us to enable a window that might be disabled. + if (!isDisposing) + { + // Our window closed callback will set enabled state to a correct value after child window gets destroyed. + _parent.SetEnabled(true); + } + + // We also need to activate our parent window since again OS might try to activate a window behind if it is not set. + if (wasActive) + { + SetActiveWindow(_parent._hwnd); + } + } + } private void MaximizeWithoutCoveringTaskbar() {