Skip to content

Commit 6c123d7

Browse files
PureWeengithub-actions[bot]CopilotTamilarasan-Paranthaman
authored
[iOS] Fix SafeArea infinite layout cycle with parent hierarchy walk and pixel-level comparison (dotnet#34024)
<!-- 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! ### Root Cause `SafeAreaInsetsDidChange` fires repeatedly during iOS animations (e.g., `TranslateToAsync`, bottom sheet transitions) as views move relative to the window. This caused two distinct infinite loop patterns: 1. **Sub-pixel oscillation** (dotnet#32586, dotnet#33934): Animations produce sub-pixel differences in `SafeAreaInsets` (e.g., `0.0000001pt`). Exact equality fails, triggering `InvalidateAncestorsMeasures` → layout pass → position change → new `SafeAreaInsetsDidChange` → infinite loop. 2. **Parent-child double application** (dotnet#33595): A `ContentPage` (implementing `ISafeAreaView`) and its child `Grid` both independently apply safe area adjustments. When the `ContentPage` adjusts its layout for the notch/status bar, it repositions the `Grid`. The `Grid`'s new position fires `SafeAreaInsetsDidChange`, causing it to re-apply its own adjustment — creating a ping-pong loop. ### Description of Change **Primary fix — `IsParentHandlingSafeArea` (parent hierarchy walk):** In both `MauiView.ValidateSafeArea` and `MauiScrollView.ValidateSafeArea`, before applying safe area adjustments, we now check whether an ancestor `MauiView` is already applying safe area for the **same edges**. If so, the child skips its own adjustment to avoid double-padding. The check is **edge-aware**: a parent handling `Top` does not block a child from independently handling `Bottom`. Only overlapping edges cause deferral. The `_parentHandlesSafeArea` result is cached per layout cycle and cleared on `SafeAreaInsetsDidChange`, `InvalidateSafeArea`, and `MovedToWindow`. **Secondary fix — `EqualsAtPixelLevel`:** Safe area values are compared at device-pixel resolution (rounding to `1 / ContentScaleFactor`) before deciding whether to trigger a layout invalidation. This absorbs sub-pixel animation noise and prevents the oscillation loops in dotnet#32586 and dotnet#33934. **MauiScrollView bug fixes:** - Inverted condition: `!UpdateContentInsetAdjustmentBehavior()` was incorrectly gating behavior; corrected to `UpdateContentInsetAdjustmentBehavior()`. - The `_appliesSafeAreaAdjustments` flag now correctly incorporates `!IsParentHandlingSafeArea()`. **What was removed:** - The "Window Guard" approach (comparing `Window.SafeAreaInsets` to filter noise) was tried and removed. It was fragile: on macCatalyst with a custom TitleBar, `WindowViewController` repositions content by pushing it down, which changes the view's own `SafeAreaInsets` without changing `Window.SafeAreaInsets`. The guard blocked this legitimate change, causing a 28px content shift regression in CI. ### Issues Fixed Fixes dotnet#32586 Fixes dotnet#33934 Fixes dotnet#33595 Fixes dotnet#34042 --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tamilarasan-Paranthaman <Tamilarasan-Paranthaman@users.noreply.github.com>
1 parent e8265c0 commit 6c123d7

21 files changed

Lines changed: 2257 additions & 42 deletions

.github/copilot-instructions.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,17 @@ git commit -m "Fix: Description of the change"
201201
2. Exception: If the user's instructions explicitly include pushing, proceed without asking.
202202

203203
### Documentation
204+
204205
- Update XML documentation for public APIs
205206
- Follow existing code documentation patterns
206207
- Update relevant docs in `docs/` folder when needed
207208

209+
**Platform-Specific Documentation:**
210+
- `.github/instructions/safe-area-ios.instructions.md` - Safe area investigation (iOS/macCatalyst)
211+
- `.github/instructions/uitests.instructions.md` - UI test guidelines (includes safe area testing section)
212+
- `.github/instructions/android.instructions.md` - Android handler implementation
213+
- `.github/instructions/xaml-unittests.instructions.md` - XAML unit test guidelines
214+
208215
### Opening PRs
209216

210217
All PRs are required to have this at the top of the description:
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
applyTo:
3+
- "**/Platform/iOS/MauiView.cs"
4+
- "**/Platform/iOS/MauiScrollView.cs"
5+
- "**/Platform/iOS/*SafeArea*"
6+
---
7+
8+
# Safe Area Guidelines (iOS/macCatalyst)
9+
10+
## Platform Differences
11+
12+
| | macOS 14/15 | macOS 26+ |
13+
|-|-------------|-----------|
14+
| Title bar inset | ~28px | ~0px |
15+
| Used in CI |||
16+
17+
Local macOS 26+ testing does NOT validate CI behavior. Fixes must pass CI on macOS 14/15.
18+
19+
| Platform | `UseSafeArea` default |
20+
|----------|-----------------------|
21+
| iOS | `false` |
22+
| macCatalyst | `true` |
23+
24+
## Architecture (PR #34024)
25+
26+
**`IsParentHandlingSafeArea`** — before applying adjustments, `MauiView`/`MauiScrollView` walk ancestors to check if any ancestor handles the **same edges**. If so, descendant skips (avoids double-padding). Edge-aware: parent handling `Top` does not block child handling `Bottom`. Result cached in `bool? _parentHandlesSafeArea`; cleared on `SafeAreaInsetsDidChange`, `InvalidateSafeArea`, `MovedToWindow`. `AppliesSafeAreaAdjustments` is `internal` for cross-type ancestor checks.
27+
28+
**`EqualsAtPixelLevel`** — safe area compared at device-pixel resolution to absorb sub-pixel animation noise (`0.0000001pt` during `TranslateToAsync`), preventing oscillation loops (#32586, #33934).
29+
30+
## Anti-Patterns
31+
32+
**❌ Window Guard** — comparing `Window.SafeAreaInsets` to filter callbacks blocks legitimate updates. On macCatalyst + custom TitleBar, `WindowViewController` pushes content down, changing the **view's** `SafeAreaInsets` without changing the **window's**. Caused 28px CI shift (macOS 14/15 only). Never gate per-view callbacks on window-level insets.
33+
34+
**❌ Semantic mismatch**`_safeArea` is filtered by `GetSafeAreaForEdge` (zeroes edges per `SafeAreaRegions`); raw `UIView.SafeAreaInsets` includes all edges. Never compare them — compare raw-to-raw or adjusted-to-adjusted.

.github/instructions/uitests.instructions.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,3 +731,45 @@ grep -r "UITestEntry\|UITestEditor\|UITestSearchBar" src/Controls/tests/TestCase
731731
- Common helper methods
732732
- Platform-specific workarounds
733733
- UITest optimized control usage
734+
735+
### Safe Area Testing (iOS/MacCatalyst)
736+
737+
**⚠️ CRITICAL for macCatalyst safe area tests:**
738+
739+
Safe area behavior differs significantly between macOS versions. Tests must account for this variability.
740+
741+
| macOS Version | Title Bar Safe Area | CI Environment |
742+
|---------------|---------------------|----------------|
743+
| **macOS 14/15** | ~28px top inset | ✅ Used by CI |
744+
| **macOS 26 (Liquid Glass)** | ~0px top inset | ❌ Local dev only |
745+
746+
**Rules for safe area tests:**
747+
748+
1. **Use tolerances for safe area measurements** - Exact pixel values vary by macOS version
749+
2. **Test behavior, not exact values** - Verify content is NOT obscured, rather than checking exact padding pixels
750+
3. **Use `GetRect()` for child content position** - Measure where content actually appears, not parent size
751+
4. **Never hardcode safe area expectations** - Tests should pass on macOS 14/15 AND macOS 26
752+
753+
**Example patterns:**
754+
755+
```csharp
756+
// ❌ BAD: Hardcoded safe area value (breaks across macOS versions)
757+
var safeArea = element.GetRect();
758+
Assert.That(safeArea.Y, Is.EqualTo(28)); // Fails on macOS 26
759+
760+
// ✅ GOOD: Test that content is not obscured by title bar
761+
var contentRect = App.WaitForElement("MyContent").GetRect();
762+
var titleBarRect = App.WaitForElement("TitleBar").GetRect();
763+
Assert.That(contentRect.Y, Is.GreaterThanOrEqualTo(titleBarRect.Height),
764+
"Content should not be obscured by title bar");
765+
766+
// ✅ GOOD: Use tolerance for safe area (accounts for OS differences)
767+
Assert.That(contentRect.Y, Is.GreaterThan(0).And.LessThan(50),
768+
"Content should have some top padding but not excessive");
769+
```
770+
771+
**Test category**: Use `UITestCategories.SafeAreaEdges` for safe area tests.
772+
773+
**Platform scope**: Safe area tests should typically run on iOS and MacCatalyst (not just one).
774+
775+
**See also**: `.github/instructions/safe-area-debugging.instructions.md` for investigation guidelines
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
3+
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
4+
x:Class="Maui.Controls.Sample.Issues.Issue28986_ParentChildTest"
5+
Title="Issue28986 - Parent Child SafeArea">
6+
7+
<Grid x:Name="ParentGrid"
8+
BackgroundColor="#FFEB3B"
9+
SafeAreaEdges="None,Container,None,None"
10+
AutomationId="ParentGrid">
11+
12+
<Grid.RowDefinitions>
13+
<RowDefinition Height="Auto"/>
14+
<RowDefinition Height="*"/>
15+
<RowDefinition Height="Auto"/>
16+
</Grid.RowDefinitions>
17+
18+
<!-- Top Indicator -->
19+
<Label Grid.Row="0"
20+
Text="↑ Parent handles TOP safe area ↑"
21+
BackgroundColor="#FF5722"
22+
TextColor="White"
23+
HorizontalTextAlignment="Center"
24+
AutomationId="TopIndicator"/>
25+
26+
<!-- Middle Content Area -->
27+
<VerticalStackLayout Grid.Row="1"
28+
BackgroundColor="#00BCD4"
29+
VerticalOptions="Center"
30+
Spacing="20"
31+
Padding="20">
32+
33+
<Label Text="SafeAreaEdges Independent Handling Demo"
34+
FontSize="20"
35+
FontAttributes="Bold"
36+
HorizontalTextAlignment="Center"
37+
AutomationId="TitleLabel"/>
38+
39+
<Label x:Name="StatusLabel"
40+
Text="Parent: Top=Container, Bottom=None | Child: Bottom=Container"
41+
HorizontalTextAlignment="Center"
42+
AutomationId="StatusLabel"/>
43+
44+
<Button Text="Toggle Parent Top SafeArea"
45+
Clicked="OnToggleParentTop"
46+
HorizontalOptions="Center"
47+
AutomationId="ToggleParentTopButton"/>
48+
49+
<Button Text="Toggle Parent Bottom SafeArea"
50+
Clicked="OnToggleParentBottom"
51+
HorizontalOptions="Center"
52+
AutomationId="ToggleParentBottomButton"/>
53+
54+
<Button Text="Toggle Child Bottom SafeArea"
55+
Clicked="OnToggleChildBottom"
56+
HorizontalOptions="Center"
57+
AutomationId="ToggleChildBottomButton"/>
58+
59+
<Label Text="Expected behavior:"
60+
FontAttributes="Bold"
61+
Margin="0,20,0,0"
62+
AutomationId="ExpectedLabel"/>
63+
64+
<Label Text="• Top indicator stays below notch/status bar (parent handles top)&#x0a;• Bottom indicator stays above home indicator (child handles bottom)&#x0a;• Both work INDEPENDENTLY - no conflict!&#x0a;• Runtime changes to parent do NOT disrupt child's handling&#x0a;• NO DOUBLE PADDING when both parent and child handle same edge"
65+
FontSize="12"
66+
AutomationId="ExpectedDetailsLabel"/>
67+
68+
</VerticalStackLayout>
69+
70+
<!-- Bottom Indicator -->
71+
<Grid Grid.Row="2"
72+
BackgroundColor="#9C27B0"
73+
x:Name="ChildGrid"
74+
SafeAreaEdges="None,None,None,Container"
75+
AutomationId="ChildGrid">
76+
77+
<Label Text="↓ Child handles BOTTOM safe area ↓"
78+
BackgroundColor="#8BC34A"
79+
HorizontalTextAlignment="Center"
80+
AutomationId="BottomIndicator"/>
81+
82+
</Grid>
83+
</Grid>
84+
</ContentPage>
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
namespace Maui.Controls.Sample.Issues;
2+
3+
[Issue(IssueTracker.Github, 28986, "SafeAreaEdges independent handling for parent and child controls", PlatformAffected.iOS, issueTestNumber: 9)]
4+
public partial class Issue28986_ParentChildTest : ContentPage
5+
{
6+
bool _parentTopEnabled = true;
7+
bool _parentBottomEnabled = false;
8+
bool _childBottomEnabled = true;
9+
10+
public Issue28986_ParentChildTest()
11+
{
12+
InitializeComponent();
13+
UpdateParentGridSafeAreaEdges();
14+
UpdateStatusLabel();
15+
}
16+
17+
void OnToggleParentTop(object sender, EventArgs e)
18+
{
19+
_parentTopEnabled = !_parentTopEnabled;
20+
UpdateParentGridSafeAreaEdges();
21+
UpdateStatusLabel();
22+
}
23+
24+
void OnToggleParentBottom(object sender, EventArgs e)
25+
{
26+
_parentBottomEnabled = !_parentBottomEnabled;
27+
UpdateParentGridSafeAreaEdges();
28+
UpdateStatusLabel();
29+
}
30+
31+
void OnToggleChildBottom(object sender, EventArgs e)
32+
{
33+
_childBottomEnabled = !_childBottomEnabled;
34+
35+
// Toggle between Bottom=Container and Bottom=None
36+
ChildGrid.SafeAreaEdges = _childBottomEnabled
37+
? new SafeAreaEdges(SafeAreaRegions.None, SafeAreaRegions.None, SafeAreaRegions.None, SafeAreaRegions.Container)
38+
: new SafeAreaEdges(SafeAreaRegions.None);
39+
40+
UpdateStatusLabel();
41+
}
42+
43+
void UpdateParentGridSafeAreaEdges()
44+
{
45+
// Build parent grid SafeAreaEdges based on top and bottom flags
46+
SafeAreaRegions top = _parentTopEnabled ? SafeAreaRegions.Container : SafeAreaRegions.None;
47+
SafeAreaRegions bottom = _parentBottomEnabled ? SafeAreaRegions.Container : SafeAreaRegions.None;
48+
49+
ParentGrid.SafeAreaEdges = new SafeAreaEdges(SafeAreaRegions.None, top, SafeAreaRegions.None, bottom);
50+
}
51+
52+
void UpdateStatusLabel()
53+
{
54+
var parentTop = _parentTopEnabled ? "Container" : "None";
55+
var parentBottom = _parentBottomEnabled ? "Container" : "None";
56+
var childBottom = _childBottomEnabled ? "Container" : "None";
57+
StatusLabel.Text = $"Parent: Top={parentTop}, Bottom={parentBottom} | Child: Bottom={childBottom}";
58+
}
59+
}

0 commit comments

Comments
 (0)