infinite loop when assigning the Thumb of a Track

[trebahl]

Description

The Track class has a publically settable Thumb property, but I get an infinite loop when I try and set it after the template has been applied.

Reproduction Steps

            var sb = new ScrollBar();
            var w = new Window() { Content = sb };
            w.Show();
            sb.Track.Thumb = new Thumb();

Expected behavior

should return

Actual behavior

infinite loop

Regression?

No response

Known Workarounds

No response

Impact

I'm trying to turn a scrollbar into a range selector. To this end, I want to replace the Thumb by an instance of a derived class that behaves differently when a drag is started near the left or right edge of the thumb. This is why I'm trying to reassign the Thumb of the Track.

Configuration

.net framework 4.8

windows 10 x64

I doubt it is configuration specific as the mistake is easy to spot in the source code, as described in "Other Information", and is generic.

Other information

The problem is in TrackBar.UpdateComponent:

                while (i < 3) 
                {
                    // Array isn't full, break
                    if (_visualChildren[i] == null)
                        break;

                    // found the old value
                    if (_visualChildren[i] == oldValue)
                    {
                        // Move values down until end of array or a null element
                        while (i < 2 && _visualChildren[i + 1] != null)
                        {
                            _visualChildren[i] = _visualChildren[i + 1];
                            i++;
                        }
                    }
                    else
                    {
                        i++;
                    }
                }

oldValue is at position 2 in _visualChildren so that the body of the inner loop is not executed, i is not incremented, and the outer loop never ends.

[himgoyalmicro]

@trebahl Can you please provide a minimal sample repro? And is the app that you are developing is only limited to .NET Framework?

[trebahl]

@himgoyalmicro

I did provide a minimal sample: you just have to run the 4 lines of code I included in "Reproduction Steps". I don't understand what more could be needed.

My app is .net framework at the moment, but will be moving to .net in the near future.

But all that is pretty much irrelevant, the mistake in the code I pointed to is blatant and independent of .net vs .net framework.

The fix is also trivial: just using a collection (e.g. List) instead of an array for _visualChildren, and using .Remove instead of duplicating the code of List.

[h3xds1nz]

I don't see how using a List<T> or ICollection<T>-based objects over fixed-size array is a better option in this case (or even considered a fix), not mentioning totally diminishing the intent of how many visual children a Track can actually have.

The fix is indeed trivial though not by doing stuff that doesn't make sense.

[trebahl]

@h3xds1nz It's a fix because you just have to call .Remove instead of this fragile double loop (as I said "and using .Remove instead of duplicating the code of List."; please read my entire sentence before replying).

[miloush]

Please include a repro project as requested in the issue template. You can attach a zip file or link a repo. This 1) ensures we can reproduce the issue in the same environment, including project configuration, xaml templates etc. 2) significantly lowers the barrier for people to engage with your issue and offer help or fix 3) allows us to verify any proposed fix actually resolves your issue.

[h3xds1nz]

@trebahl Nice tone; I'm really keen on helping you to solve the issue now.

@miloush By the way, looking at the function posted, since you have 3 visual children on Track iirc, replacing anything that's not on i=2 should throw if I'm reading this correctly as you should end up with i=3. I don't have time to run it today but just an observation looking at the proc code.

[trebahl]

@h3xds1nz Sorry for getting annoyed, but you started calling me nonsensical even though what I suggested is the correct solution (not duplicating code from the List class).

[trebahl]

Thanks for working on it!