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 TextBox in AdornerLayer causes collection modified exception in Arrange pass #14714

Conversation

BAndysc
Copy link
Contributor

@BAndysc BAndysc commented Feb 23, 2024

What does the pull request do?

This PR changes VisualLayerManager so that layers can be added during its Arrange pass.

What is the current behavior?

In my previous PR #14484 I intentionally didn't touch ArrangeOverride method, as I didn't want to touch things that are not directly required.

However, it turned out it is required.

I assumed that Measure will always be called before Arrange. However, this is not always true. Consider this:

root.Measure(new Size(10, 10));

var adorner = new TextBox();
adornerLayer.Children.Add(adorner);
AdornerLayer.SetAdornedElement(adorner, button);

root.Arrange(new Rect(0, 0, 10, 10));

In root.Arrange, root already has valid measurement (IsMeasureValid = true) yet in the meantime a new control was added, which will reference VisualLayerManager.TextSelectorLayer -> crash.

What is the updated/expected behavior with this PR?

Both Measure and Arrange methods in VisualLayerManager will support adding layers during iteration.

Checklist

Breaking changes

n/a

Obsoletions / Deprecations

n/a

Fixed issues

#14483

@TomEdwardsEnscape
Copy link
Contributor

This pattern means that items in the collection can be skipped, or arranged twice. See #13498 for a more robust solution.

@BAndysc
Copy link
Contributor Author

BAndysc commented Feb 23, 2024

@TomEdwardsEnscape VisualLayerManager is a very specific case of a container, elements are only added to it, never removed or moved, so this is not a problem.

Also elements are added only to the end of the list, so not only this never arrange elements twice, but also never skips new elements.

@maxkatz6 maxkatz6 added this pull request to the merge queue Mar 7, 2024
Merged via the queue into AvaloniaUI:master with commit 65b1c45 Mar 7, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants