From fee3032a043ad1096b174903edd429a53c0ef664 Mon Sep 17 00:00:00 2001 From: zacfromaustinpowder <165237243+zacfromaustinpowder@users.noreply.github.com> Date: Sat, 8 Nov 2025 02:15:55 +1000 Subject: [PATCH] Fixed Selector.ValidateNestingSelector not calling overrides when iterating up through parent selectors (#19947) * Fixed Selector.ValidateNestingSelector not calling overrides when iterating up through parent selectors Changed ValidateNestingSelector to call recursively, walking up the hierarchy of parent selectors. This means function overrides are taken into account when walking up the tree. Now if a selector has an OrSelector as its parent, it doesn't throw an exception. * Updated ToString methods to surround OrSelectors with parenthesis where useful --- src/Avalonia.Base/Styling/ChildSelector.cs | 2 +- .../Styling/DescendentSelector.cs | 2 +- src/Avalonia.Base/Styling/NotSelector.cs | 2 +- src/Avalonia.Base/Styling/NthChildSelector.cs | 6 +-- src/Avalonia.Base/Styling/OrSelector.cs | 16 ++++++-- .../Styling/PropertyEqualsSelector.cs | 3 +- src/Avalonia.Base/Styling/Selector.cs | 41 +++++++++++-------- src/Avalonia.Base/Styling/Selectors.cs | 3 +- src/Avalonia.Base/Styling/TemplateSelector.cs | 2 +- .../Styling/TypeNameAndClassSelector.cs | 2 +- .../Styling/SelectorTests_Or.cs | 19 +++++++++ 11 files changed, 64 insertions(+), 34 deletions(-) diff --git a/src/Avalonia.Base/Styling/ChildSelector.cs b/src/Avalonia.Base/Styling/ChildSelector.cs index ac28d2bc46..b118ea5561 100644 --- a/src/Avalonia.Base/Styling/ChildSelector.cs +++ b/src/Avalonia.Base/Styling/ChildSelector.cs @@ -31,7 +31,7 @@ namespace Avalonia.Styling { if (_selectorString == null) { - _selectorString = _parent.ToString(owner) + " > "; + _selectorString = _parent.ToString(owner, true) + " > "; } return _selectorString; diff --git a/src/Avalonia.Base/Styling/DescendentSelector.cs b/src/Avalonia.Base/Styling/DescendentSelector.cs index 6706eb4441..646f7272a5 100644 --- a/src/Avalonia.Base/Styling/DescendentSelector.cs +++ b/src/Avalonia.Base/Styling/DescendentSelector.cs @@ -29,7 +29,7 @@ namespace Avalonia.Styling { if (_selectorString == null) { - _selectorString = _parent.ToString(owner) + ' '; + _selectorString = _parent.ToString(owner, true) + ' '; } return _selectorString; diff --git a/src/Avalonia.Base/Styling/NotSelector.cs b/src/Avalonia.Base/Styling/NotSelector.cs index 9a541cbba7..dca8a45ef8 100644 --- a/src/Avalonia.Base/Styling/NotSelector.cs +++ b/src/Avalonia.Base/Styling/NotSelector.cs @@ -39,7 +39,7 @@ namespace Avalonia.Styling { if (_selectorString == null) { - _selectorString = $"{_previous?.ToString(owner)}:not({_argument})"; + _selectorString = $"{_previous?.ToString(owner, true)}:not({_argument})"; } return _selectorString; diff --git a/src/Avalonia.Base/Styling/NthChildSelector.cs b/src/Avalonia.Base/Styling/NthChildSelector.cs index bf6247aba1..e11b81a088 100644 --- a/src/Avalonia.Base/Styling/NthChildSelector.cs +++ b/src/Avalonia.Base/Styling/NthChildSelector.cs @@ -109,9 +109,9 @@ namespace Avalonia.Styling public override string ToString(Style? owner) { var expectedCapacity = NthLastChildSelectorName.Length + 8; - var stringBuilder = StringBuilderCache.Acquire(expectedCapacity); - stringBuilder.Append(_previous?.ToString(owner)); - + var stringBuilder = StringBuilderCache.Acquire(expectedCapacity); + stringBuilder.Append(_previous?.ToString(owner, true)); + stringBuilder.Append(':'); stringBuilder.Append(_reversed ? NthLastChildSelectorName : NthChildSelectorName); stringBuilder.Append('('); diff --git a/src/Avalonia.Base/Styling/OrSelector.cs b/src/Avalonia.Base/Styling/OrSelector.cs index cc77aa9fcf..a58ced7c65 100644 --- a/src/Avalonia.Base/Styling/OrSelector.cs +++ b/src/Avalonia.Base/Styling/OrSelector.cs @@ -45,11 +45,19 @@ namespace Avalonia.Styling internal override Type? TargetType => _targetType ??= EvaluateTargetType(); /// - public override string ToString(Style? owner) + public override string ToString(Style? owner) => ToString(owner, false); + + /// + internal override string ToString(Style? owner, bool hasNext) { if (_selectorString == null) { - _selectorString = string.Join(", ", _selectors.Select(x => x.ToString(owner))); + _selectorString = string.Join(", ", _selectors.Select(x => x.ToString(owner, true))); + + if (hasNext) + { + _selectorString = $"({_selectorString})"; + } } return _selectorString; @@ -97,13 +105,13 @@ namespace Avalonia.Styling private protected override Selector? MovePrevious() => null; private protected override Selector? MovePreviousOrParent() => null; - internal override void ValidateNestingSelector(bool inControlTheme) + internal override void ValidateNestingSelector(bool inControlTheme, int templateCount = 0) { var count = _selectors.Count; for (var i = 0; i < count; i++) { - _selectors[i].ValidateNestingSelector(inControlTheme); + _selectors[i].ValidateNestingSelector(inControlTheme, templateCount); } } diff --git a/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs b/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs index 1d684eeca3..316b7a2853 100644 --- a/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs +++ b/src/Avalonia.Base/Styling/PropertyEqualsSelector.cs @@ -45,7 +45,7 @@ namespace Avalonia.Styling if (_previous != null) { - builder.Append(_previous.ToString(owner)); + builder.Append(_previous.ToString(owner, true)); } builder.Append('['); @@ -85,7 +85,6 @@ namespace Avalonia.Styling ? SelectorMatch.AlwaysThisInstance : SelectorMatch.NeverThisInstance; } - } private protected override Selector? MovePrevious() => _previous; diff --git a/src/Avalonia.Base/Styling/Selector.cs b/src/Avalonia.Base/Styling/Selector.cs index cb3ddc343c..9102d2e770 100644 --- a/src/Avalonia.Base/Styling/Selector.cs +++ b/src/Avalonia.Base/Styling/Selector.cs @@ -47,7 +47,7 @@ namespace Avalonia.Styling // right-to-left, so MatchUntilCombinator reverses this order because the type selector // will be on the left. var match = MatchUntilCombinator(control, this, parent, subscribe, out var combinator); - + // If the pre-combinator selector matches, we can now match the combinator, if any. if (match.IsMatch && combinator is object) { @@ -76,6 +76,10 @@ namespace Avalonia.Styling /// The owner style. public abstract string ToString(Style? owner); + /// + /// Whether there is a selector that comes after this one. + internal virtual string ToString(Style? owner, bool hasNext) => ToString(owner); + /// /// Evaluates the selector for a match. /// @@ -100,30 +104,31 @@ namespace Avalonia.Styling /// private protected abstract Selector? MovePreviousOrParent(); - internal virtual void ValidateNestingSelector(bool inControlTheme) + internal virtual void ValidateNestingSelector(bool inControlTheme, int templateCount = 0) { var s = this; - var templateCount = 0; - do + if (inControlTheme) { - if (inControlTheme) - { - if (!s.InTemplate && s.IsCombinator) - throw new InvalidOperationException( - "ControlTheme style may not directly contain a child or descendent selector."); - if (s is TemplateSelector && templateCount++ > 0) - throw new InvalidOperationException( - "ControlTemplate styles cannot contain multiple template selectors."); - } + if (!s.InTemplate && s.IsCombinator) + throw new InvalidOperationException( + "ControlTheme style may not directly contain a child or descendent selector."); + if (s is TemplateSelector && templateCount++ > 0) + throw new InvalidOperationException( + "ControlTemplate styles cannot contain multiple template selectors."); + } - var previous = s.MovePreviousOrParent(); + var previous = s.MovePreviousOrParent(); - if (previous is null && s is not NestingSelector) + if (previous is null) + { + if (s is not NestingSelector) throw new InvalidOperationException("Child styles must have a nesting selector."); - - s = previous; - } while (s is not null); + } + else + { + previous.ValidateNestingSelector(inControlTheme, templateCount); + } } private static SelectorMatch MatchUntilCombinator( diff --git a/src/Avalonia.Base/Styling/Selectors.cs b/src/Avalonia.Base/Styling/Selectors.cs index d7406f2164..a58ed3f11d 100644 --- a/src/Avalonia.Base/Styling/Selectors.cs +++ b/src/Avalonia.Base/Styling/Selectors.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; namespace Avalonia.Styling { @@ -124,7 +123,7 @@ namespace Avalonia.Styling { return new NotSelector(previous, argument(null)); } - + /// /// Returns a selector which inverts the results of selector argument. /// diff --git a/src/Avalonia.Base/Styling/TemplateSelector.cs b/src/Avalonia.Base/Styling/TemplateSelector.cs index 1fa2ca2d0f..4fcccef87e 100644 --- a/src/Avalonia.Base/Styling/TemplateSelector.cs +++ b/src/Avalonia.Base/Styling/TemplateSelector.cs @@ -30,7 +30,7 @@ namespace Avalonia.Styling { if (_selectorString == null) { - _selectorString = _parent.ToString(owner) + " /template/ "; + _selectorString = _parent.ToString(owner, true) + " /template/ "; } return _selectorString; diff --git a/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs b/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs index 81b204761b..4fba8c02c6 100644 --- a/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs +++ b/src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs @@ -143,7 +143,7 @@ namespace Avalonia.Styling if (_previous != null) { - builder.Append(_previous.ToString(owner)); + builder.Append(_previous.ToString(owner, true)); } if (TargetType != null) diff --git a/tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Or.cs b/tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Or.cs index fb5d54bd1f..c013778128 100644 --- a/tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Or.cs +++ b/tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Or.cs @@ -1,3 +1,4 @@ +using System; using Avalonia.Controls; using Avalonia.Styling; using Xunit; @@ -89,6 +90,24 @@ namespace Avalonia.Base.UnitTests.Styling Assert.Equal(null, target.TargetType); } + + [Fact] + public void ValidateNestingSelector_Checks_Children_When_Parent_Is_An_OrSelector() + { + var target = Selectors.Or( + default(Selector).Class("foo"), + default(Selector).Class("bar") + ).Name("baz"); + + Assert.Throws(() => target.ValidateNestingSelector(false)); + + target = Selectors.Or( + default(Selector).Nesting().Class("foo"), + default(Selector).Nesting().Class("bar") + ).Name("baz"); + + target.ValidateNestingSelector(false); + } public class Control1 : Control {