[iOS, MacCatalyst] Fix Image and ImageButton BackgroundColor does not reset when set to null - #36427
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36427Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36427" |
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
… usable When a gate agent has 0 iOS runtime images on disk (build 14699070, #27153), or images present but all 'Invalid runtime' uncreatable even after a CoreSimulatorService restart + 'simctl runtime add' enroll (build 14690025, #36427), the gate boot dead-ended at 'No iPhone simulator found' -> the iOS gate degraded to INCONCLUSIVE. The DEEP stage recovers via its 'Install iOS simulator runtimes' step (xcodebuild -downloadPlatform iOS -buildVersion $SDK_VER), but the GATE stage (ReviewPR/CopilotReview) has no such step and boots solely via Start-Emulator.ps1, which only re-enrolled EXISTING on-disk images — so a runtime-less agent could never run the iOS gate. Add Invoke-IosRuntimeDownload (mirrors the deep stage: select newest Xcode -> probe iphonesimulator SDK version -> xcodebuild -downloadPlatform iOS -buildVersion <SDK>; sudo -n with generic + non-sudo fallbacks) and invoke it as the FINAL boot recovery, after the rescue + CoreSimulatorService restart + enroll passes all fail to produce a bootable device; then enroll + retry the create loop once. Reached only after every cheaper recovery failed, so a healthy or merely-slow boot never pays the multi-GB/minutes download. Gate task timeout is 150 min — ample headroom. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15d2af20-e4ab-4e88-9011-cfbd83513bc0
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
| if (imageButton.Background is ImageSourcePaint imagePaint) | ||
| { | ||
| var provider = handler.GetRequiredService<IImageSourceServiceProvider>(); | ||
| platformView.UpdateBackgroundImageSourceAsync(imagePaint.ImageSource, provider) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Async and Threading Safety — This fire-and-forget image-background load is not ordered against later Background changes. Concrete scenario: set an ImageButton.Background to an ImageSourcePaint backed by a slow stream/URI, then clear it (BackgroundColor = null) before GetImageAsync completes; the null branch removes the layer, but this awaited continuation can still insert the old image layer afterward because UpdateBackgroundImageSourceAsync has no generation/current-background/connected check. The same pattern was added in ImageHandler.iOS.cs:32 (visible when the image keeps a container, e.g. Clip/Shadow), so please guard async completions or centralize versioning before inserting the layer.
| #endif | ||
| [nameof(IImage.Aspect)] = MapAspect, | ||
| [nameof(IImage.IsAnimationPlaying)] = MapIsAnimationPlaying, | ||
| #if __ANDROID__ || __IOS__ || MACCATALYST || WINDOWS || TIZEN |
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the suggestions?
…no premature INCONCLUSIVE degrade) The maui-copilot gate/deep iOS runtime install (eng/pipelines/common/provision.yml "Install Simulator Runtimes") gave up after a fixed 3 download attempts and then only WARNED, letting the iOS sim boot self-degrade the gate to INCONCLUSIVE — the exact "iOS sim provisioning: 0 runtimes enrolled / 0 images on disk; retried 3x, degraded gracefully" failure seen on PR #36427/#36577. Directive: iOS must actually work — keep trying as long as it works. Fix: replace the fixed `for smokeAttempt in 1 2` give-up loop with a persistent restart+purge+re-download+smoke-test loop that keeps going until a runtime is provably create-usable, bounded by BOTH an attempt cap (8) AND a wall-clock budget (installBudgetSeconds=2100 ≈ 35 min) kept safely under the step timeout so a retry can never be killed mid-download (which would leave a half-staged image). Escalate to the nuclear on-disk image-store purge from the 2nd attempt on — covering the ghost-collision / drain-succeeds-but-still-wedged case a conditional purge alone missed. Only the already-failed retry branch is affected; a healthy agent installs on the first download (~7 min) and exits untouched. Bump the step timeout 30 -> 45 min so the extra retries fit (only consumed on the failing-agent path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15d2af20-e4ab-4e88-9011-cfbd83513bc0
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@devanathan-vaithiyanathan — new AI review results are available based on this last commit:
478e807. To request a fresh review after new comments or commits, comment/review rerun.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ✅ PASSED
Platform: IOS · Base: main · Merge base: a4ede427
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🖥️ Issue36302 Issue36302 |
✅ FAIL — 301s | ✅ PASS — 108s |
🔴 Without fix — 🖥️ Issue36302: FAIL ✅ · 301s
Error-relevant lines (filtered from the build log):
at VisualTestUtils.VisualRegressionTester.Fail(String message) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 162
at VisualTestUtils.VisualRegressionTester.VerifyMatchesSnapshot(String name, ImageSnapshot actualImage, String environmentName, ITestContext testContext) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 123
at Microsoft.Maui.TestCases.Tests.UITest.<VerifyScreenshot>g__Verify|13_0(String name, <>c__DisplayClass13_0&) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 477
at Microsoft.Maui.TestCases.Tests.UITest.VerifyScreenshot(String name, Nullable`1 retryDelay, Nullable`1 retryTimeout, Int32 cropLeft, Int32 cropRight, Int32 cropTop, Int32 cropBottom, Double tolerance) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 309
at Microsoft.Maui.TestCases.Tests.Issues.Issue36302.ImageAndImageButtonBackgroundClearsWhenSetToNull() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36302.cs:line 18
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
🟢 With fix — 🖥️ Issue36302: PASS ✅ · 108s
(no coded error found; showing last 1200 chars)
...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.05] Discovering: Controls.TestCases.iOS.Tests
[xUnit.net 00:00:00.19] Discovered: Controls.TestCases.iOS.Tests
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll
NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 7/19/2026 2:11:33 AM FixtureSetup for Issue36302(iOS)
>>>>> 7/19/2026 2:11:38 AM ImageAndImageButtonBackgroundClearsWhenSetToNull Start
>>>>> 7/19/2026 2:11:39 AM ImageAndImageButtonBackgroundClearsWhenSetToNull Stop
Passed ImageAndImageButtonBackgroundClearsWhenSetToNull [1 s]
NUnit Adapter 4.5.0.0: Test execution complete
Results File: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue36302.trx
Test Run Successful.
Total tests: 1
Passed: 1
Total time: 20.2446 Seconds
>>> TRX_RESULT_FILE: /Users/cloudtest/vss/_work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue36302.trx
📁 Fix files reverted (4 files)
src/Core/src/Handlers/Image/ImageHandler.cssrc/Core/src/Handlers/Image/ImageHandler.iOS.cssrc/Core/src/Handlers/ImageButton/ImageButtonHandler.cssrc/Core/src/Handlers/ImageButton/ImageButtonHandler.iOS.cs
📱 UI Tests — Image,ViewBaseTests
Detected UI test categories: Image,ViewBaseTests
✅ Deep UI tests — 158 passed, 0 failed across 2 categories on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
Image |
46/48 ✓ | — |
ViewBaseTests |
112/112 ✓ | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
📋 Pre-Flight — Context & Validation
Issue: #36302 - [iOS, MacCatalyst] Image and ImageButton BackgroundColor does not reset when set to null
PR: #36427 - [iOS, MacCatalyst] Fix Image and ImageButton BackgroundColor does not reset when set to null
Platforms Affected: iOS, MacCatalyst
Files Changed: 4 implementation, 6 test
Key Findings
- Issue #36302 reports that setting
BackgroundColor/BackgroundtonullonImageandImageButtonleaves the previous background visible on iOS and MacCatalyst. - PR #36427 maps
IImage.BackgroundandIImageButton.Backgroundon iOS/MacCatalyst, explicitly removing background layers and clearing native background color when the MAUI background is null. - Gate was already completed before this run: tests fail without the fix and pass with the PR fix. This phase did not re-run gate verification.
- Prior review concerns about public API exposure, missing ImageSourcePaint handling, Image opacity after container transitions, and iOS/Mac screenshot baselines appear addressed in the current diff.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 1
Key code review findings:
- ℹ
ImageHandler.iOS.cs:29/ImageButtonHandler.iOS.cs:58— background mapping logic is duplicated; a shared helper could reduce future divergence risk, but the current code appears correct.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36427 | Add iOS/MacCatalyst background mappers for Image and ImageButton; remove background layers and clear native background color when background is null; preserve image background sources and opacity/container behavior. |
✅ PASSED (Gate) | ImageHandler.cs, ImageHandler.iOS.cs, ImageButtonHandler.cs, ImageButtonHandler.iOS.cs, UI test files/snapshots |
Original PR fix. |
🔬 Code Review — Deep Analysis
Code Review — PR #36427
Independent Assessment
What this changes: Adds iOS/MacCatalyst-specific Background mapping for Image/ImageButton so setting background to null removes previous native background state; adds screenshot UI coverage.
Inferred motivation: Fix stale background color after BackgroundColor = null on iOS/MacCatalyst.
Reconciliation with PR Narrative
Author claims: Fixes #36302 by clearing background layers/colors for Image and ImageButton.
Agreement/disagreement: Matches the implementation and linked issue.
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
| Image opacity not reapplied after container removal | MauiBot | ✅ Fixed | ImageHandler.iOS.cs:49 now calls handler.UpdateValue(nameof(IView.Opacity)). |
Other prior major findings were checked: public API issue fixed by internal; NETSTANDARD guard fixed; ImageSourcePaint branches preserved; iOS/Mac baselines now present.
Blast Radius Assessment
- Runs for all instances: Yes, all iOS/MacCatalyst
Image.BackgroundandImageButton.Backgroundmappings. - Startup impact: No; property mapper runs during handler/property updates.
- Static/shared state: No new static mutable state.
CI Status
- Required-check result:
gh pr checks --requiredunavailable due missing GitHub auth. - Classification: Undetermined required-check enumeration. Public check-runs for head SHA show
maui-prand related checks completed successfully. - Action taken: Capped confidence low per skill rules; no GitHub comments posted.
Findings
💡 Suggestion — Consider centralizing duplicated iOS background mapping
ImageHandler.iOS.cs:29 and ImageButtonHandler.iOS.cs:58 duplicate ImageSourcePaint/null/generic background handling. This is currently correct, but a shared helper would reduce future divergence risk.
Failure-Mode Probing
- Null background after colored background: native layer removed and background color cleared.
- Image with container transition: opacity is remapped after container update.
- ImageButton container behavior: code targets
handler.PlatformView, avoiding wrapper/background mismatch. - Handler disconnect: no new subscriptions or static state.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: Code changes look sound, and prior blocking review findings appear addressed. However, required CI status could not be verified via gh pr checks --required, so the skill rules prevent an LGTM.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Centralize only iOS null-background clearing in an internal UIView.ClearBackground(UIColor?) helper, while preserving PR handler targets. |
✅ PASS | 3 files | Reduces duplicate clear code and self-review was clean, but remains close to the PR's handler-branching strategy. |
| 2 | try-fix | Broaden ViewExtensions.UpdateBackground(Paint?) null clearing and simplify handlers to delegate to existing platform helpers. |
✅ PASS (rejected by self-review) | 3 files | Narrow test passed, but self-review found major regressions: ImageSourcePaint backgrounds would no longer be applied. |
| 3 | try-fix | Centralize the full iOS paint policy in an internal platform overload covering ImageSourcePaint, null clearing, and solid/gradient paints. | ✅ PASS | 3 files | Best alternative: removes duplicated handler decision trees while preserving ImageSourcePaint and correct ImageButton native target. |
| PR | PR #36427 | Add iOS/MacCatalyst background mappers for Image and ImageButton; remove background layers and clear native background color when background is null; preserve image background sources and opacity/container behavior. |
✅ PASSED (Gate) | 4 implementation files + UI tests/snapshots | Original PR fix. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Suggested centralizing iOS clear semantics while preserving handler-specific native targets. This became try-fix-1. |
| maui-expert-reviewer | 2 | Yes | After try-fix-2 lost ImageSourcePaint handling, suggested a platform overload that owns the full paint decision. This became try-fix-3. |
| maui-expert-reviewer | 3 | No | After try-fix-3, remaining ideas were either more complex async-versioning not justified by the issue/test or trivial variations of the full-policy helper. |
Exhausted: Yes
Selected Fix: Candidate #3 — It passes the iOS regression test and is demonstrably cleaner than the PR's duplicated handler branches while avoiding try-fix-2's ImageSourcePaint regression. The PR's current fix also passes and is correct, but Candidate #3 is the best alternative found.
Environment Notes
EstablishBrokenBaseline.ps1could not be used because the shared worktree already contained unrelated uncommitted changes outside PR #36427. To avoid reverting those changes, candidates were tested as deltas against the current PR fix.- Gate verification was not re-run; the supplied gate result was used as requested.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current metadata accurately describes the raw PR, but the winning pr-plus-reviewer fix centralizes the iOS paint policy in ViewExtensions, so the description should reflect that implementation detail while preserving the issue link, platform notes, and tested-platform details.
Recommended title
[iOS, MacCatalyst] Image/ImageButton: Clear null BackgroundColor
Recommended description
### Issue details
Setting `BackgroundColor` to `null` on `Image` and `ImageButton` controls does not reset/clear the background color on iOS and macOS (MacCatalyst). The previously set background color persists visually even though the property value reports `null`.
### Description of Change
Adds iOS/MacCatalyst background mapping for `Image` and `ImageButton` so null/empty backgrounds remove the MAUI background layer and reset the native background state.
The winning implementation centralizes the iOS paint policy in `ViewExtensions` so `Image` and `ImageButton` share the same handling for:
- `ImageSourcePaint` backgrounds via `UpdateBackgroundImageSourceAsync(...)`
- null/empty backgrounds via `RemoveBackgroundLayer()` plus control-specific native reset (`null` for `Image`, `UIColor.Clear` for `ImageButton`)
- normal solid/gradient paint backgrounds via the existing platform update path
`ImageHandler.iOS.cs` also remaps opacity after the background/container transition so Image opacity remains correct when the wrapper is added or removed.
### Issues Fixed
Fixes #36302
**Tested the behavior in the following platforms.**
- [x] Android
- [x] Windows
- [x] iOS
- [x] Mac
| Before | After |
|---------|--------|
| **iOS**<br> <video src="https://github.com/user-attachments/assets/eac5d0f7-181b-4c9e-b5e6-87f7445c2ed7" width="300" height="600"> | **iOS**<br> <video src="https://github.com/user-attachments/assets/7964d7a7-0b4d-4472-b839-f91ec61bd4cf" width="300" height="600"> |
🏁 Report — Final Recommendation
Comparative Fix Report — PR #36427
Candidate Ranking
| Rank | Candidate | Regression result | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
✅ PASS by equivalence to try-fix-3 targeted iOS run |
Best choice. It keeps the PR's behavior and all important fixes, then applies the main expert-review maintainability feedback by centralizing the duplicated iOS paint-policy decision tree. |
| 2 | try-fix-3 |
✅ PASS | Technically equivalent to pr-plus-reviewer: centralizes full iOS paint policy, preserves ImageSourcePaint, preserves Image opacity remapping, and keeps ImageButton targeted at UIButton. Ranked just below because the named pr-plus-reviewer candidate represents applying the same improvement to the PR path. |
| 3 | pr |
✅ PASSED supplied gate | Correct and safe enough functionally: fixes null background clearing, preserves ImageSourcePaint, fixes Image opacity after container transitions, and targets ImageButton's UIButton. Ranked below the centralized candidates because it duplicates background paint-policy logic in two handlers. |
| 4 | try-fix-1 |
✅ PASS | Smaller cleanup that centralizes only the clear operation. It reduces duplicate null-clearing code but leaves the duplicated full paint decision trees in both handlers. |
| 5 | try-fix-2 |
✅ targeted PASS, but rejected by self-review | Must rank lower despite targeted pass because it regresses ImageSourcePaint backgrounds by delegating to helpers that do not handle image-source paints for these controls. |
Candidate Details
pr
The submitted PR adds iOS/MacCatalyst mapper entries for Image and ImageButton backgrounds and implements handler-specific MapBackground logic. It fixes the reported null-clear behavior and preserves important edge cases added during review: ImageSourcePaint, Image opacity remapping after container transitions, ImageButton targeting of handler.PlatformView, and UI screenshot baselines.
Remaining downside: the same ImageSourcePaint / null / normal-paint decision tree is duplicated in both iOS handlers.
pr-plus-reviewer
The sandbox-applied expert-review candidate centralizes the full iOS paint policy in an internal platform helper:
internal static void UpdateBackground(this UIView platformView, Paint? paint, UIColor? nullBackgroundColor, IElementHandler handler)Handlers pass their control-specific null-clear color (null for Image, UIColor.Clear for ImageButton) and retain their existing native target selection. This addresses the expert review's maintainability feedback without sacrificing correctness.
It does not change ImageButtonHandler.MapBackground from internal to public; that remains a deliberate public API follow-up for NET 11. It also does not add extra ImageSourcePaint UI coverage, which is a shared coverage gap across all viable candidates.
try-fix-1
Adds a helper for only the null-clear operation. This is a safe incremental improvement over pr, but it does not remove the duplicated full background policy.
try-fix-2
Broadens generic UIView.UpdateBackground(Paint?) null clearing and simplifies handlers. The targeted Issue36302 test passed, but self-review found it loses ImageSourcePaint support for Image/ImageButton backgrounds, so it is not viable.
try-fix-3
Centralizes the full iOS paint policy and passed the targeted iOS regression test. This is the best independent try-fix and is effectively the same implementation selected for pr-plus-reviewer.
Winner
pr-plus-reviewer wins. It has the same behavioral coverage as the PR and the same clean architecture as try-fix-3, while incorporating the expert-review paint-policy feedback directly into the PR fix path. The remaining expert findings (iOS ImageButton mapper public API symmetry and direct ImageSourcePaint test coverage) are follow-up concerns shared by all viable candidates, and no candidate that failed or regressed behavior is ranked above a passing candidate.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 4 findings
See inline comments for details.
| if (platformView is null) | ||
| return; | ||
|
|
||
| if (imageButton.Background is ImageSourcePaint imagePaint) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Handler Mapper and Property Patterns / Complexity Reduction — This re-implements the ImageSourcePaint FireAndForget branch and the generic-paint fallback that already exist in ViewHandler.MapBackground (src/Core/src/Handlers/View/ViewHandler.cs ~L381-397), instead of delegating for the non-empty cases. Since handler.PlatformView is the same target the base mapper already uses (unlike ImageHandler, which needs the container), this could be: if (imageButton.Background.IsNullOrEmpty()) { platformView.RemoveBackgroundLayer(); platformView.BackgroundColor = UIColor.Clear; } else { Microsoft.Maui.Handlers.ViewHandler.MapBackground(handler, imageButton); }. As written, two independent copies of the image-background-loading logic (Image and ImageButton) must now be kept in sync manually — if the shared async-loading path is later hardened (cancellation, disposal guards, new Paint types), this copy won't receive the fix.
| handler.ToPlatform().UpdateBackground(image); | ||
| var platformView = handler.ToPlatform(); | ||
|
|
||
| if (image.Background is ImageSourcePaint imagePaint) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[minor] Handler Mapper and Property Patterns — Same ImageSourcePaint async-loading branch is duplicated here from ViewHandler.MapBackground. Here it's harder to fully delegate since this method deliberately targets handler.ToPlatform() (container-aware) rather than handler.PlatformView, so full base-mapper reuse isn't a drop-in fix — but it does mean the same async-loading logic now exists in three places (base ViewHandler, ImageHandler.iOS, ImageButtonHandler.iOS) that must be kept behaviorally consistent by hand.
| } | ||
|
|
||
| //TODO : Make it public in NET 11. | ||
| internal static void MapBackground(IImageButtonHandler handler, IImageButton imageButton) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Public API Surface / Handler Mapper Patterns — This MapBackground is internal, but the equivalent method is public on Android (ImageButtonHandler.Android.cs) and Windows (ImageButtonHandler.Windows.cs). Calling ImageButtonHandler.MapBackground(handler, view) to chain default behavior when overriding the mapper via PrependToMapping/AppendToMapping (a documented extensibility pattern) will compile on Android/Windows but fail on iOS/MacCatalyst specifically. The //TODO: Make it public in NET 11 acknowledges this is deliberate (likely to avoid a mid-cycle public API addition), but it leaves a real cross-platform extensibility gap until then — worth tracking explicitly (e.g. a linked issue) so it isn't dropped.
| }; | ||
| clearButton.Clicked += (s, e) => | ||
| { | ||
| image.BackgroundColor = null; |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Regression Prevention and Test Coverage — This test (and the added handler code) only exercises the solid-Color-cleared-to-null scenario from the original report. The new ImageSourcePaint (image-as-background) branch added in ImageHandler.iOS.cs/ImageButtonHandler.iOS.cs MapBackground has no test coverage anywhere in the repo (no unit test or UI test sets Background to an image paint on iOS/MacCatalyst for Image/ImageButton). Consider adding a scenario that sets an image background then clears it, to guard the newly-added code path from regressing silently.
… reset when set to null (#36427) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue details Setting [BackgroundColor] to null on Image and ImageButton controls does not reset/clear the background color on iOS and macOS (MacCatalyst). The previously set background color persists visually even though the property value reports null. ### Description of Change <!-- Enter description of the fix in this section --> ImageHandler.iOS.cs / ImageButtonHandler.iOS.cs, Added a MapBackground method that calls RemoveBackgroundLayer() + resets BackgroundColor when the background is null, otherwise applies it normally. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #36302 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **iOS**<br> <video src="https://github.com/user-attachments/assets/eac5d0f7-181b-4c9e-b5e6-87f7445c2ed7" width="300" height="600"> | **iOS**<br> <video src="https://github.com/user-attachments/assets/7964d7a7-0b4d-4472-b839-f91ec61bd4cf" width="300" height="600"> |
… reset when set to null (#36427) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue details Setting [BackgroundColor] to null on Image and ImageButton controls does not reset/clear the background color on iOS and macOS (MacCatalyst). The previously set background color persists visually even though the property value reports null. ### Description of Change <!-- Enter description of the fix in this section --> ImageHandler.iOS.cs / ImageButtonHandler.iOS.cs, Added a MapBackground method that calls RemoveBackgroundLayer() + resets BackgroundColor when the background is null, otherwise applies it normally. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #36302 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **iOS**<br> <video src="https://github.com/user-attachments/assets/eac5d0f7-181b-4c9e-b5e6-87f7445c2ed7" width="300" height="600"> | **iOS**<br> <video src="https://github.com/user-attachments/assets/7964d7a7-0b4d-4472-b839-f91ec61bd4cf" width="300" height="600"> |
… reset when set to null (#36427) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue details Setting [BackgroundColor] to null on Image and ImageButton controls does not reset/clear the background color on iOS and macOS (MacCatalyst). The previously set background color persists visually even though the property value reports null. ### Description of Change <!-- Enter description of the fix in this section --> ImageHandler.iOS.cs / ImageButtonHandler.iOS.cs, Added a MapBackground method that calls RemoveBackgroundLayer() + resets BackgroundColor when the background is null, otherwise applies it normally. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #36302 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **iOS**<br> <video src="https://github.com/user-attachments/assets/eac5d0f7-181b-4c9e-b5e6-87f7445c2ed7" width="300" height="600"> | **iOS**<br> <video src="https://github.com/user-attachments/assets/7964d7a7-0b4d-4472-b839-f91ec61bd4cf" width="300" height="600"> |
… reset when set to null (#36427) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue details Setting [BackgroundColor] to null on Image and ImageButton controls does not reset/clear the background color on iOS and macOS (MacCatalyst). The previously set background color persists visually even though the property value reports null. ### Description of Change <!-- Enter description of the fix in this section --> ImageHandler.iOS.cs / ImageButtonHandler.iOS.cs, Added a MapBackground method that calls RemoveBackgroundLayer() + resets BackgroundColor when the background is null, otherwise applies it normally. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #36302 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **iOS**<br> <video src="https://github.com/user-attachments/assets/eac5d0f7-181b-4c9e-b5e6-87f7445c2ed7" width="300" height="600"> | **iOS**<br> <video src="https://github.com/user-attachments/assets/7964d7a7-0b4d-4472-b839-f91ec61bd4cf" width="300" height="600"> |
… reset when set to null (#36427) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue details Setting [BackgroundColor] to null on Image and ImageButton controls does not reset/clear the background color on iOS and macOS (MacCatalyst). The previously set background color persists visually even though the property value reports null. ### Description of Change <!-- Enter description of the fix in this section --> ImageHandler.iOS.cs / ImageButtonHandler.iOS.cs, Added a MapBackground method that calls RemoveBackgroundLayer() + resets BackgroundColor when the background is null, otherwise applies it normally. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #36302 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **iOS**<br> <video src="https://github.com/user-attachments/assets/eac5d0f7-181b-4c9e-b5e6-87f7445c2ed7" width="300" height="600"> | **iOS**<br> <video src="https://github.com/user-attachments/assets/7964d7a7-0b4d-4472-b839-f91ec61bd4cf" width="300" height="600"> |
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!
Issue details
Setting [BackgroundColor] to null on Image and ImageButton controls does not reset/clear the background color on iOS and macOS (MacCatalyst). The previously set background color persists visually even though the property value reports null.
Description of Change
ImageHandler.iOS.cs / ImageButtonHandler.iOS.cs, Added a MapBackground method that calls RemoveBackgroundLayer() + resets BackgroundColor when the background is null, otherwise applies it normally.
Issues Fixed
Fixes #36302
Tested the behavior in the following platforms.
Before.mov
After.mov