Skip to content

[Extensibility] XAML: Expose GTK, macOS, and WPF in OnPlatform - #35901

Merged
kubaflo merged 19 commits into
dotnet:net11.0from
kubaflo:fix/35695-onplatform-onidiom-data-driven
Jul 30, 2026
Merged

[Extensibility] XAML: Expose GTK, macOS, and WPF in OnPlatform#35901
kubaflo merged 19 commits into
dotnet:net11.0from
kubaflo:fix/35695-onplatform-onidiom-data-driven

Conversation

@kubaflo

@kubaflo kubaflo commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

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.

OnPlatformExtension already had runtime matching branches for GTK, macOS, and WPF, but those properties were internal. External/community backends therefore could not use them from the inline {OnPlatform ...} markup-extension form.

This PR:

  • makes OnPlatformExtension.GTK, .macOS, and .WPF public;
  • adds XML documentation explaining that inline markup supports the case-sensitive named properties, while the OnPlatform<T> element form remains the route for arbitrary platform strings such as Web;
  • adds the corresponding PublicAPI entries across all seven XAML TFMs;
  • adds parser, runtime LoadFromXaml, and non-simplified SourceGen coverage for GTK, macOS, and WPF.

The existing DevicePlatform matching implementation is unchanged. OnIdiomExtension is unchanged. This keeps the PR focused on the actual missing public surface rather than behavior-neutral rewrites.

API Changes

public object GTK { get; set; }
public object macOS { get; set; }
public object WPF { get; set; }

Tests

  • SimplifyOnPlatform SourceGen tests pass 8/8, including netstandard2.0 code generation that creates OnPlatformExtension, assigns GTK/macOS/WPF, and calls ProvideValue.
  • Runtime XAML coverage passes for GTK, macOS, and WPF.
  • Parser coverage verifies all three inline named arguments.
  • The regression gate proves the focused tests fail without the public surface and pass with the fix.

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.

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>
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35901

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35901"
@github-actions github-actions Bot added the area-xaml XAML, CSS, Triggers, Behaviors label Jun 12, 2026
@kubaflo

kubaflo commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).
@kubaflo

kubaflo commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Multimodal code review summary

I 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)

  • UWP/WinUI precedence preserved exactly. Opus exhaustively simulated the old if‑chain vs. the new data‑driven lookup across all 2⁹ property combinations × all platform strings (11,264 cases) and found 0 behavioral differences for real (exact‑case) platform strings — including the regression‑critical "both UWP and WinUI set, platform = WinUI ⇒ WinUI" case (covered by a test).
  • OnIdiom null sentinel is equivalent to the old Idiom ?? Default.
  • Thread‑safety, public API entries (~‑prefixed, present in all 7 TFM folders, correct GTK/macOS/WPF casing), and test coverage all clean.

🔧 Finding to address — case sensitivity (GPT‑5.5: Major, Opus: Minor)

The old chain compared via DevicePlatform/DeviceIdiom equality, which is Ordinal (case‑sensitive) (DevicePlatform.shared.cs). The new lookup used StringComparer.OrdinalIgnoreCase — the only behavioral delta (Opus: 3,328 differing cases, all attributable to casing). All three models also flagged that the code/test comments justifying this as "case‑insensitive, like the element form" are inaccurate: the element form (OnPlatform<T>IList<string>.Contains) and the compile‑time SimplifyOnPlatformVisitor (split.Trim() == target) are both case‑sensitive. Making only the runtime markup‑extension path case‑insensitive introduces a runtime/compile‑time inconsistency.

➡️ Fix: switch both lookups to StringComparer.Ordinal for exact behavioral parity and consistency with the element form + compile‑time path, and correct the comments/tests.

📝 Considered — per‑call dictionary allocation (Gemini: Critical; GPT‑5.5 & Opus: acceptable)

