Update behavior for ComboBox Items property is not consistent

[GingerAvalanche]

Describe the bug Half the time: Setting ComboBox Items does not update SelectedItem (and does not call SelectedItem setters) Other half the time: Setting ComboBox Items updates SelectedItem to null (and calls SelectedItem setters with null)

To Reproduce Steps to reproduce the behavior:

1. Make 2 ComboBox elements in XAML (c1, c2)
2. Tie them to 4 properties in the code-behind (string[] P1, string P2, string[] P3, string P4)
3. Tie those 4 properties to 4 fields (string[] f1, string f2, string[] f3, string f4)
4. Make the setters call the next in line (e.g. P1 { get => f1; set => P2 = this.RaiseAndSetIfChanged(ref f1, value).First(); } )
5. Run/test the program and set c1 to some value other than the first one, then do it again.
6. Observe c2 on the first and second time. The first time, it should not update. The second time, it should be empty.

Expected behavior ComboBoxes should behave the same way all the time. (Either invoke the dependent setters or not, either way)

Desktop (please complete the following information):

• OS: Windows
• Version 10 (unless you mean AvaloniaUI, in which case 0.10.14)

Additional context This is a rather difficult one to set up, due to the cascading nature of the properties. I originally intended to just use events, but I read that using events goes against the design philosophy of AvaloniaUI.

I've logged this problem to kingdom come as I'm having it with my own code, and here's the output. My XAML elements are combo boxes, where the contents of one box rely on the selected item of the previous box, so my properties alternate between SortedSet<string> and string, but the principle is the same. You can see that on every odd-numbered call to Raise, it functions fine, but on every even-numbered call, it calls the next setter in line 3 times(!). This is evident because the second and ninth (last) usages of setter 3 show Demo003_0.msbt, but the second usage is broken while the last usage works fine.

---Setter 3
value.Count: 33
value.First(): ArmorHead.msbt
---Still in Setter 3, now calling RaiseAndSetIfChanged
---Back in Setter 3, RaiseAndSetIfChanged has concluded
First() on Raise's return value: ArmorHead.msbt
---Now assigning previously logged value with Setter 4
---Setter 4
value: ArmorHead.msbt

---Setter 3
value.Count: 163
value.First(): Demo003_0.msbt
---Still in Setter 3, now calling RaiseAndSetIfChanged
---Setter 4
value: 
---Setter 4
value: Demo003_0.msbt
---Setter 4
value: 
---Back in Setter 3, RaiseAndSetIfChanged has concluded
First() on Raise's return value: Demo003_0.msbt
---Now assigning previously logged value with Setter 4
---Setter 4
value: Demo003_0.msbt

---Setter 3
value.Count: 640
value.First(): 100enemy.msbt
---Still in Setter 3, now calling RaiseAndSetIfChanged
---Back in Setter 3, RaiseAndSetIfChanged has concluded
First() on Raise's return value: 100enemy.msbt
---Now assigning previously logged value with Setter 4
---Setter 4
value: 100enemy.msbt

---Setter 3
value.Count: 55
value.First(): AmiiboWindow_00.msbt
---Still in Setter 3, now calling RaiseAndSetIfChanged
---Setter 4
value: 
---Setter 4
value: AmiiboWindow_00.msbt
---Setter 4
value: 
---Back in Setter 3, RaiseAndSetIfChanged has concluded
First() on Raise's return value: AmiiboWindow_00.msbt
---Now assigning previously logged value with Setter 4
---Setter 4
value: AmiiboWindow_00.msbt

---Setter 3
value.Count: 169
value.First(): QL_100enemy.msbt
---Still in Setter 3, now calling RaiseAndSetIfChanged
---Back in Setter 3, RaiseAndSetIfChanged has concluded
First() on Raise's return value: QL_100enemy.msbt
---Now assigning previously logged value with Setter 4
---Setter 4
value: QL_100enemy.msbt

---Setter 3
value.Count: 9
value.First(): Shout_Npc_Darukeru.msbt
---Still in Setter 3, now calling RaiseAndSetIfChanged
---Setter 4
value: 
---Setter 4
value: Shout_Npc_Darukeru.msbt
---Setter 4
value: 
---Back in Setter 3, RaiseAndSetIfChanged has concluded
First() on Raise's return value: Shout_Npc_Darukeru.msbt
---Now assigning previously logged value with Setter 4
---Setter 4
value: Shout_Npc_Darukeru.msbt

