Fix VirtualView null exceptions and popup removal race condition
- Fix RemoveAsync race condition: close popup before disconnecting handler - Add null-safe VirtualView access with try-catch on all platforms - Properly unsubscribe SizeChanged event in PopupPageHandler - Fix Android Window type ambiguity with fully qualified name - Fix README path in csproj The root cause was layout events firing during popup removal while the handler was being disconnected. Now we close the popup first to stop events, then clean up. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -110,6 +110,6 @@
|
|||||||
|
|
||||||
<ItemGroup>
|
<ItemGroup>
|
||||||
<None Include="icon.png" Pack="true" PackagePath="\" />
|
<None Include="icon.png" Pack="true" PackagePath="\" />
|
||||||
<None Include="..\..\..\..\README.md" Pack="true" PackagePath="\" />
|
<None Include="..\..\README.md" Pack="true" PackagePath="\" />
|
||||||
</ItemGroup>
|
</ItemGroup>
|
||||||
</Project>
|
</Project>
|
||||||
|
|||||||
@@ -44,7 +44,7 @@ public class AndroidMopups : IPopupPlatform
|
|||||||
return AddAsync(page, null);
|
return AddAsync(page, null);
|
||||||
}
|
}
|
||||||
|
|
||||||
public Task AddAsync(PopupPage page, Window? targetWindow)
|
public Task AddAsync(PopupPage page, Microsoft.Maui.Controls.Window? targetWindow)
|
||||||
{
|
{
|
||||||
HandleAccessibility(true, page.DisableAndroidAccessibilityHandling, page);
|
HandleAccessibility(true, page.DisableAndroidAccessibilityHandling, page);
|
||||||
|
|
||||||
|
|||||||
@@ -59,7 +59,11 @@ namespace Mopups.Platforms.MacCatalyst
|
|||||||
|
|
||||||
if (Equals(subview, view))
|
if (Equals(subview, view))
|
||||||
{
|
{
|
||||||
((PopupPage)Handler.VirtualView).SendBackgroundClick();
|
try
|
||||||
|
{
|
||||||
|
(Handler.VirtualView as PopupPage)?.SendBackgroundClick();
|
||||||
|
}
|
||||||
|
catch (InvalidOperationException) { }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -74,30 +78,36 @@ namespace Mopups.Platforms.MacCatalyst
|
|||||||
|
|
||||||
void UpdateSize(PopupPageRenderer handler)
|
void UpdateSize(PopupPageRenderer handler)
|
||||||
{
|
{
|
||||||
var currentElement = ((PopupPage)Handler.VirtualView);
|
try
|
||||||
|
|
||||||
if (handler.Handler.PlatformView?.Superview?.Frame == null || currentElement == null)
|
|
||||||
return;
|
|
||||||
|
|
||||||
var superviewFrame = handler.Handler.PlatformView.Superview.Frame;
|
|
||||||
var applicationFrame = UIScreen.MainScreen.ApplicationFrame;
|
|
||||||
|
|
||||||
var systemPadding = new Thickness
|
|
||||||
{
|
{
|
||||||
Left = applicationFrame.Left,
|
var currentElement = Handler.VirtualView as PopupPage;
|
||||||
Top = applicationFrame.Top,
|
|
||||||
Right = applicationFrame.Right - applicationFrame.Width - applicationFrame.Left,
|
|
||||||
Bottom = applicationFrame.Bottom - applicationFrame.Height - applicationFrame.Top + handler.KeyboardBounds.Height
|
|
||||||
};
|
|
||||||
|
|
||||||
if ((handler.Handler.VirtualView.Width != superviewFrame.Width && handler.Handler.VirtualView.Height != superviewFrame.Height)
|
if (handler.Handler.PlatformView?.Superview?.Frame == null || currentElement == null)
|
||||||
|| currentElement.SystemPadding.Bottom != systemPadding.Bottom)
|
return;
|
||||||
{
|
|
||||||
currentElement.BatchBegin();
|
var superviewFrame = handler.Handler.PlatformView.Superview.Frame;
|
||||||
currentElement.SystemPadding = systemPadding;
|
var applicationFrame = UIScreen.MainScreen.ApplicationFrame;
|
||||||
currentElement.Layout(new Rect(currentElement.X, currentElement.Y, superviewFrame.Width, superviewFrame.Height));
|
|
||||||
currentElement.BatchCommit();
|
var systemPadding = new Thickness
|
||||||
|
{
|
||||||
|
Left = applicationFrame.Left,
|
||||||
|
Top = applicationFrame.Top,
|
||||||
|
Right = applicationFrame.Right - applicationFrame.Width - applicationFrame.Left,
|
||||||
|
Bottom = applicationFrame.Bottom - applicationFrame.Height - applicationFrame.Top + handler.KeyboardBounds.Height
|
||||||
|
};
|
||||||
|
|
||||||
|
var virtualView = handler.Handler.VirtualView;
|
||||||
|
if (virtualView != null &&
|
||||||
|
((virtualView.Width != superviewFrame.Width && virtualView.Height != superviewFrame.Height)
|
||||||
|
|| currentElement.SystemPadding.Bottom != systemPadding.Bottom))
|
||||||
|
{
|
||||||
|
currentElement.BatchBegin();
|
||||||
|
currentElement.SystemPadding = systemPadding;
|
||||||
|
currentElement.Layout(new Rect(currentElement.X, currentElement.Y, superviewFrame.Width, superviewFrame.Height));
|
||||||
|
currentElement.BatchCommit();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
catch (InvalidOperationException) { }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -6,6 +6,8 @@ namespace Mopups.Platforms.Windows
|
|||||||
{
|
{
|
||||||
public class PopupPageHandler : PageHandler
|
public class PopupPageHandler : PageHandler
|
||||||
{
|
{
|
||||||
|
private Microsoft.UI.Xaml.SizeChangedEventHandler? _sizeChangedHandler;
|
||||||
|
|
||||||
public PopupPageHandler()
|
public PopupPageHandler()
|
||||||
{
|
{
|
||||||
}
|
}
|
||||||
@@ -14,7 +16,25 @@ namespace Mopups.Platforms.Windows
|
|||||||
{
|
{
|
||||||
base.ConnectHandler(platformView);
|
base.ConnectHandler(platformView);
|
||||||
|
|
||||||
PlatformView.SizeChanged += (_, e) => VirtualView.ComputeDesiredSize(e.NewSize.Width, e.NewSize.Height);
|
_sizeChangedHandler = (_, e) =>
|
||||||
|
{
|
||||||
|
try
|
||||||
|
{
|
||||||
|
VirtualView?.ComputeDesiredSize(e.NewSize.Width, e.NewSize.Height);
|
||||||
|
}
|
||||||
|
catch (InvalidOperationException) { }
|
||||||
|
};
|
||||||
|
PlatformView.SizeChanged += _sizeChangedHandler;
|
||||||
|
}
|
||||||
|
|
||||||
|
protected override void DisconnectHandler(ContentPanel platformView)
|
||||||
|
{
|
||||||
|
if (_sizeChangedHandler != null)
|
||||||
|
{
|
||||||
|
platformView.SizeChanged -= _sizeChangedHandler;
|
||||||
|
_sizeChangedHandler = null;
|
||||||
|
}
|
||||||
|
base.DisconnectHandler(platformView);
|
||||||
}
|
}
|
||||||
|
|
||||||
protected override ContentPanel CreatePlatformView()
|
protected override ContentPanel CreatePlatformView()
|
||||||
|
|||||||
@@ -15,7 +15,20 @@ namespace Mopups.Platforms.Windows
|
|||||||
|
|
||||||
internal WinPopup? Container { get; private set; }
|
internal WinPopup? Container { get; private set; }
|
||||||
|
|
||||||
private PopupPage CurrentElement => (PopupPage)handler.VirtualView;
|
private PopupPage? CurrentElement
|
||||||
|
{
|
||||||
|
get
|
||||||
|
{
|
||||||
|
try
|
||||||
|
{
|
||||||
|
return handler.VirtualView as PopupPage;
|
||||||
|
}
|
||||||
|
catch (InvalidOperationException)
|
||||||
|
{
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public PopupPageRenderer(PopupPageHandler handler)
|
public PopupPageRenderer(PopupPageHandler handler)
|
||||||
{
|
{
|
||||||
@@ -92,7 +105,7 @@ namespace Mopups.Platforms.Windows
|
|||||||
{
|
{
|
||||||
if ((e.OriginalSource as PopupPageRenderer) == this)
|
if ((e.OriginalSource as PopupPageRenderer) == this)
|
||||||
{
|
{
|
||||||
CurrentElement.SendBackgroundClick();
|
CurrentElement?.SendBackgroundClick();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -56,34 +56,33 @@ namespace Mopups.Windows.Implementation
|
|||||||
|
|
||||||
public async Task AddAsync(PopupPage page, Window? window)
|
public async Task AddAsync(PopupPage page, Window? window)
|
||||||
{
|
{
|
||||||
var mainPage = Application.Current.MainPage;
|
// Use target window's page if provided, otherwise fall back to MainPage
|
||||||
mainPage.AddLogicalChild(page);
|
Page parentPage;
|
||||||
|
IMauiContext mauiContext;
|
||||||
var popup = new global::Microsoft.UI.Xaml.Controls.Primitives.Popup();
|
|
||||||
|
|
||||||
// Use TOPLATFORM to create your handlers
|
|
||||||
// I'd recommend wiring up all your services through ConfigureMopups
|
|
||||||
// builder.Services.AddScoped<IPopupPlatform, PopupPlatform>();
|
|
||||||
// builder.Services.AddScoped<IPopupNavigation, PopupNavigation>();
|
|
||||||
// Then you can use contructor resolution instead of singletons
|
|
||||||
// But I figured we could do that in a later PR and just work on windows here
|
|
||||||
|
|
||||||
var renderer = (PopupPageRenderer)page.ToPlatform(mainPage.Handler.MauiContext);
|
|
||||||
renderer.Prepare(popup);
|
|
||||||
popup.Child = renderer;
|
|
||||||
|
|
||||||
// https://github.com/microsoft/microsoft-ui-xaml/issues/3389
|
|
||||||
// Use the specified window's XamlRoot if provided, otherwise fall back to main window
|
|
||||||
Microsoft.UI.Xaml.Window nativeWindow;
|
Microsoft.UI.Xaml.Window nativeWindow;
|
||||||
if (window?.Handler?.PlatformView is Microsoft.UI.Xaml.Window targetWindow)
|
|
||||||
|
if (window?.Handler?.PlatformView is Microsoft.UI.Xaml.Window targetWindow && window.Page != null)
|
||||||
{
|
{
|
||||||
|
parentPage = window.Page;
|
||||||
|
mauiContext = window.Handler.MauiContext!;
|
||||||
nativeWindow = targetWindow;
|
nativeWindow = targetWindow;
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
nativeWindow = mainPage.Handler.MauiContext.Services.GetService<Microsoft.UI.Xaml.Window>();
|
parentPage = Application.Current.MainPage;
|
||||||
|
mauiContext = parentPage.Handler.MauiContext;
|
||||||
|
nativeWindow = mauiContext.Services.GetService<Microsoft.UI.Xaml.Window>();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
parentPage.AddLogicalChild(page);
|
||||||
|
|
||||||
|
var popup = new global::Microsoft.UI.Xaml.Controls.Primitives.Popup();
|
||||||
|
|
||||||
|
var renderer = (PopupPageRenderer)page.ToPlatform(mauiContext);
|
||||||
|
renderer.Prepare(popup);
|
||||||
|
popup.Child = renderer;
|
||||||
|
|
||||||
|
// https://github.com/microsoft/microsoft-ui-xaml/issues/3389
|
||||||
popup.XamlRoot = nativeWindow.Content.XamlRoot;
|
popup.XamlRoot = nativeWindow.Content.XamlRoot;
|
||||||
popup.IsOpen = true;
|
popup.IsOpen = true;
|
||||||
page.ForceLayout();
|
page.ForceLayout();
|
||||||
@@ -96,17 +95,35 @@ namespace Mopups.Windows.Implementation
|
|||||||
if (page == null)
|
if (page == null)
|
||||||
throw new Exception("Popup page is null");
|
throw new Exception("Popup page is null");
|
||||||
|
|
||||||
var renderer = (PopupPageRenderer)page.ToPlatform(Application.Current.MainPage.Handler.MauiContext);
|
// Get the handler before we start cleanup
|
||||||
var popup = renderer.Container;
|
var handler = page.Handler;
|
||||||
|
if (handler == null)
|
||||||
|
{
|
||||||
|
// Already cleaned up
|
||||||
|
page.Parent?.RemoveLogicalChild(page);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
var renderer = handler.PlatformView as PopupPageRenderer;
|
||||||
|
var popup = renderer?.Container;
|
||||||
|
|
||||||
if (popup != null)
|
if (popup != null)
|
||||||
{
|
{
|
||||||
renderer.Destroy();
|
// First close the popup to stop layout events
|
||||||
|
|
||||||
Cleanup(page);
|
|
||||||
page.Parent?.RemoveLogicalChild(page);
|
|
||||||
popup.Child = null;
|
|
||||||
popup.IsOpen = false;
|
popup.IsOpen = false;
|
||||||
|
popup.Child = null;
|
||||||
|
|
||||||
|
// Now safe to destroy and cleanup
|
||||||
|
renderer?.Destroy();
|
||||||
|
page.Parent?.RemoveLogicalChild(page);
|
||||||
|
|
||||||
|
// Disconnect handler last
|
||||||
|
Cleanup(page);
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
page.Parent?.RemoveLogicalChild(page);
|
||||||
|
Cleanup(page);
|
||||||
}
|
}
|
||||||
|
|
||||||
await Task.Delay(5);
|
await Task.Delay(5);
|
||||||
|
|||||||
@@ -59,7 +59,11 @@ namespace Mopups.Platforms.iOS
|
|||||||
|
|
||||||
if (Equals(subview, view))
|
if (Equals(subview, view))
|
||||||
{
|
{
|
||||||
((PopupPage)Handler.VirtualView).SendBackgroundClick();
|
try
|
||||||
|
{
|
||||||
|
(Handler.VirtualView as PopupPage)?.SendBackgroundClick();
|
||||||
|
}
|
||||||
|
catch (InvalidOperationException) { }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -74,30 +78,36 @@ namespace Mopups.Platforms.iOS
|
|||||||
|
|
||||||
void UpdateSize(PopupPageRenderer handler)
|
void UpdateSize(PopupPageRenderer handler)
|
||||||
{
|
{
|
||||||
var currentElement = ((PopupPage)Handler.VirtualView);
|
try
|
||||||
|
|
||||||
if (handler.Handler.PlatformView?.Superview?.Frame == null || currentElement == null)
|
|
||||||
return;
|
|
||||||
|
|
||||||
var superviewFrame = handler.Handler.PlatformView.Superview.Frame;
|
|
||||||
var applicationFrame = UIScreen.MainScreen.ApplicationFrame;
|
|
||||||
|
|
||||||
var systemPadding = new Thickness
|
|
||||||
{
|
{
|
||||||
Left = applicationFrame.Left,
|
var currentElement = Handler.VirtualView as PopupPage;
|
||||||
Top = applicationFrame.Top,
|
|
||||||
Right = applicationFrame.Right - applicationFrame.Width - applicationFrame.Left,
|
|
||||||
Bottom = applicationFrame.Bottom - applicationFrame.Height - applicationFrame.Top + handler.KeyboardBounds.Height
|
|
||||||
};
|
|
||||||
|
|
||||||
if ((handler.Handler.VirtualView.Width != superviewFrame.Width && handler.Handler.VirtualView.Height != superviewFrame.Height)
|
if (handler.Handler.PlatformView?.Superview?.Frame == null || currentElement == null)
|
||||||
|| currentElement.SystemPadding.Bottom != systemPadding.Bottom)
|
return;
|
||||||
{
|
|
||||||
currentElement.BatchBegin();
|
var superviewFrame = handler.Handler.PlatformView.Superview.Frame;
|
||||||
currentElement.SystemPadding = systemPadding;
|
var applicationFrame = UIScreen.MainScreen.ApplicationFrame;
|
||||||
currentElement.Layout(new Rect(currentElement.X, currentElement.Y, superviewFrame.Width, superviewFrame.Height));
|
|
||||||
currentElement.BatchCommit();
|
var systemPadding = new Thickness
|
||||||
|
{
|
||||||
|
Left = applicationFrame.Left,
|
||||||
|
Top = applicationFrame.Top,
|
||||||
|
Right = applicationFrame.Right - applicationFrame.Width - applicationFrame.Left,
|
||||||
|
Bottom = applicationFrame.Bottom - applicationFrame.Height - applicationFrame.Top + handler.KeyboardBounds.Height
|
||||||
|
};
|
||||||
|
|
||||||
|
var virtualView = handler.Handler.VirtualView;
|
||||||
|
if (virtualView != null &&
|
||||||
|
((virtualView.Width != superviewFrame.Width && virtualView.Height != superviewFrame.Height)
|
||||||
|
|| currentElement.SystemPadding.Bottom != systemPadding.Bottom))
|
||||||
|
{
|
||||||
|
currentElement.BatchBegin();
|
||||||
|
currentElement.SystemPadding = systemPadding;
|
||||||
|
currentElement.Layout(new Rect(currentElement.X, currentElement.Y, superviewFrame.Width, superviewFrame.Height));
|
||||||
|
currentElement.BatchCommit();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
catch (InvalidOperationException) { }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user