Gemini flagged the Dictionary allocated on each ProvideValue. GPT‑5.5 and Opus both assessed it as negligible: ProvideValue runs once per XAML inflation (not per frame), only for Windows/Tizen/runtime‑loaded XAML — android/ios/macos/maccatalyst compiled XAML is resolved at compile time by SimplifyOnPlatformVisitor and never calls it — and a ~9‑entry dictionary is dwarfed by the reflection/ConvertTo already in the method. #35695 also explicitly requests a "data‑driven lookup … the same dictionary", so I'm keeping the dictionary.

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 kubaflo left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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.Ordinal restores exact parity with the old DevicePlatform/DeviceIdiom (Ordinal) comparison and is consistent with the element form + compile-time SimplifyOnPlatformVisitor.
  • 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 7 PublicAPI.Unshipped.txt entries correct; no new issues.
  • 📝 Dictionary allocation (Gemini round-1 Critical): both round-2 reviewers confirm it's acceptable — ProvideValue runs 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. 👇

Comment thread src/Controls/src/Xaml/MarkupExtensions/OnPlatformExtension.cs Outdated
Comment thread src/Controls/src/Xaml/MarkupExtensions/OnPlatformExtension.cs Outdated
Comment thread src/Controls/src/Xaml/MarkupExtensions/OnIdiomExtension.cs Outdated
@kubaflo

kubaflo commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator Author

Automated fleet code review — PR #35901

Reviewed head SHA: e328453687977d27102d1eb6b60941eef0396c93

Verdict: NEEDS_DISCUSSION
Confidence: medium

Independent assessment

The code replaces hardcoded DevicePlatform/DeviceIdiom if chains in {OnPlatform} / {OnIdiom} markup extensions with string-keyed lookups using StringComparer.Ordinal, makes the existing GTK, macOS, and WPF OnPlatformExtension properties public, and preserves UWP as a WinUI fallback only when WinUI is not explicitly set. This matches the existing exact-case DevicePlatform/DeviceIdiom equality semantics and the compile-time SimplifyOnPlatformVisitor behavior.

Findings

❌ Errors

None found in the changed code.

⚠️ Warnings

  • The PR description is stale after the case-sensitivity fix: it still says matching is "case-insensitive" and says tests cover case-insensitive matching. The implementation and tests now intentionally use exact Ordinal matching, which looks correct, but the narrative should be updated before merge.
  • Required CI is red. I do not see a direct correlation between the failures and these XAML markup-extension changes, but Build Analysis reports them as unmatched, so this cannot be treated as green/known-infra from the available evidence.

💡 Suggestions

  • Consider tightening the OnIdiomExtension.GetValue() comment. The lookup is data-driven internally, but {OnIdiom ...} still only exposes the fixed built-in property names (Phone, Tablet, Desktop, TV, Watch); a truly custom idiom name still cannot be expressed in the markup-extension form and falls back to Default.

CI note

gh pr checks shows required maui-pr and Build Analysis failing for build 1462356. Failed areas include RunOniOS_MauiRelease*, AOT macOS, and Build windows; logs show integration/template build failures such as AppleTemplateTests.RunOniOS_MauiReleaseTrimFull*, AOTTemplateTest.PublishNativeAOTRootAllMauiAssemblies, ResizetizerTests.CollectsAssets, plus a Provision JDK failure. Unit-test and pack/build legs for the touched XAML code passed, but the required pipeline remains red.

Devil's advocate

The main behavior-risk I checked was parity with the old branch order and sentinel semantics: UWP/WinUI precedence is preserved, explicit Default handling still distinguishes unset from set-to-null for OnPlatformExtension, and OnIdiomExtension preserves the prior null-as-unset behavior. The new dictionary allocation is on XAML inflation, not a hot layout/render path.

This is an automated non-approval review and not a substitute for human review.

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 kubaflo left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 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 emits HybridWebViewHandler IL2026/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:

  1. SimplifyOnPlatformVisitor only compile-simplifies android/ios/macos/maccatalyst → {OnPlatform}/{OnIdiom} do run at runtime on Windows/Tizen (incl. the DataTemplate inflation path).
  2. MarkupExtensionParser only 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.

Comment thread src/Controls/src/Xaml/MarkupExtensions/OnPlatformExtension.cs Outdated
@kubaflo

kubaflo commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator Author

Automated fleet code review — PR #35901

Reviewed head SHA: 7a5977777d72f72f727a2cbb9575c8a78dad9491

Verdict: NEEDS_DISCUSSION
Confidence: high for code correctness; medium for merge readiness because required CI is red with unmatched failures.

Independent assessment

