Picker.cs Line 215 Throws Argument Out Of Range Exception when Binding Context gets set to Null

[t4lm1d]

Description

In Picker.cs, when we close a popup dialog that contains a picker control inside the "SelectedIndexChanged" event, the Items collection has 0 elements and a SelectedIndex is still set. (This can occur when some portion of the code sets the BindingContext to null)

Under these conditions, we get an argument out of range exception within the UpdateSelectedItem() method on line 215:

void UpdateSelectedItem()
{
	if (SelectedIndex == -1) {
		SelectedItem = null;
		return;
	}
	if (ItemsSource != null) {
		SelectedItem = ItemsSource [SelectedIndex];
		return;
	}
	SelectedItem = Items [SelectedIndex]; //Line 215
}

Steps to Reproduce

1. Set up a Picker control in a popup dialog
2. In the SelectedIndexChanged event, set the BindingContext to null when the last element in the picker is chosen
3. In the SelectedIndexChanged event close the popup when the last element in the Picker is chosen
4. Run the code
5. Select the last element
6. Index out of bounds exception on line 215 of Picker.cs

Expected Behavior

I would expect there to be a check on line 215 of the Picker.cs that does not allow choosing an item by index value when there aren't any elements in the list.

Actual Behavior

Throws exception, app bombs

Reproductions

Picker picker = new Picker();
			picker.ItemsSource = new List<string> { "cat", "mouse", "rabbit" };
			picker.SelectedIndexChanged += (_, __) =>
			{
				picker.ItemsSource = null;
			};

			return new ContentPage()
			{
				Content = picker
			};

https://github.com/xamarin/Xamarin.Forms/files/2036809/ExceptionExample.zip

[samhouts]

Is there any chance you could provide a small reproduction project to demonstrate this issue? I want to make sure I've got the structure right to reproduce this. Thank you!

[BradChase2011]

I have rewritten a bunch of the Picker at this point. This bug and one other where the SelectedIndex is being set to -1 when the BindingContext and ItemsSource were being reset/bound, were addressed. I can put up a PR when I get time, unless someone has already started work on either of these.

The issue with this and the other is the syncing of Items, ItemsSource, SelectedItem, and SelectedIndex and trying to figure out what should be set when and what comes first. The way I solved it is to let the developer be in charge of the SelectedItem and dont try to guess what the developer wanted.

[samhouts]

@BradChase2011 Please do! Thanks!

[t4lm1d]

@samhouts Do you still need a small project with the ability to reproduce?

[samhouts]

@t4lm1d It would certainly be helpful, if only to confirm that @BradChase2011's fix actually applies to your issue.

[BradChase2011]

As a side note, it happens on iOS, Droid, UWP, and WPF. It happens when you have ItemsSource and SelectedItem bound. We found it when we use the same controls bound to an object (model) that when you switch out the parent's BindingContext's model it would cause the Pickers to go haywire resetting the SelectedIndex to -1 and then trying to set it back again to the SelectedItem only to find the Items list hasnt been synced yet, and even if it is it wont matter much because everything has lost sync at this point. The fix actually involves code in the Picker as well as the PickerRenderer's in each platform.

Side note, is there an enhancement open for SelectedValue and DisplayMemberPath/SelectedValuePath? This might be a good time to pack it all into one PR.

[t4lm1d]

@samhouts Just letting you know I haven't forgotten. I will get you the example as soon as I have a moment. Other work has taken my efforts away from this at the moment. Thanks! Eric

[DennisWelu]

PickerBindingContextNull.zip

I've attached a test project that illustrates a problem I've encountered that looks like it's in the same neighborhood as the problem description and comments here. I'm testing with UWP on XF3. When the BindingContext is set to null and the Picker has an ItemsSource set, then the binding still fires back a change to the source. All the relevant code in the attachment is in the MainPage.xaml/cs. The binding does not fire for other control types bound to the same data, such as the Entry in this example (commented out by default in the XAML).

[DennisWelu]

A workaround that works in my situation, where the BindingContext going to null means I'm done with the page, is this:

        /// <summary>
        /// Works around this BUG: https://github.com/xamarin/Xamarin.Forms/issues/2499
        /// </summary>
        void Picker_OnPropertyChanging(object sender, Xamarin.Forms.PropertyChangingEventArgs e)
        {
            if (e.PropertyName == nameof(Picker.ItemsSource))
            {
                var picker = (Picker) sender;
                if (picker.BindingContext == null)
                    picker.RemoveBinding(Picker.SelectedItemProperty);
            }
        }

While that might not work for everyone maybe it gives an idea or two for a workaround.

In figuring this out I noticed in the call stack that the binding context change to Null triggers the OnItemsCollectionChanged to set the SelectedIndex to -1 and calls UpdateSelectedItem, which sets SelectedItem to null. That all seems reasonable except that the SelectedItem binding is still applying the null back to the bound source...which I'm not sure why (since context should be null).

[PureWeen]

@DennisWelu I think the issue you pointed out causes this issue to manifest but I think they are still two different issues. Can you create a new issue for the behavior you noticed?