---Setter 3
value.Count: 13
value.First(): CharExtraction.msbt
---Still in Setter 3, now calling RaiseAndSetIfChanged
---Back in Setter 3, RaiseAndSetIfChanged has concluded
First() on Raise's return value: CharExtraction.msbt
---Now assigning previously logged value with Setter 4
---Setter 4
value: CharExtraction.msbt

---Setter 3
value.Count: 9
value.First(): TipsChallenge.msbt
---Still in Setter 3, now calling RaiseAndSetIfChanged
---Setter 4
value: 
---Setter 4
value: TipsChallenge.msbt
---Setter 4
value: 
---Back in Setter 3, RaiseAndSetIfChanged has concluded
First() on Raise's return value: TipsChallenge.msbt
---Now assigning previously logged value with Setter 4
---Setter 4
value: TipsChallenge.msbt

---Setter 3
value.Count: 163
value.First(): Demo003_0.msbt
---Still in Setter 3, now calling RaiseAndSetIfChanged
---Back in Setter 3, RaiseAndSetIfChanged has concluded
First() on Raise's return value: Demo003_0.msbt
---Now assigning previously logged value with Setter 4
---Setter 4
value: Demo003_0.msbt

[timunie]

Can you setup a small sample App and upload it to GitHub? Then anyone can have a look theirself.

Also this issue may be related to Reactive UI and not Avalonia

[GingerAvalanche]

https://github.com/GingerAvalanche/ComboBoxTest

Can add debug lines if necessary, but it's easily reproducible by selecting a value from the top box, then selecting a value from the second box. It'll set the SelectedItem for the third box to null, and when trying to read that value to create the contents of the fourth box, it'll throw. When you select a value for the first box, only the setter calls in the project code are fired, so no nulls are ever passed.

Note that the XAML designer behaves as the code expects all of the time, not just the first time. It has to be built and run to see the error.

[timunie]

If I have some free time next week I'll have a look if I have a solution for this. Thanks for the sample 👍

[GingerAvalanche]

Having done some more research on the issue, this might be related to #4048 in that my new Items don't have the old SelectedItem, so the SelectedIndex is getting set to -1 (and since they're strings, the value at -1 is presumably null). I'm assuming that SelectedItem is basically shorthand for Items[SelectedIndex], which would cause the next setter in line to be called with the null value, causing it to throw when that null is used for something.

There's 2 weird things about this issue:

1. That the null set happens only half the time (from reading up on #4048 and related issues, it seems like it should be set to null all the time when the original value at SelectedIndex is not available in the new Items)
2. That, when it is set, it is set three times, instead of once (and one of those times, it is set to the proper value that my code is attempting to set it to at the end. Maybe it sets SelectedIndex to -1, then to 0, then back to -1?)

I think the correct behavior, at least in keeping with what's been mentioned in previous issues, would probably be for the cascaded values to be set to null all the time (unless the SelectedItem is also available in the new Items). Though this would be disappointing, because it fully breaks what I'm trying to do.

Other options:

• Reset SelectedIndex to 0 if Items.Length > 0. This ensures that the ComboBox text is always set to a valid item if one exists.
• Not fire NotifyPropertyChanged (and others) events for that reset, letting the end user decide what to do with the changed property (including firing the event, themselves, if they want).

[timunie]

Okay, so I had a look at your sample. For me it looks like you are switching the items source of the combo box. While doing so the selected item gets null as it was not found in the list, which is OK imo.

For me your sample seems to work if I add a null check before updating the lists. See https://github.com/GingerAvalanche/ComboBoxTest/pull/1

Happy coding Tim

[GingerAvalanche]

Thanks for looking into it for me! :)

Adding a null check is the solution that I had hit upon with my own code. I left this issue up, though, because the underlying error is not fixed, only the symptom is. The code is still non-deterministic.

It should throw on the first box selection you make, and on the second selection, it should throw once, not twice. Neither of those things is the case.