[Extensibility] XAML: Expose GTK, macOS, and WPF in OnPlatform - #35901
Conversation
The {OnPlatform} and {OnIdiom} XAML markup extensions resolved values via a
hardcoded if-chain over DevicePlatform/DeviceIdiom, and the GTK, macOS and WPF
properties on OnPlatformExtension were internal. As a result, custom backends
(e.g. GTK/Linux) could not express platform-conditional values the way
first-party platforms can, even though the element form (OnPlatform<T>/On) is
already data-driven and supports arbitrary platform strings.
- Resolve values by comparing DeviceInfo.Platform.ToString() /
DeviceInfo.Idiom.ToString() (case-insensitive) against a lookup built from the
named properties, mirroring the data-driven element form. UWP/WinUI
precedence is preserved.
- Make GTK, macOS and WPF public on OnPlatformExtension.
Fixes dotnet#35695
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35901Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35901" |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
🤖 Multimodal code review summaryI ran an independent multi-model review of this PR with three top models — Claude Opus 4.8, GPT‑5.5, and Gemini 3.1 Pro. Consolidated findings: ✅ Verified correct (unanimous)
🔧 Finding to address — case sensitivity (GPT‑5.5: Major, Opus: Minor)The old chain compared via ➡️ Fix: switch both lookups to 📝 Considered — per‑call dictionary allocation (Gemini: Critical; GPT‑5.5 & Opus: acceptable)Gemini flagged the Applying the case‑sensitivity fix now and re‑reviewing afterward. |
Multimodal review (Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro) found the new lookups used StringComparer.OrdinalIgnoreCase, which was the only behavioral delta versus the old DevicePlatform/DeviceIdiom comparison (Ordinal) and was inconsistent with the case-sensitive element form (OnPlatform<T>/On) and the compile-time SimplifyOnPlatformVisitor. Switch both lookups to StringComparer.Ordinal for exact behavioral parity and runtime/compile-time consistency, and correct the related comments and tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kubaflo
left a comment
There was a problem hiding this comment.
🤖 Round-2 multimodal review — all clear
Re-reviewed the case-sensitivity fix with Claude Opus 4.8 and GPT-5.5:
- ✅ Case-sensitivity finding resolved. Both confirm the switch to
StringComparer.Ordinalrestores exact parity with the oldDevicePlatform/DeviceIdiom(Ordinal) comparison and is consistent with the element form + compile-timeSimplifyOnPlatformVisitor. - ✅ Exhaustive parity (Opus): old
if-chain vs. new lookup across 15,360 platform + 640 idiom combinations → 0 mismatches. - ✅ Tests updated correctly (case-variant strings →
Default); comments accurate; the 7PublicAPI.Unshipped.txtentries correct; no new issues. - 📝 Dictionary allocation (Gemini round-1 Critical): both round-2 reviewers confirm it's acceptable —
ProvideValueruns once per inflation and is dominated by the surrounding reflection/ConvertTo, and #35695 explicitly asks for a data-driven dictionary. Kept intentionally.
Blocking issues: none. Local tests: all 145 OnPlatform/OnIdiom cases pass; no regressions (the 2 unrelated failures in the wider suite are pre-existing culture-dependent decimal-binding tests).
Inline notes on the key lines below. 👇
Automated fleet code review — PR #35901Reviewed head SHA: Verdict: NEEDS_DISCUSSION Independent assessmentThe code replaces hardcoded Findings❌ ErrorsNone found in the changed code.
|
A third multimodal review round (Claude Opus 4.8, GPT-5.5, Gemini 3.1 Pro) noted the per-call Dictionary built in ProvideValue: it allocates on the runtime resolution path (Windows/Tizen, and any runtime-loaded XAML, since SimplifyOnPlatformVisitor only compile-simplifies android/ios/macos/maccatalyst), including the DataTemplate inflation path. It also added no extensibility over a plain comparison, because the markup-extension parser only accepts properties declared on the type. Replace the Dictionary with a zero-allocation, string-keyed if-chain over the cached DeviceInfo.Platform.ToString() / DeviceInfo.Idiom.ToString() value. This keeps the data-driven string matching (Ordinal, matching DevicePlatform/ DeviceIdiom equality and the element form), preserves exact behavior including UWP/WinUI precedence, and removes the now-unused System.Collections.Generic import. All 145 OnPlatform/OnIdiom unit tests still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kubaflo
left a comment
There was a problem hiding this comment.
🤖 Round-3 multimodal review + CI triage
Ran a third independent review with Claude Opus 4.8, GPT-5.5, and Gemini 3.1 Pro, plus a full CI investigation.
CI failures — NOT caused by this PR ✅
The 7 red checks are pre-existing net11.0 issues, not this change:
- AOT macOS + RunOniOS_MauiRelease / TrimFull / TrimFull_CoreCLR fail in
BuildWarningsUtilities.AssertWarnings— the AOT/trim expected-warnings baseline drifted because the template now emitsHybridWebViewHandlerIL2026/IL3050(System.Text.Json) warnings unrelated to OnPlatform/OnIdiom. The identical failures appear on unrelated net11.0 PRs (#35892, #35891, #35870). - Build windows was canceled, not failed (known issue dnceng#3008).
- All Build, Pack, and Helix Unit Test jobs (which run the XAML unit tests exercising this change, under trim/AOT analyzers +
TreatWarningsAsErrors) passed.
All three models independently concluded the change cannot explain the AOT/Trim/Windows failures (ref-type generic instantiation is AOT-safe, static local function lowers to a private static method, additive public API).
Finding addressed — per-call Dictionary allocation
Gemini reaffirmed Critical; Opus rated it Minor but agreed "a zero-alloc string-keyed if-chain is a reasonable micro-opt"; GPT-5.5 had no objection. I verified Gemini's two supporting claims:
SimplifyOnPlatformVisitoronly compile-simplifies android/ios/macos/maccatalyst →{OnPlatform}/{OnIdiom}do run at runtime on Windows/Tizen (incl. theDataTemplateinflation path).MarkupExtensionParseronly accepts properties declared on the type → the dictionary added no extensibility over a direct comparison.
➡️ Fix applied (commit 7a59777): replaced the per-call Dictionary with a zero-allocation, string-keyed if-chain over the cached DeviceInfo.Platform.ToString() / DeviceInfo.Idiom.ToString(). Keeps the data-driven Ordinal matching and exact UWP/WinUI precedence; removed the now-unused using. Source builds with 0 warnings under warnings-as-errors; all 145 OnPlatform/OnIdiom tests still pass.
Everything else — clean
Correctness, UWP/WinUI precedence, OnIdiom null sentinel, nullability/NRT, trimming/AOT, SourceGen↔runtime consistency, public API (7 TFMs), and edge cases (Unknown/empty/tvOS/watchOS → Default, Converter path) all verified across the three models. No remaining blocking issues.
Automated fleet code review — PR #35901Reviewed head SHA: Verdict: NEEDS_DISCUSSION Independent assessmentThe new commit replaces the prior per-call lookup/dictionary approach with a zero-allocation string-keyed if-chain. The behavior remains exact-case ( Reconciliation with prior review / PR narrativeRound22's allocation concern appears addressed by the new zero-allocation matching commit. However, the PR description is still stale: it says matching is case-insensitive and that tests cover Findings by severity❌ ErrorsNone found in the changed code.
|
✅ Final multimodal merge-readiness review — unanimous
|
| Model | Verdict |
|---|---|
| Claude Opus 4.8 | MERGEABLE |
| Claude Opus 4.6 | MERGEABLE |
| Gemini 3.1 Pro | MERGEABLE |
| GPT‑5.5 | MERGEABLE |
What all four models independently confirmed:
- Behavioral equivalence is exhaustive — Opus 4.8 simulated 13,312
{OnPlatform}combinations (13 platform strings × all 2¹⁰ set/unset states) and 576{OnIdiom}combinations against the old hardcoded if‑chain → 0 mismatches. - UWP/WinUI precedence preserved exactly —
WinUIwins overUWP;UWPstill matches both the WinUI platform and a legacy"UWP"string. (Including the trap thatDevicePlatform.UWP's internal string is actually"WinUI".) Ordinalis the correct comparison — it matchesDevicePlatform/DeviceIdiom.Equals(which useStringComparison.Ordinal), the runtime element form (List<string>.ContainsonDeviceInfo.Platform.ToString()), and the compile‑timeSimplifyOnPlatformVisitor(case‑sensitive==). Case‑variant inputs correctly fall through toDefault— covered by new"android"/"gtk"/"phone"tests.OnIdiomnull‑sentinel maps exactly to the old?? Defaultcoalescing; matching is allocation‑free (ToString()returns the backing string field; theDictionaryfrom an earlier round was replaced by a zero‑alloc if‑chain).- Public
GTK/macOS/WPFis justified, not speculative — these are functional value properties wired intoTryGetValueForPlatform, sitting alongside already‑public Android/iOS/MacCatalyst/WinUI/UWP, enabling community backends via{OnPlatform GTK=…}; new tests demonstrate real resolution.PublicAPI.Unshipped.txtis correct and identical across all 7 TFMs.
CI note: red AOT‑macOS / RunOniOS legs are the pre‑existing net11 HybridWebViewHandler baseline drift shared by sibling PRs; Build / Pack / Helix Unit Tests are green, and 145 OnPlatform/OnIdiom unit tests pass locally.
|
@StephaneDelcroix — design question before we take this further. cc @PureWeen Context. This PR makes the The limitation I want your call on. The matching is now data-driven, but the property surface of the markup extension is still a fixed set of CLR properties ( The element form already supports arbitrary platforms today — <Label.Text>
<OnPlatform x:TypeArguments="x:String">
<On Platform="Web" Value="..." />
</OnPlatform>
</Label.Text>So the gap is specifically the attribute markup-extension syntax Questions:
Same question applies to |
|
@StephaneDelcroix — concrete follow-up to the dynamic-key question, now traced through all three code paths so we can pick a direction. Goal: let a custom backend (e.g. a future Where the hardcoded surface bites:
So the attribute form is blocked on paths (1) and (3); the element form Proposed design (opt-in, scoped to these two extensions): interface IAcceptArbitraryKeys { bool TrySetKey(string key, object value, IServiceProvider sp); }
Questions for you:
Happy to implement whichever direction you prefer on |
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
Updated-head local validation on |
|
Fresh main build 1530348 fails only |
PureWeen
left a comment
There was a problem hiding this comment.
Follow-up — prior 💡 resolved ✅
Re-checked the delta since my earlier review (pullrequestreview-4787762726). The change at HEAD (commit 8669744b + a net11.0 merge) is documentation-only — 4 added lines inside the <remarks> block on OnPlatformExtension.cs; no executable code changed, so no re-run of the full review was needed.
The new wording is factually accurate — verified against DevicePlatform at HEAD:
DevicePlatform.Equals(string)(DevicePlatform.shared.cs:92-93) isstring.Equals(devicePlatform, other, StringComparison.Ordinal), andoperator ==(line 123) delegates to it. So the matcher'sDeviceInfo.Platform == DevicePlatform.Create("GTK")is an ordinal, case-sensitive comparison — exactly as the new text states.- The concrete example is correct:
DevicePlatform.Create("gtk")(lowercase) does not match theGTKproperty and falls through toDefault. - The
<see cref="Default"/>andDevicePlatformcref links resolve to real members.
Nothing is overstated or misstated. My earlier 💡 documentation finding is fully addressed and I have no further findings. Code-only review; CI status out of scope.
3 independent reviewers · adversarial consensus (prior round) · doc-accuracy re-verification (this round).
|
Device build 1530359 fails the exact same six platform tasks as net11.0 base 1530285: MacCatalyst CoreCLR, iOS CoreCLR/Mono, Android CoreCLR/Mono, and Windows. Representative evidence matches the same six HybridWebView exception-message assertions and Android |
|
UI build 1530352 has one isolated failure: |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 66f84348-6476-4097-8b7f-f240338e85c3
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
src/Controls/src/Xaml/PublicAPI/net/PublicAPI.Unshipped.txt:9
- PublicAPI.Unshipped entries should be kept in the same (case-insensitive) alphabetical order used in PublicAPI.Shipped for consistency and to reduce merge churn. In the shipped net TFM file,
iOSsorts betweenDefaultandMacCatalyst, indicating the sort is not ordinal-case-sensitive; heremacOSshould come beforeWPF.
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.macOS.get -> object
src/Controls/src/Xaml/PublicAPI/netstandard/PublicAPI.Unshipped.txt:9
- PublicAPI.Unshipped entries should be kept in the same (case-insensitive) alphabetical order used in PublicAPI.Shipped for consistency and to reduce merge churn;
macOSshould come beforeWPF.
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.macOS.get -> object
src/Controls/src/Xaml/PublicAPI/net-android/PublicAPI.Unshipped.txt:9
- PublicAPI.Unshipped entries should be kept sorted consistently (case-insensitive alphabetical, as in PublicAPI.Shipped);
macOSshould come beforeWPF.
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.macOS.get -> object
src/Controls/src/Xaml/PublicAPI/net-ios/PublicAPI.Unshipped.txt:9
- PublicAPI.Unshipped entries should be kept sorted consistently (case-insensitive alphabetical, as in PublicAPI.Shipped);
macOSshould come beforeWPF.
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.macOS.get -> object
src/Controls/src/Xaml/PublicAPI/net-windows/PublicAPI.Unshipped.txt:9
- PublicAPI.Unshipped entries should be kept sorted consistently (case-insensitive alphabetical, as in PublicAPI.Shipped);
macOSshould come beforeWPF.
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.macOS.get -> object
src/Controls/src/Xaml/PublicAPI/net-tizen/PublicAPI.Unshipped.txt:9
- PublicAPI.Unshipped entries should be kept sorted consistently (case-insensitive alphabetical, as in PublicAPI.Shipped);
macOSshould come beforeWPF.
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.macOS.get -> object
src/Controls/src/Xaml/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt:9
- PublicAPI.Unshipped entries should be kept sorted consistently (case-insensitive alphabetical, as in PublicAPI.Shipped);
macOSshould come beforeWPF.
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.GTK.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.get -> object
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.WPF.set -> void
~Microsoft.Maui.Controls.Xaml.OnPlatformExtension.macOS.get -> object
|
Current-head CI evidence (builds still active): device build 1531518 currently has three failed legs — MacCatalyst CoreCLR, iOS Mono, and iOS CoreCLR. The current |
|
Updated current-head classification: device build 1531518 is complete and its six failed platform jobs exactly match current |
|
Final main 1531511 classification: the only failed job is |
|
/azp run maui-pr |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Final UI 1531514 classification: CollectionView, API30 CollectionView, Material3 API36, and API30 Shell match failed jobs in current |
|
/azp run maui-pr-uitests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Final main replacement 1531711 classification: |
|
/azp run maui-pr |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
Fixes #35695 through its documented minimum option 3 and is part of #34099.
OnPlatformExtensionalready had runtime matching branches forGTK,macOS, andWPF, but those properties were internal. External/community backends therefore could not use them from the inline{OnPlatform ...}markup-extension form.This PR:
OnPlatformExtension.GTK,.macOS, and.WPFpublic;OnPlatform<T>element form remains the route for arbitrary platform strings such asWeb;LoadFromXaml, and non-simplified SourceGen coverage for GTK, macOS, and WPF.The existing
DevicePlatformmatching implementation is unchanged.OnIdiomExtensionis unchanged. This keeps the PR focused on the actual missing public surface rather than behavior-neutral rewrites.API Changes
Tests
SimplifyOnPlatformSourceGen tests pass 8/8, includingnetstandard2.0code generation that createsOnPlatformExtension, assigns GTK/macOS/WPF, and callsProvideValue.Issues Fixed
Fixes #35695
Part of #34099
Merge note
When squash-merging, use this PR title/body rather than concatenating the historical commit messages, which describe superseded matcher/OnIdiom experiments no longer present in the final diff.