The new commit replaces the prior per-call lookup/dictionary approach with a zero-allocation string-keyed if-chain. The behavior remains exact-case (StringComparison.Ordinal) and preserves first-party platform/idiom behavior, Default fallback semantics, and WinUI/UWP precedence. Making GTK, macOS, and WPF public is reflected in all relevant XAML PublicAPI.Unshipped.txt files, and the added parser/LoadFromXaml tests cover the new publicly-settable platform properties and fallback behavior.

Reconciliation with prior review / PR narrative

Round22'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 android matching Android, while the code and tests now intentionally assert exact-case matching and fallback to Default for android/gtk. The narrative also still overstates arbitrary custom platform/idiom support: the markup-extension form can now express the newly-public predefined GTK/macOS/WPF keys, but it still cannot express an arbitrary Web/Linux/custom idiom value except via Default.

Findings by severity

❌ Errors

None found in the changed code.

⚠️ Warnings

  • PR description/test summary must be updated before merge to match the current implementation: matching is case-sensitive/Ordinal, not case-insensitive.
  • Required maui-pr / Build Analysis are currently failing with unmatched failures, so I would not treat this as merge-ready from CI alone.

💡 Suggestions

  • Consider tightening the OnIdiomExtension.GetValue() comment: the refactor is behavior-preserving, but {OnIdiom ...} still exposes only the fixed built-in idiom properties, so a truly custom idiom still falls back to Default.

CI note

gh pr checks 35901 reports required maui-pr and Build Analysis failing for build 1462790. Build Analysis reports no known issue matches and unmatched failures in AOT macOS, RunOniOS_MauiRelease, RunOniOS_MauiReleaseTrimFull, and RunOniOS_MauiReleaseTrimFull_CoreCLR. Evidence includes AOTTemplateTest.PublishNativeAOTRootAllMauiAssemblies failing on an unexpected ILC ... apply-preserve-attribute.xml warning file, plus AppleTemplateTests.RunOniOS_MauiReleaseTrimFull* template project build failures. The touched XAML unit-test/build legs appear green, and I do not see a direct XAML-markup correlation, but the required pipeline is still red/unmatched.

Devil's advocate

I specifically re-checked sentinel behavior (s_notset vs null), OnIdiom prior null-as-unset semantics, exact DevicePlatform/DeviceIdiom.ToString() names, UWP/WinUI fallback ordering, and compile-time SimplifyOnPlatformVisitor parity. The implementation looks sound; the remaining concerns are documentation/narrative accuracy and unresolved CI status.

This is an automated non-approval review and not a substitute for human review.

@kubaflo

kubaflo commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator Author

✅ Final multimodal merge-readiness review — unanimous MERGEABLE

