From 5485a34fc19e1036e251433d97431d349a556aba Mon Sep 17 00:00:00 2001 From: Stephane Delcroix Date: Tue, 2 Dec 2025 15:03:38 +0100 Subject: [PATCH] Fix #31939: CommandParameter TemplateBinding lost during reparenting When a Button inside a ControlTemplate has both Command and CommandParameter with TemplateBinding, the async binding application path can cause Command to be evaluated before CommandParameter resolves, resulting in CanExecute being called with null parameter. ## Solution Add a BindableProperty dependency mechanism via DependsOn() method. When CommandProperty.DependsOn(CommandParameterProperty) is registered, the CommandElement.GetCanExecute() forces the CommandParameter binding to apply before calling CanExecute. This ensures the parameter value is available. ## Changes - BindableProperty: Add Dependencies property and DependsOn() method - BindableObject: Add ForceBindingApply() to force a binding to apply immediately - CommandElement: Force dependency bindings to apply before calling CanExecute - ButtonElement, CheckBox, SearchBar, MenuItem, RefreshView, TextCell: Register Command -> CommandParameter dependency - All ICommandElement implementations: Pass CommandProperty to GetCanExecute() ## Testing Added unit tests that verify: 1. Initial template binding works correctly 2. CommandParameter is preserved after reparenting (the bug scenario) --- src/Controls/src/Core/BindableObject.cs | 19 +++ src/Controls/src/Core/BindableProperty.cs | 15 ++ src/Controls/src/Core/Button/Button.cs | 2 +- src/Controls/src/Core/Button/ButtonElement.cs | 23 ++- src/Controls/src/Core/Cells/TextCell.cs | 26 +-- .../src/Core/CheckBox/CheckBox.Mapper.cs | 8 +- src/Controls/src/Core/CheckBox/CheckBox.cs | 2 +- src/Controls/src/Core/CommandElement.cs | 14 +- .../src/Core/ImageButton/ImageButton.cs | 2 +- src/Controls/src/Core/Menu/MenuItem.cs | 27 ++- .../Core/RefreshView/RefreshView.Mapper.cs | 7 + .../src/Core/RefreshView/RefreshView.cs | 2 +- .../src/Core/SearchBar/SearchBar.Mapper.cs | 7 + src/Controls/src/Core/SearchBar/SearchBar.cs | 2 +- .../Xaml.UnitTests/Issues/Maui31939.xaml | 20 +++ .../Xaml.UnitTests/Issues/Maui31939.xaml.cs | 161 ++++++++++++++++++ 16 files changed, 306 insertions(+), 31 deletions(-) create mode 100644 src/Controls/tests/Xaml.UnitTests/Issues/Maui31939.xaml create mode 100644 src/Controls/tests/Xaml.UnitTests/Issues/Maui31939.xaml.cs diff --git a/src/Controls/src/Core/BindableObject.cs b/src/Controls/src/Core/BindableObject.cs index f205dbe7e611..11ef8b8cac64 100644 --- a/src/Controls/src/Core/BindableObject.cs +++ b/src/Controls/src/Core/BindableObject.cs @@ -432,6 +432,25 @@ internal bool GetIsBound(BindableProperty targetProperty) return bpcontext != null && bpcontext.Bindings.Count > 0; } + /// + /// Forces the binding for the specified property to apply immediately. + /// This is used when one property depends on another and needs the dependent + /// property's binding to resolve before proceeding. + /// See https://github.com/dotnet/maui/issues/31939 + /// + internal void ForceBindingApply(BindableProperty targetProperty) + { + if (targetProperty == null) + throw new ArgumentNullException(nameof(targetProperty)); + + BindablePropertyContext bpcontext = GetContext(targetProperty); + if (bpcontext == null || bpcontext.Bindings.Count == 0) + return; + + // Force the binding to apply now + ApplyBinding(bpcontext, fromBindingContextChanged: false); + } + internal virtual void OnRemoveDynamicResource(BindableProperty property) { } diff --git a/src/Controls/src/Core/BindableProperty.cs b/src/Controls/src/Core/BindableProperty.cs index ed771a2c94c8..ad7eed736eee 100644 --- a/src/Controls/src/Core/BindableProperty.cs +++ b/src/Controls/src/Core/BindableProperty.cs @@ -252,6 +252,21 @@ public sealed class BindableProperty internal ValidateValueDelegate ValidateValue { get; private set; } + // Properties that this property depends on - when getting this property's value, + // if the dependency has a pending binding, return the default value instead. + // This is used to fix timing issues where one property binding resolves before another. + // See https://github.com/dotnet/maui/issues/31939 + internal BindableProperty[] Dependencies { get; private set; } + + /// + /// Registers a dependency on another BindableProperty. When this property's value is retrieved, + /// if the dependency has a binding that hasn't resolved yet (value is null), return null. + /// + internal void DependsOn(params BindableProperty[] dependencies) + { + Dependencies = dependencies; + } + /// Creates a new instance of the BindableProperty class. /// The name of the BindableProperty. /// The type of the property. diff --git a/src/Controls/src/Core/Button/Button.cs b/src/Controls/src/Core/Button/Button.cs index 39caa9c6a301..cc87be1f60c8 100644 --- a/src/Controls/src/Core/Button/Button.cs +++ b/src/Controls/src/Core/Button/Button.cs @@ -465,7 +465,7 @@ void ICommandElement.CanExecuteChanged(object sender, EventArgs e) => RefreshIsEnabledProperty(); protected override bool IsEnabledCore => - base.IsEnabledCore && CommandElement.GetCanExecute(this); + base.IsEnabledCore && CommandElement.GetCanExecute(this, CommandProperty); bool _wasImageLoading; diff --git a/src/Controls/src/Core/Button/ButtonElement.cs b/src/Controls/src/Core/Button/ButtonElement.cs index 00f5c3ff71d1..99869fa7c3d9 100644 --- a/src/Controls/src/Core/Button/ButtonElement.cs +++ b/src/Controls/src/Core/Button/ButtonElement.cs @@ -10,16 +10,27 @@ static class ButtonElement /// /// The backing store for the bindable property. /// - public static readonly BindableProperty CommandProperty = BindableProperty.Create( - nameof(IButtonElement.Command), typeof(ICommand), typeof(IButtonElement), null, - propertyChanging: CommandElement.OnCommandChanging, propertyChanged: CommandElement.OnCommandChanged); + public static readonly BindableProperty CommandProperty; /// /// The backing store for the bindable property. /// - public static readonly BindableProperty CommandParameterProperty = BindableProperty.Create( - nameof(IButtonElement.CommandParameter), typeof(object), typeof(IButtonElement), null, - propertyChanged: CommandElement.OnCommandParameterChanged); + public static readonly BindableProperty CommandParameterProperty; + + static ButtonElement() + { + CommandParameterProperty = BindableProperty.Create( + nameof(IButtonElement.CommandParameter), typeof(object), typeof(IButtonElement), null, + propertyChanged: CommandElement.OnCommandParameterChanged); + + CommandProperty = BindableProperty.Create( + nameof(IButtonElement.Command), typeof(ICommand), typeof(IButtonElement), null, + propertyChanging: CommandElement.OnCommandChanging, propertyChanged: CommandElement.OnCommandChanged); + + // Register dependency: Command depends on CommandParameter for CanExecute evaluation + // See https://github.com/dotnet/maui/issues/31939 + CommandProperty.DependsOn(CommandParameterProperty); + } /// /// The string identifier for the pressed visual state of this control. diff --git a/src/Controls/src/Core/Cells/TextCell.cs b/src/Controls/src/Core/Cells/TextCell.cs index fb976ead8e4b..0b22d618a9f1 100644 --- a/src/Controls/src/Core/Cells/TextCell.cs +++ b/src/Controls/src/Core/Cells/TextCell.cs @@ -11,19 +11,28 @@ namespace Microsoft.Maui.Controls public class TextCell : Cell, ICommandElement { /// Bindable property for . - public static readonly BindableProperty CommandProperty = - BindableProperty.Create(nameof(Command), typeof(ICommand), typeof(TextCell), - propertyChanging: CommandElement.OnCommandChanging, - propertyChanged: CommandElement.OnCommandChanged); + public static readonly BindableProperty CommandProperty; /// Bindable property for . - public static readonly BindableProperty CommandParameterProperty = - BindableProperty.Create(nameof(CommandParameter), + public static readonly BindableProperty CommandParameterProperty; + + static TextCell() + { + CommandParameterProperty = BindableProperty.Create(nameof(CommandParameter), typeof(object), typeof(TextCell), null, propertyChanged: CommandElement.OnCommandParameterChanged); + CommandProperty = BindableProperty.Create(nameof(Command), typeof(ICommand), typeof(TextCell), + propertyChanging: CommandElement.OnCommandChanging, + propertyChanged: CommandElement.OnCommandChanged); + + // Register dependency: Command depends on CommandParameter for CanExecute evaluation + // See https://github.com/dotnet/maui/issues/31939 + CommandProperty.DependsOn(CommandParameterProperty); + } + /// Bindable property for . public static readonly BindableProperty TextProperty = BindableProperty.Create(nameof(Text), typeof(string), typeof(TextCell), default(string)); @@ -95,10 +104,7 @@ protected internal override void OnTapped() void ICommandElement.CanExecuteChanged(object sender, EventArgs eventArgs) { - if (Command is null) - return; - - IsEnabled = Command.CanExecute(CommandParameter); + IsEnabled = CommandElement.GetCanExecute(this, CommandProperty); } WeakCommandSubscription ICommandElement.CleanupTracker { get; set; } diff --git a/src/Controls/src/Core/CheckBox/CheckBox.Mapper.cs b/src/Controls/src/Core/CheckBox/CheckBox.Mapper.cs index d777dd9c45f1..4453b3727b7c 100644 --- a/src/Controls/src/Core/CheckBox/CheckBox.Mapper.cs +++ b/src/Controls/src/Core/CheckBox/CheckBox.Mapper.cs @@ -9,7 +9,13 @@ namespace Microsoft.Maui.Controls { public partial class CheckBox { - static CheckBox() => RemapForControls(); + static CheckBox() + { + // Register dependency: Command depends on CommandParameter for CanExecute evaluation + // See https://github.com/dotnet/maui/issues/31939 + CommandProperty.DependsOn(CommandParameterProperty); + RemapForControls(); + } private new static void RemapForControls() { diff --git a/src/Controls/src/Core/CheckBox/CheckBox.cs b/src/Controls/src/Core/CheckBox/CheckBox.cs index f13ab47ff78e..60f6d77b23c6 100644 --- a/src/Controls/src/Core/CheckBox/CheckBox.cs +++ b/src/Controls/src/Core/CheckBox/CheckBox.cs @@ -145,7 +145,7 @@ void ICommandElement.CanExecuteChanged(object sender, EventArgs e) => RefreshIsEnabledProperty(); protected override bool IsEnabledCore => - base.IsEnabledCore && CommandElement.GetCanExecute(this); + base.IsEnabledCore && CommandElement.GetCanExecute(this, CommandProperty); public Paint Foreground => Color?.AsPaint(); bool ICheckBox.IsChecked diff --git a/src/Controls/src/Core/CommandElement.cs b/src/Controls/src/Core/CommandElement.cs index 6a4a70de5519..767badd9fbfa 100644 --- a/src/Controls/src/Core/CommandElement.cs +++ b/src/Controls/src/Core/CommandElement.cs @@ -37,11 +37,23 @@ public static void OnCommandParameterChanged(BindableObject bo, object o, object commandElement.CanExecuteChanged(bo, EventArgs.Empty); } - public static bool GetCanExecute(ICommandElement commandElement) + public static bool GetCanExecute(ICommandElement commandElement, BindableProperty? commandProperty = null) { if (commandElement.Command == null) return true; + // If there are dependencies (e.g., CommandParameter for Command), force their bindings + // to apply before evaluating CanExecute. This fixes timing issues where Command binding + // resolves before CommandParameter binding during reparenting. + // See https://github.com/dotnet/maui/issues/31939 + if (commandProperty?.Dependencies is not null && commandElement is BindableObject bo) + { + foreach (var dependency in commandProperty.Dependencies) + { + bo.ForceBindingApply(dependency); + } + } + return commandElement.Command.CanExecute(commandElement.CommandParameter); } } diff --git a/src/Controls/src/Core/ImageButton/ImageButton.cs b/src/Controls/src/Core/ImageButton/ImageButton.cs index 73ecfed62810..6262771ca9e3 100644 --- a/src/Controls/src/Core/ImageButton/ImageButton.cs +++ b/src/Controls/src/Core/ImageButton/ImageButton.cs @@ -244,7 +244,7 @@ bool IImageElement.IsAnimationPlaying bool IImageController.GetLoadAsAnimation() => false; protected override bool IsEnabledCore => - base.IsEnabledCore && CommandElement.GetCanExecute(this); + base.IsEnabledCore && CommandElement.GetCanExecute(this, CommandProperty); void ICommandElement.CanExecuteChanged(object sender, EventArgs e) => RefreshIsEnabledProperty(); diff --git a/src/Controls/src/Core/Menu/MenuItem.cs b/src/Controls/src/Core/Menu/MenuItem.cs index fd7f8260b0fc..59577062ed00 100644 --- a/src/Controls/src/Core/Menu/MenuItem.cs +++ b/src/Controls/src/Core/Menu/MenuItem.cs @@ -12,15 +12,26 @@ namespace Microsoft.Maui.Controls public partial class MenuItem : BaseMenuItem, IMenuItemController, ICommandElement, IMenuElement, IPropertyPropagationController { /// Bindable property for . - public static readonly BindableProperty CommandProperty = BindableProperty.Create( - nameof(Command), typeof(ICommand), typeof(MenuItem), null, - propertyChanging: CommandElement.OnCommandChanging, - propertyChanged: CommandElement.OnCommandChanged); + public static readonly BindableProperty CommandProperty; /// Bindable property for . - public static readonly BindableProperty CommandParameterProperty = BindableProperty.Create( - nameof(CommandParameter), typeof(object), typeof(MenuItem), null, - propertyChanged: CommandElement.OnCommandParameterChanged); + public static readonly BindableProperty CommandParameterProperty; + + static MenuItem() + { + CommandParameterProperty = BindableProperty.Create( + nameof(CommandParameter), typeof(object), typeof(MenuItem), null, + propertyChanged: CommandElement.OnCommandParameterChanged); + + CommandProperty = BindableProperty.Create( + nameof(Command), typeof(ICommand), typeof(MenuItem), null, + propertyChanging: CommandElement.OnCommandChanging, + propertyChanged: CommandElement.OnCommandChanged); + + // Register dependency: Command depends on CommandParameter for CanExecute evaluation + // See https://github.com/dotnet/maui/issues/31939 + CommandProperty.DependsOn(CommandParameterProperty); + } /// Bindable property for . public static readonly BindableProperty IsDestructiveProperty = BindableProperty.Create(nameof(IsDestructive), typeof(bool), typeof(MenuItem), false); @@ -122,7 +133,7 @@ static object CoerceIsEnabledProperty(BindableObject bindable, object value) return false; } - var canExecute = CommandElement.GetCanExecute(menuItem); + var canExecute = CommandElement.GetCanExecute(menuItem, CommandProperty); if (!canExecute) { return false; diff --git a/src/Controls/src/Core/RefreshView/RefreshView.Mapper.cs b/src/Controls/src/Core/RefreshView/RefreshView.Mapper.cs index 41329181883d..038f56fb9277 100644 --- a/src/Controls/src/Core/RefreshView/RefreshView.Mapper.cs +++ b/src/Controls/src/Core/RefreshView/RefreshView.Mapper.cs @@ -6,6 +6,13 @@ namespace Microsoft.Maui.Controls { public partial class RefreshView { + static RefreshView() + { + // Register dependency: Command depends on CommandParameter for CanExecute evaluation + // See https://github.com/dotnet/maui/issues/31939 + CommandProperty.DependsOn(CommandParameterProperty); + } + internal static new void RemapForControls() { // Adjust the mappings to preserve Controls.RefreshView legacy behaviors diff --git a/src/Controls/src/Core/RefreshView/RefreshView.cs b/src/Controls/src/Core/RefreshView/RefreshView.cs index ff2de69c5e81..564806dbeccb 100644 --- a/src/Controls/src/Core/RefreshView/RefreshView.cs +++ b/src/Controls/src/Core/RefreshView/RefreshView.cs @@ -121,7 +121,7 @@ static object CoerceIsRefreshEnabledProperty(BindableObject bindable, object val if (bindable is RefreshView refreshView) { refreshView._isRefreshEnabledExplicit = (bool)value; - return refreshView._isRefreshEnabledExplicit && CommandElement.GetCanExecute(refreshView); + return refreshView._isRefreshEnabledExplicit && CommandElement.GetCanExecute(refreshView, CommandProperty); } return false; diff --git a/src/Controls/src/Core/SearchBar/SearchBar.Mapper.cs b/src/Controls/src/Core/SearchBar/SearchBar.Mapper.cs index 0bfd6cc6806f..3278ba05f637 100644 --- a/src/Controls/src/Core/SearchBar/SearchBar.Mapper.cs +++ b/src/Controls/src/Core/SearchBar/SearchBar.Mapper.cs @@ -6,6 +6,13 @@ namespace Microsoft.Maui.Controls { public partial class SearchBar { + static SearchBar() + { + // Register dependency: SearchCommand depends on SearchCommandParameter for CanExecute evaluation + // See https://github.com/dotnet/maui/issues/31939 + SearchCommandProperty.DependsOn(SearchCommandParameterProperty); + } + internal static new void RemapForControls() { // Adjust the mappings to preserve Controls.SearchBar legacy behaviors diff --git a/src/Controls/src/Core/SearchBar/SearchBar.cs b/src/Controls/src/Core/SearchBar/SearchBar.cs index fbc3b649e5f3..2b7e09dd629a 100644 --- a/src/Controls/src/Core/SearchBar/SearchBar.cs +++ b/src/Controls/src/Core/SearchBar/SearchBar.cs @@ -164,7 +164,7 @@ private void OnRequestedThemeChanged(object sender, AppThemeChangedEventArgs e) object ICommandElement.CommandParameter => SearchCommandParameter; protected override bool IsEnabledCore => - base.IsEnabledCore && CommandElement.GetCanExecute(this); + base.IsEnabledCore && CommandElement.GetCanExecute(this, SearchCommandProperty); void ICommandElement.CanExecuteChanged(object sender, EventArgs e) => RefreshIsEnabledProperty(); diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui31939.xaml b/src/Controls/tests/Xaml.UnitTests/Issues/Maui31939.xaml new file mode 100644 index 000000000000..80a671ace710 --- /dev/null +++ b/src/Controls/tests/Xaml.UnitTests/Issues/Maui31939.xaml @@ -0,0 +1,20 @@ + + + + + + +