Fix Button Regression with null image source (#25485) (#25611)

Co-authored-by: TJ Lambert <[email protected]>

src/Controls/src/Core/Button/Button.iOS.cs

@@ -14,10 +14,10 @@ namespace Microsoft.Maui.Controls
 {
 	public partial class Button : ICrossPlatformLayout
 	{
-		// _originalImageSize and _originalImageSource are used to ensure we don't resize the image
-		// larger than the original image size and to ensure if a new image is loaded, we use that image's size for resizing.
+		// _originalImage and _originalImageSize are used to ensure we don't resize the image larger than the original image size
+		// and to ensure if a new image is loaded, we use that image's size for resizing.
+		CGImage _originalCGImage = null;
 		CGSize _originalImageSize = CGSize.Empty;
-		string _originalImageSource = string.Empty;
 
 		/// <summary>
 		/// Measure the desired size of the button based on the image and title size taking into account
@@ -83,20 +83,19 @@ Size ICrossPlatformLayout.CrossPlatformMeasure(double widthConstraint, double he
 
 			if (image is not null)
 			{
-				// Save the original image size for later image resizing
-				if (_originalImageSize == CGSize.Empty || _originalImageSource == string.Empty || _originalImageSource != button.ImageSource.ToString())
-				{
-					_originalImageSource = button.ImageSource.ToString();
-					_originalImageSize = image.Size;
-				}
-
 				// Resize the image if necessary and then update the image variable
-				if (ResizeImageIfNecessary(platformButton, button, image, padding, spacing, borderWidth, widthConstraint, heightConstraint, _originalImageSize))
+				if (ResizeImageIfNecessary(platformButton, button, image, padding, spacing, borderWidth, widthConstraint, heightConstraint))
 				{
 					image = platformButton.CurrentImage;
 				}
 			}
 
+			else
+			{
+				_originalCGImage = null;
+				_originalImageSize = CGSize.Empty;
+			}
+
 			platformButton.ImageView.ContentMode = contentMode;
 
 			// This is used to match the behavior between platforms.
@@ -334,10 +333,16 @@ CGRect ComputeTitleRect (UIButton platformButton, Button button, UIImage image,
 		/// <param name="borderWidth"></param>
 		/// <param name="widthConstraint"></param>
 		/// <param name="heightConstraint"></param>
-		/// <param name="originalImageSize"></param>
 		/// <returns></returns>
-		static bool ResizeImageIfNecessary(UIButton platformButton, Button button, UIImage image, Thickness padding, double spacing, double borderWidth, double widthConstraint, double heightConstraint, CGSize originalImageSize)
+		bool ResizeImageIfNecessary(UIButton platformButton, Button button, UIImage image, Thickness padding, double spacing, double borderWidth, double widthConstraint, double heightConstraint)
 		{
+			// Save the original image for later image resizing
+			if (_originalImageSize == CGSize.Empty || _originalCGImage is null || image.CGImage != _originalCGImage)
+			{
+				_originalCGImage = image.CGImage;
+				_originalImageSize = image.Size;
+			}
+
 			var currentImageWidth = image.Size.Width;
 			var currentImageHeight = image.Size.Height;
 
@@ -387,15 +392,15 @@ static bool ResizeImageIfNecessary(UIButton platformButton, Button button, UIIma
 				// if the image is too large then we will size it smaller
 				if (currentImageHeight - availableHeight > buffer || currentImageWidth - availableWidth > buffer)
 				{
-					image = ResizeImageSource(image, availableWidth, availableHeight, originalImageSize);
+					image = ResizeImageSource(image, availableWidth, availableHeight, _originalImageSize);
 				}
 				// if the image is already sized down but now has more space, we will size it up no more than the original image size
 				else if (availableHeight - additionalVerticalSpace - currentImageHeight > buffer
 					&& availableWidth - additionalHorizontalSpace - currentImageWidth > buffer
-					&& currentImageHeight != originalImageSize.Height
-					&& currentImageWidth != originalImageSize.Width)
+					&& currentImageHeight != _originalImageSize.Height
+					&& currentImageWidth != _originalImageSize.Width)
 				{
-					image = ResizeImageSource(image, (nfloat)widthConstraint - additionalHorizontalSpace, (nfloat)heightConstraint - additionalVerticalSpace, originalImageSize, true);
+					image = ResizeImageSource(image, (nfloat)widthConstraint - additionalHorizontalSpace, (nfloat)heightConstraint - additionalVerticalSpace, _originalImageSize, true);
 				}
 				else
 				{
@@ -465,5 +470,12 @@ internal static void MapBorderWidth(IButtonHandler handler, Button button)
 		{
 			handler.PlatformView?.UpdateContentLayout(button);
 		}
+
+		private protected override void OnHandlerChangingCore(HandlerChangingEventArgs args)
+		{
+			base.OnHandlerChangingCore(args);
+			_originalImageSize = CGSize.Empty;
+			_originalCGImage = null;
+		}
 	}
 }

src/Controls/tests/TestCases.HostApp/Issues/Issue25409.xaml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
+             xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+             x:Class="Maui.Controls.Sample.Issues.Issue25409"
+             Title="Issue25409">
+    <Button
+        x:Name="button1"
+        AutomationId="button1"
+        Text="Click me"
+        Clicked="OnCounterClicked" />
+</ContentPage>

src/Controls/tests/TestCases.HostApp/Issues/Issue25409.xaml.cs

@@ -0,0 +1,54 @@
+namespace Maui.Controls.Sample.Issues;
+
+[Issue(IssueTracker.Github, 25409, "UIButton CurrentImage can be set without crashing", PlatformAffected.iOS)]
+public partial class Issue25409 : ContentPage
+{
+	public Issue25409()
+	{
+		InitializeComponent();
+	}
+
+#if IOS
+	int count = 0;
+#endif
+
+	private void OnCounterClicked(object sender, EventArgs e)
+	{
+#if IOS
+		((Action)(async () =>
+		{
+			if (count % 2 == 0)
+			{
+				var imageSource = new FileImageSource() { File = "shopping_cart.png" };
+
+				if (button1.Handler?.MauiContext is not null && (button1.Handler.PlatformView is UIKit.UIButton platformButton))
+				{
+					var imageResult = await imageSource.GetPlatformImageAsync(button1.Handler.MauiContext);
+					if (imageResult is not null)
+					{
+						platformButton.SetImage(imageResult.Value, UIKit.UIControlState.Normal);
+					}
+				}
+
+				// since changing the UIButton.CurrentImage with SetImage does not trigger a layout pass,
+				// we can change something else to force a layout pass
+				button1.Text = string.Empty;
+			}
+
+			else
+			{
+				if (button1.Handler?.PlatformView is UIKit.UIButton platformButton)
+				{
+					platformButton.SetImage(null, UIKit.UIControlState.Normal);
+				}
+
+				// since changing the UIButton.CurrentImage with SetImage does not trigger a layout pass,
+				// we can change something else to force a layout pass
+				button1.Text = "Trigger Layout!";
+			}
+
+			count++;
+		}))();
+#endif
+	}
+}

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue25409.cs

@@ -0,0 +1,28 @@
+#if IOS
+using NUnit.Framework;
+using UITest.Appium;
+using UITest.Core;
+
+namespace Microsoft.Maui.TestCases.Tests.Issues;
+
+public class Issue25409 : _IssuesUITest
+{
+	public Issue25409(TestDevice testDevice) : base(testDevice)
+	{
+	}
+
+	public override string Issue => "UIButton CurrentImage can be set without crashing";
+
+	[Test]
+	[Category(UITestCategories.Button)]
+	public void CurrentImageSet()
+	{
+		App.WaitForElement("button1");
+		App.Tap("button1");
+		App.WaitForElement("button1");
+		App.Tap("button1");
+		App.WaitForElement("button1");
+		App.Tap("button1");
+	}
+}
+#endif