-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[iOS] Fixed the Application crash when ToolbarItem is created with invalid IconImageSource name #27175
Conversation
Hey there @Ahamed-Ali! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
/rebase |
94a6964
to
b20c542
Compare
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
@@ -1,5 +1,4 @@ | |||
#if TEST_FAILS_ON_IOS //Application crashes when ToolbarItem is created with invalid IconImageSource name.Issue Link: https://github.com/dotnet/maui/issues/27095 | |||
using NUnit.Framework; | |||
using NUnit.Framework; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens on other platforms? Based on the test, guess not throwing an exception, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An exception is not thrown on other platforms when an invalid IconImageSource name is provided @jsuarezruiz
@@ -41,7 +41,7 @@ public partial class FileImageSourceService | |||
{ | |||
Logger?.LogWarning(ex, "Unable to load image file '{File}'.", imageSource.File); | |||
|
|||
throw; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattleibow I understand that throwing the exception at this point was a expected behavior, right?
@@ -41,7 +41,7 @@ public partial class FileImageSourceService | |||
{ | |||
Logger?.LogWarning(ex, "Unable to load image file '{File}'.", imageSource.File); | |||
|
|||
throw; | |||
return FromResult(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on https://github.com/dotnet/maui/pull/27175/files#r1939317042
This align the behavior between platforms. LGTM.
Root Cause of the issue
FileImageSourceService.iOS
when the image is null, causing the application to crash if an invalid IconImageSource is provided.Description of Change
Reference
https://github.com/xamarin/Xamarin.Forms/blob/5.0.0/Xamarin.Forms.Platform.MacOS/ImageSourceHandlers.cs#L19
Issues Fixed
Fixes #27095
Tested the behaviour in the following platforms
Screenshot
WithCrash.mov
FixedWithouCrash.mov