From cfd65240317e80a711b20683cf7f7284ab0b334e Mon Sep 17 00:00:00 2001 From: donandren Date: Tue, 12 Jul 2016 01:56:20 +0300 Subject: [PATCH 1/4] added failing unit test for issue #589 and #591 --- .../ListBoxTests.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index 7e1347daf5..65d2e74d54 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -234,6 +234,43 @@ namespace Avalonia.Controls.UnitTests target.Arrange(new Rect(0, 0, 100, 100)); } + [Fact] + public void ListBox_Set_SelectedItem_Should_Not_Crash_bug589_bug591() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var items = new[] { "Foo", "Bar", "Baz " }; + var target = new ListBox + { + Template = ListBoxTemplate() + }; + + Prepare(target); + + //emulate control is not in foreground or not visible + //also control can be child of parent with size 0,0 + //that's real case scenario although it looks bit unreal + target.Measure(new Size(0, 0)); + target.Arrange(new Rect(0, 0, 0, 0)); + + target.Items = items; + + //bug #591 in ItemVirtualizerSimple.cs:line 471 + //var container = generator.ContainerFromIndex(index); <- here container is null + //and NullReferenceException will be raised if LayoutManager is not null + //line 482 + //if (!new Rect(panel.Bounds.Size).Contains(container.Bounds)) <- this check will fail + + //bug #589 - currently this unit test is failing with + //IndexOutOfRangeException + // in CreateAndRemoveContainers() ItemVirtualizerSimple.cs:line 274 + //generator.Materialize(index, Items.ElementAt(index), memberSelector); <- index is -1 + target.SelectedItem = items.First(); + + Assert.Equal(items.First(), target.SelectedItem); + } + } + private class Item { public Item(string value) From dd728dae5b76aab80f12917b1a862f7da8ac3666 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 21 Jul 2016 14:34:13 -0600 Subject: [PATCH 2/4] Fix measuring to infinity when scrolled to end. When a virtualized list was scrolled to the bottom and then the list was measured with a size larger than needed to fit all items (in this case we use infinity) then the virtualizer tries to go backwards to add items at the top of the currently visible items by setting `step = -1`; however it didn't check whether the current index was < 0. Fixes #589. --- .../Presenters/ItemVirtualizerSimple.cs | 2 +- ...emsPresenterTests_Virtualization_Simple.cs | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs index a54e502033..bea9cb8a1a 100644 --- a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs +++ b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs @@ -252,7 +252,7 @@ namespace Avalonia.Controls.Presenters var index = NextIndex; var step = 1; - while (!panel.IsFull) + while (!panel.IsFull && index >= 0) { if (index >= ItemCount) { diff --git a/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs b/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs index 34b074d185..264c2b2e89 100644 --- a/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs +++ b/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs @@ -326,6 +326,27 @@ namespace Avalonia.Controls.UnitTests.Presenters Assert.Equal(expected, actual); } + [Fact] + public void Measuring_To_Infinity_When_Scrolled_To_End_Should_Not_Throw() + { + var target = CreateTarget(useAvaloniaList: true); + + target.ApplyTemplate(); + target.Measure(new Size(100, 100)); + target.Arrange(new Rect(0, 0, 100, 100)); + + ((ILogicalScrollable)target).Offset = new Vector(0, 10); + + // Check for issue #589: this should not throw. + target.Measure(Size.Infinity); + + var expected = Enumerable.Range(0, 20).Select(x => $"Item {x}").ToList(); + var items = (AvaloniaList)target.Items; + var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); + + Assert.Equal(expected, actual); + } + [Fact] public void Replacing_Items_Should_Update_Containers() { From 1c88b3bd85d582b8883fa63b536620e3e8bcad4c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 21 Jul 2016 14:40:51 -0600 Subject: [PATCH 3/4] Fix scrolling to item when size == 0,0 When virtualized presenter size == 0,0 no containers will be materialized so no container will be found. Fixes #591. --- .../Presenters/ItemVirtualizerSimple.cs | 2 +- ...ItemsPresenterTests_Virtualization_Simple.cs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs index bea9cb8a1a..00d896925a 100644 --- a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs +++ b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs @@ -475,7 +475,7 @@ namespace Avalonia.Controls.Presenters // is only partially visible due to differing item sizes. If the container is only // partially visible, scroll again. Don't do this if there's no layout manager: // it means we're running a unit test. - if (layoutManager != null) + if (container != null && layoutManager != null) { layoutManager.ExecuteLayoutPass(); diff --git a/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs b/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs index 264c2b2e89..3ab5a928b4 100644 --- a/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs +++ b/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs @@ -505,6 +505,23 @@ namespace Avalonia.Controls.UnitTests.Presenters Assert.Equal(0, ((IVirtualizingPanel)target.Panel).PixelOffset); } + [Fact] + public void Scrolling_To_Item_In_Zero_Sized_Presenter_Doesnt_Throw() + { + using (UnitTestApplication.Start(TestServices.RealLayoutManager)) + { + var target = CreateTarget(itemCount: 10); + var items = (IList)target.Items; + + target.ApplyTemplate(); + target.Measure(Size.Empty); + target.Arrange(Rect.Empty); + + // Check for issue #591: this should not throw. + target.ScrollIntoView(items[0]); + } + } + public class Vertical { [Fact] From 6eaa5b74dc575987dc7fffde1e49fac4cb37bc6b Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 21 Jul 2016 14:42:14 -0600 Subject: [PATCH 4/4] Test has been refactored into 2 tests. Removed test that has been refactored into 2 separate tests. Thanks @donandren for finding those bugs! --- .../ListBoxTests.cs | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index 65d2e74d54..7e1347daf5 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -234,43 +234,6 @@ namespace Avalonia.Controls.UnitTests target.Arrange(new Rect(0, 0, 100, 100)); } - [Fact] - public void ListBox_Set_SelectedItem_Should_Not_Crash_bug589_bug591() - { - using (UnitTestApplication.Start(TestServices.StyledWindow)) - { - var items = new[] { "Foo", "Bar", "Baz " }; - var target = new ListBox - { - Template = ListBoxTemplate() - }; - - Prepare(target); - - //emulate control is not in foreground or not visible - //also control can be child of parent with size 0,0 - //that's real case scenario although it looks bit unreal - target.Measure(new Size(0, 0)); - target.Arrange(new Rect(0, 0, 0, 0)); - - target.Items = items; - - //bug #591 in ItemVirtualizerSimple.cs:line 471 - //var container = generator.ContainerFromIndex(index); <- here container is null - //and NullReferenceException will be raised if LayoutManager is not null - //line 482 - //if (!new Rect(panel.Bounds.Size).Contains(container.Bounds)) <- this check will fail - - //bug #589 - currently this unit test is failing with - //IndexOutOfRangeException - // in CreateAndRemoveContainers() ItemVirtualizerSimple.cs:line 274 - //generator.Materialize(index, Items.ElementAt(index), memberSelector); <- index is -1 - target.SelectedItem = items.First(); - - Assert.Equal(items.First(), target.SelectedItem); - } - } - private class Item { public Item(string value)