From a61ce488da1e33989bd2913881be3d0dd30d49ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Sua=CC=81rez=20Ruiz?= Date: Mon, 18 Oct 2021 15:56:24 +0200 Subject: [PATCH 1/3] Add repro sample --- .../Issue6387.xaml | 14 +++++ .../Issue6387.xaml.cs | 59 +++++++++++++++++++ ...rin.Forms.Controls.Issues.Shared.projitems | 6 +- 3 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml.cs diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml new file mode 100644 index 00000000000..9acbb32a293 --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml @@ -0,0 +1,14 @@ + + + + + + + \ No newline at end of file diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml.cs new file mode 100644 index 00000000000..f5823dc71f9 --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml.cs @@ -0,0 +1,59 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; +using System.Reflection; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +using Xamarin.Forms.Core.UITests; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ +#if UITEST + [Category(UITestCategories.ToolbarItem)] +#endif + [Preserve(AllMembers = true)] + [Issue(IssueTracker.Github, 6387, + "[Bug] [iOS] Crash When the First Toolbar Item Has an Icon and a Second Item Gets Added", + PlatformAffected.iOS)] + public partial class Issue6387 : TestContentPage + { + readonly ToolbarItem _item0; + readonly ToolbarItem _item1; + + public Issue6387() + { +#if APP + InitializeComponent(); + + _item0 = new ToolbarItem("Item 0", null, ClearAndAddToolbarItems) + { + IconImageSource = ImageSource.FromResource("Xamarin.Forms.Controls.GalleryPages.crimson.jpg", typeof(Issue6387).GetTypeInfo().Assembly) + }; + _item1 = new ToolbarItem("Item 1", null, ClearAndAddToolbarItems) + { + // It doesn't matter if this item has an image or not. + }; + + ClearAndAddToolbarItems(); +#endif + } + + protected override void Init() + { + + } + + void ClearAndAddToolbarItems() + { + ToolbarItems.Clear(); + + ToolbarItems.Add(_item0); + ToolbarItems.Add(_item1); + } + } +} \ No newline at end of file diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index 37c1424198e..423d34104c0 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -1796,6 +1796,7 @@ + @@ -2279,6 +2280,9 @@ MSBuild:UpdateDesignTimeXaml + + MSBuild:UpdateDesignTimeXaml + @@ -2799,7 +2803,7 @@ Designer - MSBuild:Compile + MSBuild:UpdateDesignTimeXaml Designer From 1e8668953ba2aafffcd93424b44e71ff0856445d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Sua=CC=81rez=20Ruiz?= Date: Mon, 18 Oct 2021 16:11:30 +0200 Subject: [PATCH 2/3] Fixed the issue --- .../Issue6387.xaml | 5 +-- .../Extensions/ToolbarItemExtensions.cs | 33 ++++++++++++++----- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml index 9acbb32a293..c0d618e903f 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml @@ -7,8 +7,9 @@ Title="Issue 6387"> - \ No newline at end of file diff --git a/Xamarin.Forms.Platform.iOS/Extensions/ToolbarItemExtensions.cs b/Xamarin.Forms.Platform.iOS/Extensions/ToolbarItemExtensions.cs index 29d919138a8..d92a97f2b41 100644 --- a/Xamarin.Forms.Platform.iOS/Extensions/ToolbarItemExtensions.cs +++ b/Xamarin.Forms.Platform.iOS/Extensions/ToolbarItemExtensions.cs @@ -1,4 +1,5 @@ using System.ComponentModel; +using System.Threading.Tasks; using CoreGraphics; using UIKit; using PointF = CoreGraphics.CGPoint; @@ -23,6 +24,7 @@ public static UIBarButtonItem ToUIBarButtonItem(this ToolbarItem item, bool forc sealed class PrimaryToolbarItem : UIBarButtonItem { + bool _disposed; readonly bool _forceName; readonly ToolbarItem _item; @@ -32,7 +34,7 @@ public PrimaryToolbarItem(ToolbarItem item, bool forceName) _item = item; if (item.IconImageSource != null && !item.IconImageSource.IsEmpty && !forceName) - UpdateIconAndStyle(); + Device.BeginInvokeOnMainThread(async () => { await UpdateIconAndStyleAsync(); }); else UpdateTextAndStyle(); UpdateIsEnabled(); @@ -50,11 +52,15 @@ public PrimaryToolbarItem(ToolbarItem item, bool forceName) protected override void Dispose(bool disposing) { if (disposing) + { _item.PropertyChanged -= OnPropertyChanged; + _disposed = true; + } + base.Dispose(disposing); } - void OnPropertyChanged(object sender, PropertyChangedEventArgs e) + async void OnPropertyChanged(object sender, PropertyChangedEventArgs e) { if (e.PropertyName == MenuItem.IsEnabledProperty.PropertyName) UpdateIsEnabled(); @@ -68,7 +74,7 @@ void OnPropertyChanged(object sender, PropertyChangedEventArgs e) if (!_forceName) { if (_item.IconImageSource != null && !_item.IconImageSource.IsEmpty) - UpdateIconAndStyle(); + await UpdateIconAndStyleAsync(); else UpdateTextAndStyle(); } @@ -79,8 +85,11 @@ void OnPropertyChanged(object sender, PropertyChangedEventArgs e) this.SetAccessibilityLabel(_item); } - async void UpdateIconAndStyle() + async Task UpdateIconAndStyleAsync() { + if (_disposed) + return; + Image = await _item.IconImageSource.GetNativeImageAsync(); Style = UIBarButtonItemStyle.Plain; } @@ -100,13 +109,14 @@ void UpdateTextAndStyle() sealed class SecondaryToolbarItem : UIBarButtonItem { + bool _disposed; readonly ToolbarItem _item; public SecondaryToolbarItem(ToolbarItem item) : base(new SecondaryToolbarItemContent()) { _item = item; UpdateText(); - UpdateIcon(); + Device.BeginInvokeOnMainThread(async () => { await UpdateIconAsync(); }); UpdateIsEnabled(); ((SecondaryToolbarItemContent)CustomView).TouchUpInside += (sender, e) => ((IMenuItemController)_item).Activate(); @@ -122,16 +132,20 @@ public SecondaryToolbarItem(ToolbarItem item) : base(new SecondaryToolbarItemCon protected override void Dispose(bool disposing) { if (disposing) + { _item.PropertyChanged -= OnPropertyChanged; + _disposed = true; + } + base.Dispose(disposing); } - void OnPropertyChanged(object sender, PropertyChangedEventArgs e) + async void OnPropertyChanged(object sender, PropertyChangedEventArgs e) { if (e.PropertyName == MenuItem.TextProperty.PropertyName) UpdateText(); else if (e.PropertyName == MenuItem.IconImageSourceProperty.PropertyName) - UpdateIcon(); + await UpdateIconAsync(); else if (e.PropertyName == MenuItem.IsEnabledProperty.PropertyName) UpdateIsEnabled(); else if (e.PropertyName == AutomationProperties.HelpTextProperty.PropertyName) @@ -140,8 +154,11 @@ void OnPropertyChanged(object sender, PropertyChangedEventArgs e) this.SetAccessibilityLabel(_item); } - async void UpdateIcon() + async Task UpdateIconAsync() { + if (_disposed) + return; + UIImage image = null; if (_item.IconImageSource != null && !_item.IconImageSource.IsEmpty) { From f05fff694a894478c764bfa36bfa2e1f5cd86c57 Mon Sep 17 00:00:00 2001 From: Gerald Versluis Date: Tue, 19 Oct 2021 10:55:49 +0200 Subject: [PATCH 3/3] Fix build error --- .../Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml.cs index f5823dc71f9..538f43ac492 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml.cs +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue6387.xaml.cs @@ -22,9 +22,10 @@ namespace Xamarin.Forms.Controls.Issues PlatformAffected.iOS)] public partial class Issue6387 : TestContentPage { +#if APP readonly ToolbarItem _item0; readonly ToolbarItem _item1; - +#endif public Issue6387() { #if APP @@ -47,7 +48,7 @@ protected override void Init() { } - +#if APP void ClearAndAddToolbarItems() { ToolbarItems.Clear(); @@ -55,5 +56,6 @@ void ClearAndAddToolbarItems() ToolbarItems.Add(_item0); ToolbarItems.Add(_item1); } +#endif } } \ No newline at end of file