Ran the full fix → review loop with four top models on the current head (7a5977777d):

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 exactlyWinUI wins over UWP; UWP still matches both the WinUI platform and a legacy "UWP" string. (Including the trap that DevicePlatform.UWP's internal string is actually "WinUI".)
  • Ordinal is the correct comparison — it matches DevicePlatform/DeviceIdiom.Equals (which use StringComparison.Ordinal), the runtime element form (List<string>.Contains on DeviceInfo.Platform.ToString()), and the compile‑time SimplifyOnPlatformVisitor (case‑sensitive ==). Case‑variant inputs correctly fall through to Default — covered by new "android"/"gtk"/"phone" tests.
  • OnIdiom null‑sentinel maps exactly to the old ?? Default coalescing; matching is allocation‑free (ToString() returns the backing string field; the Dictionary from an earlier round was replaced by a zero‑alloc if‑chain).
  • Public GTK/macOS/WPF is justified, not speculative — these are functional value properties wired into TryGetValueForPlatform, sitting alongside already‑public Android/iOS/MacCatalyst/WinUI/UWP, enabling community backends via {OnPlatform GTK=…}; new tests demonstrate real resolution. PublicAPI.Unshipped.txt is 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.

@kubaflo

kubaflo commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

@StephaneDelcroix — design question before we take this further. cc @PureWeen

Context. This PR makes the {OnPlatform} / {OnIdiom} markup extensions resolve their value by string-comparing DeviceInfo.Platform.ToString() against the per-platform keys (Ordinal, matching DevicePlatform equality and SimplifyOnPlatformVisitor), and it makes the existing GTK / macOS / WPF properties public so more first-party-ish backends can be expressed.

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 (Android, iOS, macOS, MacCatalyst, Tizen, WinUI, GTK, WPF, UWP). For a brand-new backend — say a Web head that registers an IDeviceInfo/DevicePlatform returning "Web" — there is no Web property to set, so {OnPlatform Web=...} can't be authored. The PR's own test confirms this: an unknown "Web" platform falls back to Default.

The element form already supports arbitrary platforms todayOn.Platform is an IList<string> matched via Contains(DeviceInfo.Platform.ToString()), so a custom head can do:

<Label.Text>
  <OnPlatform x:TypeArguments="x:String">
    <On Platform="Web" Value="..." />
  </OnPlatform>
</Label.Text>

So the gap is specifically the attribute markup-extension syntax {OnPlatform Web=...}, where, as far as I can tell, the XAML parser maps attributes to CLR properties and there's no hook to accept arbitrary keys.

Questions:

  1. Is opening up the predefined property surface (this PR) the intended ceiling for the markup-extension form, with the element form as the documented escape hatch for truly-custom platforms/idioms?
  2. Or do we want the markup extension to support arbitrary keys — e.g. a dictionary-style member or a parser hook so {OnPlatform Web=...; iOS=...} works for any registered DevicePlatform? If so, is there an existing XAML mechanism you'd point me at, or does this need new parser support?

Same question applies to {OnIdiom} for custom DeviceIdiom.Create(...) values. Happy to implement whichever direction you prefer — just want the design decision before building a speculative API.

@kubaflo
kubaflo requested a review from StephaneDelcroix June 16, 2026 13:28
@kubaflo

kubaflo commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

@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 Web that registers an IDeviceInfo/DevicePlatform returning "Web") author {OnPlatform Web=..., Default=...} — not just the hardcoded Android/iOS/WinUI/… property surface on OnPlatformExtension/OnIdiomExtension.

Where the hardcoded surface bites:

  1. Runtime (LoadFromXaml)MarkupExtensionParser.SetPropertyValue resolves attributes via GetRuntimeProperty(prop).SetMethod; an unknown Web returns null → NRE.
  2. Compiled, android/ios/macos/maccatalystSimplifyOnPlatformVisitor matches the target by string in node.Properties and replaces the node before any CLR-property check, so arbitrary keys would already survive here.
  3. Compiled, Windows/Tizen/WebTarget is null, no simplification, and SetPropertiesVisitor.SetPropertyValue throws BuildException(MemberResolution) (SetPropertiesVisitor.cs:1281) on the unknown Web member.

So the attribute form is blocked on paths (1) and (3); the element form <On Platform="Web" Value=.../> already works everywhere (On.Platform is IList<string>).

Proposed design (opt-in, scoped to these two extensions):

interface IAcceptArbitraryKeys { bool TrySetKey(string key, object value, IServiceProvider sp); }

OnPlatformExtension/OnIdiomExtension back it with a Dictionary<string,object>; ProvideValue looks up DeviceInfo.Platform.ToString() (resp. idiom) there, merged with the existing typed props for back-compat. Integration points:

  • Runtime: in MarkupExtensionParser.SetPropertyValue, when GetRuntimeProperty(prop) == null and the extension implements the interface, route into TrySetKey instead of NRE-ing.
  • XamlC: at SetPropertiesVisitor.cs:1281, before throwing MemberResolution, if the parent type implements the interface emit a call to TrySetKey(key, value). This IL emit is the only genuinely new/risky piece.
  • SimplifyOnPlatformVisitor: unchanged (already string-keyed; only simplifies the 4 known TFMs).

Questions for you:

  1. Are you comfortable with the XamlC SetPropertiesVisitor IL-emit hook, or would you rather keep the compiler strict and steer custom backends to the element form as the documented path?
  2. If we add the dict, do you want unknown keys allowed on any markup extension that opts in, or hard-restricted to OnPlatform/OnIdiom?
  3. Case sensitivity / collision rules when a key matches both a typed prop and a dict entry — typed prop wins?

Happy to implement whichever direction you prefer on fix/35695-onplatform-onidiom-data-driven.

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Review — 1 findings

See inline comments for details.

