From ad2522354a29c5005d1978684ee8514d5553df8a Mon Sep 17 00:00:00 2001 From: Samantha Houts Date: Wed, 20 Jun 2018 14:14:26 -0700 Subject: [PATCH] [iOS] RecycleElement ListView should not crash when bound to ObservableCollection (ArgumentNullException & ArgumentException) (#3042) fixes #1900 fixes #3026 * [iOS] Default to TextCell in GetPrototypicalCell fixes #3026 * [iOS] Don't throw exception when Index doesn't match in RecycleElement fixes #1900 * Add repros for #1342, #1900, #1927 * errant space * Remove repros for #1342 and #1927 (tbc in another branch) * Update Issue1900.cs * Revert exception type did not mean to change that! yay for ui tests * Update Issue1900.cs --- .../Issue1900.cs | 56 +++++++++++++++++++ ...rin.Forms.Controls.Issues.Shared.projitems | 1 + .../Renderers/ListViewRenderer.cs | 17 +++--- 3 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1900.cs diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1900.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1900.cs new file mode 100644 index 000000000..79f09d4bc --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue1900.cs @@ -0,0 +1,56 @@ +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Linq; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +using Xamarin.Forms.Core.UITests; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ +#if UITEST + [Category(UITestCategories.ListView)] +#endif + [Preserve(AllMembers = true)] + [Issue(IssueTracker.Github, 1900, "Xamarin ios ListView ObservableCollection. Collection.Add() throwing 'Index # is greater than the number of rows #' exception", PlatformAffected.iOS)] + public class Issue1900 : TestContentPage + { + public ObservableCollection Items { get; set; } = new ObservableCollection(Enumerable.Range(0, 25).Select(i => $"Initial {i}")); + + public void AddItemsToList(IEnumerable items) + { + foreach (var item in items) + { + Items.Add(item); + } + } + + protected override void Init() + { + var listView = new ListView(ListViewCachingStrategy.RecycleElement) { AutomationId = "ListView", ItemsSource = Items }; + listView.ItemAppearing += ItemList_ItemAppearing; + Content = new StackLayout { Children = { new Label { Text = "If this test crashes when it loads or when you scroll the list, then this test has failed. Obviously." }, listView } }; + } + + void ItemList_ItemAppearing(object sender, ItemVisibilityEventArgs e) + { + if (e.Item.ToString() == Items.Last()) + { + AddItemsToList(Enumerable.Range(0, 10).Select(i => $"Item {i}")); + } + } + + +#if UITEST + [Test] + public void Issue1900Test () + { + RunningApp.WaitForElement (q => q.Marked ("ListView")); + } +#endif + } +} 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 98c4568ed..cf8d8e49f 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 @@ -452,6 +452,7 @@ + diff --git a/Xamarin.Forms.Platform.iOS/Renderers/ListViewRenderer.cs b/Xamarin.Forms.Platform.iOS/Renderers/ListViewRenderer.cs index 58302314f..b7fe073c5 100644 --- a/Xamarin.Forms.Platform.iOS/Renderers/ListViewRenderer.cs +++ b/Xamarin.Forms.Platform.iOS/Renderers/ListViewRenderer.cs @@ -532,7 +532,9 @@ namespace Xamarin.Forms.Platform.iOS var groupReset = resetWhenGrouped && Element.IsGroupingEnabled; - if (!groupReset) + // We can't do this check on grouped lists because the index doesn't match the number of rows in a section. + // Likewise, we can't do this check on lists using RecycleElement because the number of rows in a section will remain constant because they are reused. + if (!groupReset && Element.CachingStrategy == ListViewCachingStrategy.RetainElement) { var lastIndex = Control.NumberOfRowsInSection(section); if (e.NewStartingIndex > lastIndex || e.OldStartingIndex > lastIndex) @@ -752,9 +754,10 @@ namespace Xamarin.Forms.Platform.iOS else // ListViewCachingStrategy.RetainElement return GetCellForPath(indexPath); + if (itemTypeOrDataTemplate == null) + itemTypeOrDataTemplate = typeof(TextCell); - Cell protoCell; - if (!_prototypicalCellByTypeOrDataTemplate.TryGetValue(itemTypeOrDataTemplate, out protoCell)) + if (!_prototypicalCellByTypeOrDataTemplate.TryGetValue(itemTypeOrDataTemplate, out Cell protoCell)) { // cache prototypical cell by item type; Items of the same Type share // the same DataTemplate (this is enforced by RecycleElementAndDataTemplate) @@ -988,7 +991,7 @@ namespace Xamarin.Forms.Platform.iOS var renderer = (CellRenderer)Internals.Registrar.Registered.GetHandlerForObject(cell); view = new HeaderWrapperView { Cell = cell }; view.AddSubview(renderer.GetCell(cell, null, tableView)); - + return view; } @@ -1115,8 +1118,8 @@ namespace Xamarin.Forms.Platform.iOS if (_isDragging && scrollView.ContentOffset.Y < -10f && _uiTableViewController._usingLargeTitles && Device.Info.CurrentOrientation.IsPortrait()) { - _uiTableViewController.ForceRefreshing(); - } + _uiTableViewController.ForceRefreshing(); + } } public override string[] SectionIndexTitles(UITableView tableView) @@ -1350,7 +1353,7 @@ namespace Xamarin.Forms.Platform.iOS { if (RefreshControl == null) return; - + _refresh.EndRefreshing(); UpdateContentOffset(-1);