Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IndexOutOfRangeException/Infinite loop in UpdateComponent (Track control) #9934

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

h3xds1nz
Copy link
Contributor

@h3xds1nz h3xds1nz commented Oct 11, 2024

Fixes #9904

Description

Fixes IndexOutOfRangeException and infinite loop issues when updating any of Track's children after they've been already set from template or manually.

This issue is present when you attempt to update via any of the public properties (DecreaseRepeatButton, IncreaseRepeatButton, Thumb) after all the public members of Track has already been previously set (and therefore _visualChildren is initialized and has stored all three properties).

In standard Aero2 theme, using a ScrollBar (as Track has no template on its own), the order of the children is:

  • _visualChildren[0] = RepeatButton1
  • _visualChildren[1] = RepeatButton2
  • _visualChildren[2] = Thumb

Setting item at position 2 (which would be Thumb property now) will result in an infinite loop.
Setting items at positions 0 and 1 will result in IndexOutOfRangeException during assigment.

The simplest reproduction code with pure Track goes like this:

Track crashMe = new Track();
crashMe.DecreaseRepeatButton = new RepeatButton();
crashMe.IncreaseRepeatButton = new RepeatButton();
crashMe.Thumb = new Thumb();

// Uncomment this for IndexOutOfRangeException
// crashMe.IncreaseRepeatButton = new RepeatButton();

// Uncomment this for infinite loop
// crashMe.Thumb = new Thumb()

The original code was written with updates in mind but I think it got a bit lost in shifting array members. I have fixed it to behave exactly as described in Track.GetVisualChild and Track.VisualChildrenCount describes (+ the original comments), that means whenever you update an item, the array is shifted to the left if needed, so the order is always from oldest to newest (left to right).
This is aligned with VisualChildrenCount behavior where NULL allows you to remove children basically as well.

Customer Impact

Customers will be finally able to change the Track components as they please and won't experience undocumented behavior and exceptions that were not originally inteded.

Regression

This seems to be present way back. PresentationFramework v3 from NetFX has a bit different code but also buggy.

Testing

Testing the forementioned exceptions and the order of the items, visual refresh, everything seems in order.

Risk

Low, a loop has been fixed to work properly without changing anything else (e.g. collection types, etc.)

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz requested review from a team as code owners October 11, 2024 00:12
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

infinite loop when assigning the Thumb of a Track
1 participant