Comment thread src/Controls/src/Xaml/MarkupExtensions/OnPlatformExtension.cs Outdated
@MauiBot MauiBot added s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Jun 18, 2026
@kubaflo

This comment has been minimized.

@kubaflo

kubaflo commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

@kubaflo

kubaflo commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Updated-head local validation on 87941ab832fe: focused runtime/parser OnPlatform coverage passes 26/26, and SimplifyOnPlatform SourceGen coverage passes 8/8.

@kubaflo

kubaflo commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Fresh main build 1530348 fails only AppThemeTests.ParentSetResolvesAppThemeDynamicResourceWithoutListener in the Controls.Core work item on Windows/macOS Helix. The assertion (Expected: Light, Actual: Unspecified) is identical to exact-base build 1530284 and is tracked by #36794 / fix PR #36885. This is outside the XAML OnPlatform changes; no rerun is useful until #36885 lands.

@PureWeen PureWeen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) is string.Equals(devicePlatform, other, StringComparison.Ordinal), and operator == (line 123) delegates to it. So the matcher's DeviceInfo.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 the GTK property and falls through to Default.
  • The <see cref="Default"/> and DevicePlatform cref 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).

@kubaflo

kubaflo commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

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 Fragment.OnDestroyView SIGSEGV documented on #36885. This is unrelated to XAML OnPlatform; no rerun adds signal.

@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

UI build 1530352 has one isolated failure: ButtonsLayoutResolveWhenParentSizeChanges, a 1.85% Android API 30 screenshot mismatch (198/202 passed, 3 skipped). The identical Border,BoxView,Brush,Button shard passes on concurrent updated-base runs #1530396 and #1530465. This is a confirmed run-specific visual baseline flake outside XAML OnPlatform; rerunning before #36885 lands would only repeat the known main/device base failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 66f84348-6476-4097-8b7f-f240338e85c3
@kubaflo

This comment has been minimized.

@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, iOS sorts between Default and MacCatalyst, indicating the sort is not ordinal-case-sensitive; here macOS should come before WPF.
~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; macOS should come before WPF.
~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); macOS should come before WPF.
~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); macOS should come before WPF.
~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); macOS should come before WPF.
~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); macOS should come before WPF.
~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); macOS should come before WPF.
~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
@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Current-head CI evidence (builds still active): device build 1531518 currently has three failed legs — MacCatalyst CoreCLR, iOS Mono, and iOS CoreCLR. The current net11.0 baseline device build 1531485 has the exact same three failed job names. This is current-base evidence that these device reds are unrelated to the XAML changes; remaining jobs are still pending.

@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Updated current-head classification: device build 1531518 is complete and its six failed platform jobs exactly match current net11.0 baseline 1531485. UI build 1531514 has one Windows visual failure, Issue18896Test (7.09%). That page does not use OnPlatform/OnIdiom; it loads remote monkey images, and the same test has documented remote-image screenshot flakiness in #33507. Classification: unrelated UI flake; the UI build remains active and is a rerun candidate after completion.

@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Final main 1531511 classification: the only failed job is AOT windows. Azure abandoned the agent during the AOT integration run and emitted The Operation will be canceled. The next steps may not contain expected logs, matching known dnceng infrastructure issue #3008; no test failure was reported. A main-only replacement run is justified.

@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

/azp run maui-pr

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Final UI 1531514 classification: CollectionView, API30 CollectionView, Material3 API36, and API30 Shell match failed jobs in current net11.0 baseline 1531376; the Editor/Effects group failed concurrently across unrelated PRs; Issue18896Test is the documented remote-image visual flake from #33507. None correlate with the OnPlatform API-accessibility diff. A UI-only replacement run is justified.

@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

/azp run maui-pr-uitests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Final main replacement 1531711 classification: RunOniOS_MauiRelease_CoreCLR ARM64 was canceled after the agent exceeded its 45-minute limit. No TRX or product failure was produced; the only failed tasks were the downstream missing-results/log publishers. This is infrastructure and unrelated to the XAML API-accessibility change. Another main-only run is justified.

@kubaflo

kubaflo commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

/azp run maui-pr

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-xaml XAML, CSS, Triggers, Behaviors p/0 Current heighest priority issues that we are targeting for a release. s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review)

5 participants