Avalonia icon indicating copy to clipboard operation
Avalonia copied to clipboard

Fix TextBox in AdornerLayer causes collection modified exception in Arrange pass

Open BAndysc opened this issue 1 year ago • 2 comments

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 https://github.com/AvaloniaUI/Avalonia/pull/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

  • [x] Added unit tests (if possible)?
  • [ ] Added XML documentation to any related classes?
  • [ ] Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Breaking changes

n/a

Obsoletions / Deprecations

n/a

Fixed issues

#14483

BAndysc avatar Feb 23 '24 14:02 BAndysc

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

TomEdwardsEnscape avatar Feb 23 '24 15:02 TomEdwardsEnscape

@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.

BAndysc avatar Feb 23 '24 15:02 BAndysc