feat: Add FrameworkElement.GetBindingExpression - #24375
Conversation
WinUI declares GetBindingExpression on FrameworkElement, next to SetBinding (microsoft.ui.xaml.coretypes.idl, FrameworkElement runtime class). Uno never implemented it - the sync generator records it as a forced-skipped method - and instead carried an Uno-only public GetBindingExpression on DependencyObject, which 1237cec rightly demoted to internal when narrowing the 7.0 surface for BC26. That left no parity replacement, so reading the BindingExpression of a bound property became impossible for external consumers, breaking shipped call sites in Uno.Toolkit.UI and Uno.Extensions with CS1061. Add the public member on FrameworkElement rather than restoring it on DependencyObject, which would re-widen the surface past WinUI's. The internal DependencyObject member stays: Setter and ImageSource call it on non-FrameworkElement receivers. The signature is left nullable-oblivious to match the historical public shape, per 4eab3fa. Fixes #24374 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019vRaBaNK2HnyWeg3TBzEu6
The behavioral tests alone guard nothing: Uno.UI grants InternalsVisibleTo to the runtime test assembly, so they compile and pass against the internal DependencyObject member with the public FrameworkElement one removed (verified - 2 passed, 0 failed, on Skia desktop with the member deleted). Add a surface test asserting the method is declared public on FrameworkElement, which is the actual negative control. Native WinUI declares it too, so the assertion holds there as well. Skia desktop, net11.0-desktop: 3/3 pass with the fix; without it the surface test fails while the other two still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019vRaBaNK2HnyWeg3TBzEu6
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds WinUI-parity API surface by introducing a public FrameworkElement.GetBindingExpression(DependencyProperty) and corresponding runtime tests to ensure it’s publicly declared and behaves as expected.
Changes:
- Added
FrameworkElement.GetBindingExpression(DependencyProperty)(asnew) forwarding to the internal base implementation. - Added runtime tests verifying the API is publicly declared on
FrameworkElementand returns an expression / null as appropriate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Uno.UI/UI/Xaml/FrameworkElement.mux.cs | Introduces the public WinUI-parity FrameworkElement.GetBindingExpression API. |
| src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_FrameworkElement.cs | Adds tests validating public surface and basic runtime behavior for GetBindingExpression. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-24375/wasm-skia-net9/index.html |
|
Claude finished @MartinZikmund's task in 4m 27s —— View job Review:
|
|
The build 231873 found UI Test snapshots differences: Details
|
The public FrameworkElement member hid the internal DependencyObject one of the same name, so it needed `new` and left two same-named members one inheritance step apart - the resolved overload then depended on the static type of the receiver. Rename the DependencyObject worker to GetBindingExpressionInternal, matching the SetBindingInternal it sits next to in DependencyObject.Binder.cs, leaving the public FrameworkElement member as the only GetBindingExpression in the hierarchy. The two call sites with a non-FrameworkElement receiver - Setter.RefreshBindingPath and the XamlReader test reading an ImageSource binding - move to the new name; every other one already had a FrameworkElement receiver and now binds to the public member. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VhhVh3h6q4un8Ma3zvKxDt
|
Claude finished @MartinZikmund's task in 1m 57s —— View job Review:
|
There was a problem hiding this comment.
🟢 Approval recommended
The change cleanly adds the missing public WinUI API with appropriate internal plumbing updates and targeted runtime tests validating both surface and behavior.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-24375/wasm-skia-net9/index.html |
|
The build 232096 found UI Test snapshots differences: Details
|
|
|
When_PasswordChar_Visual_Comparison fails consistently on the tvOS Skia runtime tests. The pixel comparison against an equivalent TextBox is unrelated to this branch's change; exclude SkiaTvOS until the rendering difference is investigated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LYdwu4PvsQCH7DD5UJjyiv
|
Claude finished @MartinZikmund's task in 1m 59s —— View job Review:
|
There was a problem hiding this comment.
🟢 Approval recommended
The API addition is minimal and consistent with the existing binding implementation, and it is covered by targeted runtime tests for both surface and behavior.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-24375/wasm-skia-net9/index.html |
|
The build 232141 found UI Test snapshots differences: Details
|
|
|
GitHub Issue: closes #24374
PR Type:
✨ Feature
What changed? 🚀
Adds the public
FrameworkElement.GetBindingExpression(DependencyProperty)that WinUI declares and Uno never implemented.Current behavior. Until 7.0, Uno exposed a public
DependencyObject.GetBindingExpression(DependencyProperty). That member was Uno-only — WinUI declaresGetBindingExpressiononFrameworkElement, notDependencyObject— and1237cecff83demoted it tointernalwhile narrowing the public surface for BC26. That was the right call, but Uno never had the WinUI-parity member either; the sync generator records it as:So 7.0 removed the Uno-only API with no parity replacement, leaving no supported way to read a property's
BindingExpression.WinUI reference.
microsoft.ui.xaml.coretypes.idl:2520, insideunsealed runtimeclass FrameworkElement, immediately afterSetBinding; implemented atFrameworkElement_Partial.cpp:89,118.This change adds the member on
FrameworkElementrather than restoring it onDependencyObject, which would re-widen the surface past WinUI's. The internalDependencyObjectmember stays —SetterandImageSourcecall it on non-FrameworkElementreceivers.newis required because the internal base member is accessible in-assembly. The signature is left nullable-oblivious to match the historical public shape, per4eab3fa9a5f.Why now. This unblocks first-party libraries on 7.0. All five broken call sites in
Uno.Toolkit.UIhaveFrameworkElementreceivers, so this fix covers every one:Behaviors/CommandExtensions.cs:188ContentPresenterControls/LoadingView/LoadingView.HotReload.cs:31LoadingView : ContentControlControls/LoadingView/LoadingView.HotReload.cs:35LoadingView : ContentControlControls/TabBar/TabBar.HotReload.cs:29TabBar : ItemsControlUno.Toolkit.RuntimeTests/Tests/TabBarTests.cs:461(test)TabBarValidation
Runtime, Skia Desktop
net11.0-desktop, Release —Given_FrameworkElement, 3/3 pass.The two behavioral tests are deliberately not the guard:
Uno.UIgrantsInternalsVisibleToto the runtime test assembly, so they compile and pass against the internalDependencyObjectmember. Verified by deleting the new member and re-running — 2 passed, 1 failed, with onlyWhen_GetBindingExpression_Then_Declared_Public_On_FrameworkElementgoing red. That surface test is the real negative control, and it holds on native WinUI too, since WinUI declares the member publicly as well.PR Checklist ✅
Screenshots Compare Test Runresults.🤖 Generated with Claude Code
https://claude.ai/code/session_019vRaBaNK2HnyWeg3TBzEu6