From a8e8d6db26aba1cd9e097707d563d5f4e4387713 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Tue, 11 Aug 2026 13:16:46 +0200 Subject: [PATCH 01/13] Never park an element request a collapsed branch cannot retry; share the baked-safe-area predicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two adopted findings from the latest review round: An element-mode request parked for arranged geometry is retried only by OnSizeAllocated/ContentSizeChanged, and a view anywhere inside a collapsed (IsVisible=false) branch is skipped by layout entirely — so parking there left the caller's ScrollToAsync task pending forever. Both park sites now check WillArrange() (the IsVisible chain) and dispatch immediately when no arrange is coming: the target clamps and the task completes, matching the other platforms' behavior for a collapsed scroll view. SafeAreaBakedIntoContent restated the arrange branch's predicate, so an edit to one could silently desynchronize the other by a full safe-area thickness. Both now share UIKitCompensatesForSafeArea, and the doc spells out the one deliberate asymmetry: the horizontal safe-area origin the landscape-notch arrange keeps for vertical Automatic is not reported, because that axis cannot scroll and the arranged-rect origin already carries it for the extent. Verified: ScrollViewUnitTests 25/25 (two new regression tests), and the Issue36801 + Issue36801DeferredElement + ShellFlyoutHeaderScrollViewContent iOS UI suites 12/12 on the simulator. Co-Authored-By: Claude Fable 5 --- .../src/Core/ScrollView/ScrollView.cs | 42 +++++++++++++++---- .../Core.UnitTests/ScrollViewUnitTests.cs | 42 +++++++++++++++++++ src/Core/src/Platform/iOS/MauiScrollView.cs | 29 +++++++++---- 3 files changed, 96 insertions(+), 17 deletions(-) diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index 2dd1ed0159ee..3301650f46c7 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -71,19 +71,25 @@ void DispatchPendingScrollToRequest() return; } - if (pending.Mode == ScrollToMode.Element) + if (pending.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady()) { - if (!IsElementTargetGeometryReady()) + if (WillArrange()) { // The request has to wait; OnSizeAllocated and ContentSizeChanged retry it. return; } - // Those callbacks run while the pass that produced the sizes is still arranging - // children, so resolve on the next tick, once positions are final. Posting on - // every retry is deliberate: SendPendingScrollToRequest is a no-op once the - // request has been sent or superseded, so a dropped callback cannot wedge the - // request the way an "already queued" flag would. + // A collapsed branch is skipped by layout entirely, so those callbacks will + // never fire: dispatch now — the target clamps — rather than hang the + // caller's task forever. Falls through to the immediate send below. + } + else if (pending.Mode == ScrollToMode.Element) + { + // The geometry callbacks run while the pass that produced the sizes is still + // arranging children, so resolve on the next tick, once positions are final. + // Posting on every retry is deliberate: SendPendingScrollToRequest is a no-op + // once the request has been sent or superseded, so a dropped callback cannot + // wedge the request the way an "already queued" flag would. Dispatcher.Dispatch(SendPendingScrollToRequest); return; } @@ -101,6 +107,24 @@ void DispatchPendingScrollToRequest() bool IsElementTargetGeometryReady() => Width >= 0 && Height >= 0 && Content is not ({ Width: < 0 } or { Height: < 0 }); + // A parked element request is retried only from OnSizeAllocated/ContentSizeChanged, + // which fire when this view gets arranged — and a view anywhere inside a collapsed + // (IsVisible=false) branch is skipped by layout entirely. Parking in that state would + // leave the caller's task pending forever; dispatching instead clamps the target and + // completes it, which is also what the other platforms do with a collapsed scroll view. + bool WillArrange() + { + for (Element element = this; element is VisualElement visual; element = element.RealParent) + { + if (!visual.IsVisible) + { + return false; + } + } + + return true; + } + void SendPendingScrollToRequest() { if (Handler is null || _pendingScrollToRequested is not { } pending) @@ -527,12 +551,14 @@ void OnScrollToRequested(ScrollToRequestedEventArgs e) _pendingScrollToRequested = e; _replayPendingScrollToRequestedEvent = true; } - else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady()) + else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady() && WillArrange()) { // The handler exists but layout has not run yet (e.g. ScrollToAsync from // OnAppearing): resolving the element target now would compute against the -1 // never-arranged sentinels. Park it for the layout callbacks instead — the // subscribers were already notified above, so the replay must not re-raise. + // A collapsed branch never gets those callbacks, so it dispatches immediately + // below instead of parking a request nothing will ever retry. _pendingScrollToRequested = e; _replayPendingScrollToRequestedEvent = false; } diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index 9c1e8e23f72f..5cd765806b62 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -487,6 +487,48 @@ public void ElementRequestWithHandlerAttachedWaitsForArrange() Assert.True(task.IsCompleted); } + [Fact] + public void ElementRequestOnCollapsedScrollViewCompletesInsteadOfHanging() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout, IsVisible = false }; + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // A collapsed ScrollView is skipped by layout, so the geometry callbacks that + // retry a parked request never fire: the request must dispatch immediately (the + // target clamps) so the caller's task can complete instead of hanging forever + var task = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); + Assert.Single(handler.ScrollToRequests); + + scrollView.SendScrollFinished(); + Assert.True(task.IsCompleted); + } + + [Fact] + public void DeferredElementRequestOnCollapsedAncestorDispatchesOnAttach() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + _ = new StackLayout { IsVisible = false, Children = { scrollView } }; + + // Parked because the handler is missing, not because of visibility + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // On attach the collapsed ancestor means no arrange is coming: the deferred + // request must dispatch right away rather than wait for callbacks that never fire + Assert.Single(handler.ScrollToRequests); + + scrollView.SendScrollFinished(); + Assert.True(task.IsCompleted); + } + [Fact] public void DeferredRequestReplaysEventForSubscribersAttachedWithTheHandler() { diff --git a/src/Core/src/Platform/iOS/MauiScrollView.cs b/src/Core/src/Platform/iOS/MauiScrollView.cs index dedd7501a4b3..c4ed5873e0e9 100644 --- a/src/Core/src/Platform/iOS/MauiScrollView.cs +++ b/src/Core/src/Platform/iOS/MauiScrollView.cs @@ -481,6 +481,18 @@ internal CGSize ScrollableContentSize /// CGRect? _arrangedContentRect; + /// + /// Whether UIKit is compensating for the safe area through + /// . When it is not + /// (, or a zero system + /// inset), bakes the safe area into the content's + /// coordinate space instead. Shared by the arrange branch and + /// so the two can never desynchronize. + /// + bool UIKitCompensatesForSafeArea => + SystemAdjustedContentInset != UIEdgeInsets.Zero + && ContentInsetAdjustmentBehavior != UIScrollViewContentInsetAdjustmentBehavior.Never; + /// /// The safe area baked into the content's coordinate /// space: when it applies the safe area while UIKit is not compensating through @@ -489,16 +501,15 @@ internal CGSize ScrollableContentSize /// padding still obscures the viewport without ever appearing in the adjusted inset. /// /// - /// Mirrors the arrange-side branch exactly: bounds are inset only while - /// _appliesSafeAreaAdjustments, and the inset origin is kept only when UIKit - /// contributes nothing (, - /// or a zero system inset). In the remaining case () - /// the content is re-based at the origin and UIKit's inset owns the compensation, so the - /// arranged rect measures naturally excludes it (issue #36801). + /// Shares with the arrange branch, so the + /// baked state always describes what the last arrange actually did: when UIKit is not + /// compensating, the content is arranged inside safe-area-inset bounds and the padding + /// lives in its coordinate space; when UIKit compensates, the content is re-based at + /// the origin and the adjusted inset owns the compensation, which the arranged rect + /// measures naturally excludes (issue #36801). /// internal SafeAreaPadding SafeAreaBakedIntoContent => - _appliesSafeAreaAdjustments && - (SystemAdjustedContentInset == UIEdgeInsets.Zero || ContentInsetAdjustmentBehavior == UIScrollViewContentInsetAdjustmentBehavior.Never) + _appliesSafeAreaAdjustments && !UIKitCompensatesForSafeArea ? _safeArea : SafeAreaPadding.Empty; @@ -538,7 +549,7 @@ Size CrossPlatformArrange(CGRect bounds) CGPoint contentOrigin; double width; double height; - if (SystemAdjustedContentInset == UIEdgeInsets.Zero || ContentInsetAdjustmentBehavior == UIScrollViewContentInsetAdjustmentBehavior.Never) + if (!UIKitCompensatesForSafeArea) { contentSize = CrossPlatformLayout?.CrossPlatformArrange(bounds.ToRectangle()) ?? Size.Zero; contentOrigin = bounds.Location; From d79d09c02a16dae49085443f3a572702c4d2295b Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Thu, 13 Aug 2026 18:55:34 +0200 Subject: [PATCH 02/13] Restore Automatic expectations for SafeAreaEdges.Default after the landscape-notch revert The Issue36801 fixtures merged with PR #37060 asserted the resolved ContentInsetAdjustmentBehavior for SafeAreaEdges.Default as Never, matching the landscape-notch fix (#35533) that was in inflight/current at the time. That fix has since been reverted (#36580), so Default resolves to Automatic again and the shipped resolved-mode sentinel now fails the suite on the current base. Restore the Automatic expectations in the shared tests and the HostApp oracle; the sentinel is doing its job both times, failing loudly instead of silently testing a different clamp branch. Co-Authored-By: Claude Fable 5 --- .../tests/TestCases.HostApp/Issues/Issue36801.cs | 5 +---- .../Tests/Issues/Issue36801.cs | 14 +++++++------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue36801.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue36801.cs index 0a3ee58aa362..632010939786 100644 --- a/src/Controls/tests/TestCases.HostApp/Issues/Issue36801.cs +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue36801.cs @@ -167,10 +167,7 @@ string CheckResolvedMode(UIKit.UIScrollView nativeScrollView, string kind) var expected = edges.Equals(new SafeAreaEdges(SafeAreaRegions.Container)) ? UIKit.UIScrollViewContentInsetAdjustmentBehavior.Always : edges.Equals(SafeAreaEdges.None) || edges.Equals(SafeAreaEdges.All) ? UIKit.UIScrollViewContentInsetAdjustmentBehavior.Never : - // Default on a vertical scroll view resolves to Never since the landscape-notch - // fix (#35533): MAUI owns all edges there, and Automatic remains in use only for - // horizontal scroll views - UIKit.UIScrollViewContentInsetAdjustmentBehavior.Never; + UIKit.UIScrollViewContentInsetAdjustmentBehavior.Automatic; return nativeScrollView.ContentInsetAdjustmentBehavior == expected ? null diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36801.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36801.cs index 0c072373ceca..2847e5d412e9 100644 --- a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36801.cs +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36801.cs @@ -66,14 +66,14 @@ public void ScrollToElementEndLandsInsideVisibleViewport() "Probe label should be fully visible after ScrollToAsync(element, End)"); } - // The reachable ContentInsetAdjustmentBehavior modes bake the safe area into the content + // The three ContentInsetAdjustmentBehavior modes bake the safe area into the content // differently, so each is exercised. The page asserts the resolved native behavior, so a - // mode that silently drifted fails instead of quietly testing a different branch. - // Default on a vertical scroll view resolves to Never since the landscape-notch fix - // (#35533); Automatic remains in use only for horizontal scroll views. + // mode that silently drifted fails instead of quietly testing a different branch — + // exactly what happened when the landscape-notch fix (#35533, since reverted in #36580) + // briefly remapped Default to Never. [Test] [Category(UITestCategories.ScrollView)] - [TestCase("ModeDefaultButton", "Never")] + [TestCase("ModeDefaultButton", "Automatic")] [TestCase("ModeNoneButton", "Never")] // Also resolves to Never, but bakes the safe area into the arranged content, which None // does not — so it is the case that actually exercises the measured extent's baked padding @@ -97,13 +97,13 @@ public void ScrollToExtremesInEachInsetMode(string modeButton, string expectedMo } // Element targets resolve against the effective viewport, and each inset mode obscures it - // differently: Always through AdjustedContentInset, Default/SafeAreaEdges.All by baking + // differently: Automatic/Always through AdjustedContentInset, SafeAreaEdges.All by baking // the safe area into the content where AdjustedContentInset never reports it. The page's // oracle measures the probe's bottom edge against the unobscured viewport bottom in window // coordinates, so the mode where MAUI itself obscures the viewport is proven too. [Test] [Category(UITestCategories.ScrollView)] - [TestCase("ModeDefaultButton", "Never")] + [TestCase("ModeDefaultButton", "Automatic")] [TestCase("ModeNoneButton", "Never")] // Also resolves to Never, but bakes the safe area into the content — the case where the // viewport shrink comes from MAUI's own arrange instead of a UIKit inset From 9f439edcf6aef396ed947e6b8e8dac74c4ce9337 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Fri, 14 Aug 2026 10:37:59 +0200 Subject: [PATCH 03/13] Convert the last raw occurrence of the shared safe-area predicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ValidateSafeArea still selected the _safeArea source with the inline condition that UIKitCompensatesForSafeArea now names — and that value is exactly what SafeAreaBakedIntoContent returns, so leaving it duplicated kept alive the desynchronization the refactor exists to prevent. The De Morgan negation is exact; no behavior change. Co-Authored-By: Claude Fable 5 --- src/Core/src/Platform/iOS/MauiScrollView.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Core/src/Platform/iOS/MauiScrollView.cs b/src/Core/src/Platform/iOS/MauiScrollView.cs index c4ed5873e0e9..dc15ad93718a 100644 --- a/src/Core/src/Platform/iOS/MauiScrollView.cs +++ b/src/Core/src/Platform/iOS/MauiScrollView.cs @@ -392,7 +392,7 @@ bool ValidateSafeArea() // it can push ContentSize over the Bounds, causing AdjustedContentInset to become non-zero and SafeAreaInsets on the child to reset to zero. // This can result in a loop of invalidations as the layout toggles between these states. // To prevent this, we ignore safe area calculations on child views when they are inside a scroll view. - if (SystemAdjustedContentInset == UIEdgeInsets.Zero || ContentInsetAdjustmentBehavior == UIScrollViewContentInsetAdjustmentBehavior.Never) + if (!UIKitCompensatesForSafeArea) _safeArea = GetInset().ToSafeAreaInsets(); else _safeArea = SystemAdjustedContentInset.ToSafeAreaInsets(); @@ -486,8 +486,9 @@ internal CGSize ScrollableContentSize /// . When it is not /// (, or a zero system /// inset), bakes the safe area into the content's - /// coordinate space instead. Shared by the arrange branch and - /// so the two can never desynchronize. + /// coordinate space instead. Shared by the arrange branch, the _safeArea source + /// selection in , and + /// so the sites can never desynchronize. /// bool UIKitCompensatesForSafeArea => SystemAdjustedContentInset != UIEdgeInsets.Zero From 73060003fff0bd49ba2313f53b6172465c1ecf06 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Fri, 14 Aug 2026 10:39:03 +0200 Subject: [PATCH 04/13] Check every visual ancestor's visibility, skipping non-visual links in the chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WillArrange() terminated its walk at the first non-VisualElement parent, so a collapsed VisualElement above a non-visual link — Shell itself above ShellContent/ShellSection, or any custom non-visual container — was never seen and the request parked for an arrange that cannot come. The walk now traverses the whole RealParent chain and checks IsVisible on the visual nodes it crosses. Non-visual containers' own visibility semantics (a hidden tab, say) are deliberately not consulted: parking keeps the scroll correct if that container is ever shown, and a handler detach still completes the task; the comment now states that guarantee precisely. Co-Authored-By: Claude Fable 5 --- .../src/Core/ScrollView/ScrollView.cs | 11 ++++++--- .../Core.UnitTests/ScrollViewUnitTests.cs | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index 3301650f46c7..ed39e603d7e4 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -108,15 +108,20 @@ bool IsElementTargetGeometryReady() => Width >= 0 && Height >= 0 && Content is not ({ Width: < 0 } or { Height: < 0 }); // A parked element request is retried only from OnSizeAllocated/ContentSizeChanged, - // which fire when this view gets arranged — and a view anywhere inside a collapsed + // which fire when this view gets arranged — and a view inside a collapsed // (IsVisible=false) branch is skipped by layout entirely. Parking in that state would // leave the caller's task pending forever; dispatching instead clamps the target and // completes it, which is also what the other platforms do with a collapsed scroll view. + // The walk checks IsVisible on every VisualElement ancestor, skipping over non-visual + // links in the chain (e.g. Shell's ShellContent/ShellSection) rather than stopping at + // them. Non-visual containers' own visibility semantics (a hidden tab, say) are + // deliberately not consulted: parking keeps the scroll correct if that container is + // ever shown, and a handler detach still completes the task. bool WillArrange() { - for (Element element = this; element is VisualElement visual; element = element.RealParent) + for (Element element = this; element is not null; element = element.RealParent) { - if (!visual.IsVisible) + if (element is VisualElement { IsVisible: false }) { return false; } diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index 5cd765806b62..c08287e6bfd7 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -529,6 +529,30 @@ public void DeferredElementRequestOnCollapsedAncestorDispatchesOnAttach() Assert.True(task.IsCompleted); } + [Fact] + public void CollapsedShellAncestorBeyondNonVisualLinksIsDetected() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var page = new ContentPage { Content = scrollView }; + var shell = new Shell { IsVisible = false }; + shell.Items.Add(new ShellContent { Content = page }); + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // The ancestor chain crosses non-VisualElement links (ShellContent/ShellSection) + // before reaching the collapsed Shell: the visibility walk must skip over them + // rather than stop, so the request dispatches immediately instead of parking for + // an arrange that never comes + var task = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); + Assert.Single(handler.ScrollToRequests); + + scrollView.SendScrollFinished(); + Assert.True(task.IsCompleted); + } + [Fact] public void DeferredRequestReplaysEventForSubscribersAttachedWithTheHandler() { From 7f625b35c26d3d8e5a629bb1d1e16a35a0f108be Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Fri, 14 Aug 2026 18:23:38 +0200 Subject: [PATCH 05/13] Release the caller but keep an element request parked while its branch is collapsed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dispatching a collapsed-branch element request immediately traded the hang for a lost target: the conversion ran against the -1 never-arranged sentinels, clamped to the origin, and consumed the request — so showing the branch later scrolled nowhere. Now both park sites complete the caller's task immediately (no hang) while leaving the request parked: the first arrange after the branch is shown replays it against real geometry and the scroll lands on the element, exactly as a visible pre-arrange park behaves. A newer request supersedes the parked one as usual, and a handler detach still drains it. This differs from the detach case deliberately: a hidden view being shown is the same living view continuing its lifecycle, so honoring the last requested target is the expected outcome — replaying across a handler detach/reattach would instead resurrect a request from a completed lifecycle. The three collapsed-branch unit tests now assert the full sequence — caller released, no premature dispatch, then shown-and-arranged replay landing on the element's real position — covering the collapsed-then- shown ordering the previous shape left untested. Verified: ScrollViewUnitTests 26/26, Issue36801 + Issue36801DeferredElement + ShellFlyoutHeaderScrollViewContent iOS UI suites 12/12 on the simulator. Co-Authored-By: Claude Fable 5 --- .../src/Core/ScrollView/ScrollView.cs | 38 +++++++----- .../Core.UnitTests/ScrollViewUnitTests.cs | 58 ++++++++++++------- 2 files changed, 63 insertions(+), 33 deletions(-) diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index ed39e603d7e4..829590bf5521 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -73,17 +73,21 @@ void DispatchPendingScrollToRequest() if (pending.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady()) { - if (WillArrange()) + if (!WillArrange()) { - // The request has to wait; OnSizeAllocated and ContentSizeChanged retry it. - return; + // A collapsed branch is skipped by layout, so no callback is coming to + // resolve the target. Release the caller now, but keep the request + // parked: if the branch is later shown, the first arrange replays it + // against real geometry so the scroll still lands on the element + // (a newer request supersedes it as usual). + SendScrollFinished(); } - // A collapsed branch is skipped by layout entirely, so those callbacks will - // never fire: dispatch now — the target clamps — rather than hang the - // caller's task forever. Falls through to the immediate send below. + // The request has to wait; OnSizeAllocated and ContentSizeChanged retry it. + return; } - else if (pending.Mode == ScrollToMode.Element) + + if (pending.Mode == ScrollToMode.Element) { // The geometry callbacks run while the pass that produced the sizes is still // arranging children, so resolve on the next tick, once positions are final. @@ -109,9 +113,10 @@ bool IsElementTargetGeometryReady() => // A parked element request is retried only from OnSizeAllocated/ContentSizeChanged, // which fire when this view gets arranged — and a view inside a collapsed - // (IsVisible=false) branch is skipped by layout entirely. Parking in that state would - // leave the caller's task pending forever; dispatching instead clamps the target and - // completes it, which is also what the other platforms do with a collapsed scroll view. + // (IsVisible=false) branch is skipped by layout entirely, so a request parked there + // has no callback coming. In that state the awaiting caller is released immediately + // while the request stays parked: showing the branch arranges it and replays the + // scroll against real geometry (see DispatchPendingScrollToRequest). // The walk checks IsVisible on every VisualElement ancestor, skipping over non-visual // links in the chain (e.g. Shell's ShellContent/ShellSection) rather than stopping at // them. Non-visual containers' own visibility semantics (a hidden tab, say) are @@ -556,16 +561,23 @@ void OnScrollToRequested(ScrollToRequestedEventArgs e) _pendingScrollToRequested = e; _replayPendingScrollToRequestedEvent = true; } - else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady() && WillArrange()) + else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady()) { // The handler exists but layout has not run yet (e.g. ScrollToAsync from // OnAppearing): resolving the element target now would compute against the -1 // never-arranged sentinels. Park it for the layout callbacks instead — the // subscribers were already notified above, so the replay must not re-raise. - // A collapsed branch never gets those callbacks, so it dispatches immediately - // below instead of parking a request nothing will ever retry. _pendingScrollToRequested = e; _replayPendingScrollToRequestedEvent = false; + + if (!WillArrange()) + { + // A collapsed branch gets no layout callbacks, so parking alone would + // hang the caller. Complete the task now and leave the request parked: + // showing the branch arranges it and replays the scroll against real + // geometry. + SendScrollFinished(); + } } else { diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index c08287e6bfd7..7c6c2ba10758 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -488,7 +488,7 @@ public void ElementRequestWithHandlerAttachedWaitsForArrange() } [Fact] - public void ElementRequestOnCollapsedScrollViewCompletesInsteadOfHanging() + public void ElementRequestOnCollapsedScrollViewReleasesCallerAndReplaysWhenShown() { var item = new View(); var layout = new StackLayout { Children = { item } }; @@ -497,23 +497,34 @@ public void ElementRequestOnCollapsedScrollViewCompletesInsteadOfHanging() var handler = new ViewportProviderHandlerStub(); scrollView.Handler = handler; - // A collapsed ScrollView is skipped by layout, so the geometry callbacks that - // retry a parked request never fire: the request must dispatch immediately (the - // target clamps) so the caller's task can complete instead of hanging forever - var task = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); - Assert.Single(handler.ScrollToRequests); - - scrollView.SendScrollFinished(); + // A collapsed ScrollView is skipped by layout, so no geometry callback is coming: + // the caller must be released immediately instead of hanging forever... + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Center, false); Assert.True(task.IsCompleted); + + // ...but the request must not resolve against the -1 never-arranged sentinels — + // it stays parked instead of clamping a garbage target to the origin + Assert.Empty(handler.ScrollToRequests); + + // Showing the branch arranges it; the first arrange replays the parked request + // against real geometry so the scroll still lands on the element + scrollView.IsVisible = true; + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + + // Center: 450 - 100/2 + 50/2 + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(425, request.VerticalOffset); } [Fact] - public void DeferredElementRequestOnCollapsedAncestorDispatchesOnAttach() + public void DeferredElementRequestOnCollapsedAncestorReleasesCallerOnAttach() { var item = new View(); var layout = new StackLayout { Children = { item } }; var scrollView = new ScrollView { Content = layout }; - _ = new StackLayout { IsVisible = false, Children = { scrollView } }; + var parent = new StackLayout { IsVisible = false, Children = { scrollView } }; // Parked because the handler is missing, not because of visibility var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); @@ -521,12 +532,21 @@ public void DeferredElementRequestOnCollapsedAncestorDispatchesOnAttach() var handler = new ViewportProviderHandlerStub(); scrollView.Handler = handler; - // On attach the collapsed ancestor means no arrange is coming: the deferred - // request must dispatch right away rather than wait for callbacks that never fire - Assert.Single(handler.ScrollToRequests); - - scrollView.SendScrollFinished(); + // On attach the collapsed ancestor means no arrange is coming: release the + // caller, but keep the request parked rather than resolving it against the + // never-arranged sentinels Assert.True(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // Showing the ancestor arranges the branch and replays the request for real + parent.IsVisible = true; + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + + // Start aligns the element's top with the viewport top + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(450, request.VerticalOffset); } [Fact] @@ -544,13 +564,11 @@ public void CollapsedShellAncestorBeyondNonVisualLinksIsDetected() // The ancestor chain crosses non-VisualElement links (ShellContent/ShellSection) // before reaching the collapsed Shell: the visibility walk must skip over them - // rather than stop, so the request dispatches immediately instead of parking for - // an arrange that never comes + // rather than stop, so the caller is released immediately instead of parking + // behind an arrange that never comes var task = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); - Assert.Single(handler.ScrollToRequests); - - scrollView.SendScrollFinished(); Assert.True(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); } [Fact] From 7e4f1bf86e20b2d091f56a25170afb861ed70f83 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Sat, 15 Aug 2026 09:42:25 +0200 Subject: [PATCH 06/13] Capture the baked safe area at arrange time instead of deriving it live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SafeAreaBakedIntoContent re-evaluated the arrange predicate against live UIKit state, but the two can legitimately disagree between arranges: under Automatic the arrange that bakes the padding can push the content size past the bounds, at which point UIKit turns the adjusted inset non-zero — and a later live read then reported Empty while the content still carried the padding, shifting element targets in that window. CrossPlatformArrange now records the baked padding from the branch it actually took, alongside _arrangedContentRect which already does the same for the rect, so the property genuinely describes the last arrange. Co-Authored-By: Claude Fable 5 --- src/Core/src/Platform/iOS/MauiScrollView.cs | 56 +++++++++++++-------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/src/Core/src/Platform/iOS/MauiScrollView.cs b/src/Core/src/Platform/iOS/MauiScrollView.cs index dc15ad93718a..78e1de43c7b3 100644 --- a/src/Core/src/Platform/iOS/MauiScrollView.cs +++ b/src/Core/src/Platform/iOS/MauiScrollView.cs @@ -486,33 +486,40 @@ internal CGSize ScrollableContentSize /// . When it is not /// (, or a zero system /// inset), bakes the safe area into the content's - /// coordinate space instead. Shared by the arrange branch, the _safeArea source - /// selection in , and - /// so the sites can never desynchronize. + /// coordinate space instead. Shared by the arrange branch and the _safeArea + /// source selection in so the two cannot desynchronize; + /// the arrange records the outcome in for + /// readers between arranges. /// bool UIKitCompensatesForSafeArea => SystemAdjustedContentInset != UIEdgeInsets.Zero && ContentInsetAdjustmentBehavior != UIScrollViewContentInsetAdjustmentBehavior.Never; /// - /// The safe area baked into the content's coordinate - /// space: when it applies the safe area while UIKit is not compensating through - /// , the content is arranged inside - /// safe-area-inset bounds, so element positions carry the padding and the trailing - /// padding still obscures the viewport without ever appearing in the adjusted inset. + /// The safe area the last baked into the content's + /// coordinate space: when it applies the safe area while UIKit is not compensating + /// through , the content is arranged + /// inside safe-area-inset bounds, so element positions carry the padding and the + /// trailing padding still obscures the viewport without ever appearing in the + /// adjusted inset. /// /// - /// Shares with the arrange branch, so the - /// baked state always describes what the last arrange actually did: when UIKit is not - /// compensating, the content is arranged inside safe-area-inset bounds and the padding - /// lives in its coordinate space; when UIKit compensates, the content is re-based at - /// the origin and the adjusted inset owns the compensation, which the arranged rect - /// measures naturally excludes (issue #36801). + /// Captured at arrange time rather than derived live from UIKit state, because the + /// two can legitimately disagree between arranges: under + /// the arrange + /// that bakes the padding can push the content size past the bounds, at which point + /// UIKit turns the adjusted inset non-zero — the content still carries the padding + /// until the next arrange re-bases it, and this must keep saying so. Recorded + /// alongside , which does the same for the rect + /// (issue #36801). /// - internal SafeAreaPadding SafeAreaBakedIntoContent => - _appliesSafeAreaAdjustments && !UIKitCompensatesForSafeArea - ? _safeArea - : SafeAreaPadding.Empty; + internal SafeAreaPadding SafeAreaBakedIntoContent => _bakedSafeArea; + + /// + /// The value reports, set by + /// from the branch it actually took. + /// + SafeAreaPadding _bakedSafeArea = SafeAreaPadding.Empty; UIEdgeInsets SystemAdjustedContentInset { @@ -548,12 +555,15 @@ Size CrossPlatformArrange(CGRect bounds) Size contentSize; CGPoint contentOrigin; + SafeAreaPadding bakedSafeArea; double width; double height; if (!UIKitCompensatesForSafeArea) { contentSize = CrossPlatformLayout?.CrossPlatformArrange(bounds.ToRectangle()) ?? Size.Zero; contentOrigin = bounds.Location; + // The inset bounds put the safe area into the content's coordinate space + bakedSafeArea = _appliesSafeAreaAdjustments ? _safeArea : SafeAreaPadding.Empty; width = contentSize.Width; height = contentSize.Height; @@ -562,14 +572,20 @@ Size CrossPlatformArrange(CGRect bounds) { contentSize = CrossPlatformLayout?.CrossPlatformArrange(new Rect(new Point(), bounds.Size.ToSize())) ?? Size.Zero; contentOrigin = CGPoint.Empty; + // Re-based at the origin: UIKit's adjusted inset owns the compensation + bakedSafeArea = SafeAreaPadding.Empty; width = contentSize.Width; height = contentSize.Height; } - // Record where the content was actually arranged, before the ContentSize adjustments - // below: ScrollableContentSize measures the scrollable extent from this rect + // Record what this arrange actually did, before the ContentSize adjustments below: + // ScrollableContentSize measures the scrollable extent from the rect, and + // SafeAreaBakedIntoContent reports the padding the content now carries. Both are + // captured rather than re-derived later, since UIKit's inset state can move + // between arranges (see SafeAreaBakedIntoContent). _arrangedContentRect = new CGRect(contentOrigin, contentSize.ToCGSize()); + _bakedSafeArea = bakedSafeArea; // When using ContentInsetAdjustmentBehavior.Automatic, UIKit dynamically decides whether to apply From a3323ee5b6b18225f6e777248917980f9cdb8378 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Sat, 15 Aug 2026 09:42:25 +0200 Subject: [PATCH 07/13] Release a parked element request when its view is collapsed or reparented before arranging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A request parked while visible (the OnAppearing ordering) is retried only from the arrange callbacks, so collapsing the ScrollView — or reparenting it into a collapsed branch — before the first arrange left the caller's task pending forever. Both transitions now re-check WillArrange() (OnIsVisibleChanged/OnParentSet) and release the caller, leaving the request parked so a later show still replays it against real geometry, exactly as a request parked while already collapsed does. An ancestor collapsing after the park sends no signal down the tree (layout managers skip the collapsed child; nothing reaches descendants), so that request is released on re-show or handler detach; the comment states this boundary explicitly rather than adding per-ancestor subscriptions whose lifecycle cost exceeds what they would cover. Verified: ScrollViewUnitTests 28/28 (two new ordering tests), Issue36801 + Issue36801DeferredElement + ShellFlyoutHeaderScrollViewContent iOS UI suites 12/12 on the simulator. Co-Authored-By: Claude Fable 5 --- .../PublicAPI/net-ios/PublicAPI.Unshipped.txt | 9 ++-- .../PublicAPI/net/PublicAPI.Unshipped.txt | 13 ++--- .../src/Core/ScrollView/ScrollView.cs | 34 ++++++++++++ .../Core.UnitTests/ScrollViewUnitTests.cs | 53 +++++++++++++++++++ 4 files changed, 99 insertions(+), 10 deletions(-) diff --git a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt index b87e005240dd..e655d1833ee6 100644 --- a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt @@ -1,9 +1,11 @@ #nullable enable +Microsoft.Maui.Controls.IndicatorView.~IndicatorView() -> void Microsoft.Maui.Controls.Label.~Label() -> void override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void override Microsoft.Maui.Controls.Handlers.Compatibility.EntryCellRenderer.EntryCellTableViewCell.Dispose(bool disposing) -> void override Microsoft.Maui.Controls.Handlers.Compatibility.TableViewModelRenderer.Dispose(bool disposing) -> void override Microsoft.Maui.Controls.Handlers.Items.MauiCollectionView.AddSubview(UIKit.UIView! view) -> void +override Microsoft.Maui.Controls.Handlers.Items.MauiCollectionView.LayoutSubviews() -> void override Microsoft.Maui.Controls.Handlers.Items.TemplatedCell.Dispose(bool disposing) -> void override Microsoft.Maui.Controls.Handlers.Items2.CarouselViewController2.Dispose(bool disposing) -> void ~override Microsoft.Maui.Controls.Handlers.Items2.CollectionViewHandler2.DisconnectHandler(UIKit.UIView platformView) -> void @@ -13,12 +15,11 @@ override Microsoft.Maui.Controls.Platform.Compatibility.ShellFlyoutContentRender ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellFlyoutRenderer.ViewWillTransitionToSize(CoreGraphics.CGSize toSize, UIKit.IUIViewControllerTransitionCoordinator coordinator) -> void override Microsoft.Maui.Controls.Platform.Compatibility.ShellItemRenderer.ViewDidAppear(bool animated) -> void ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.DidMoveToParentViewController(UIKit.UIViewController parent) -> void +~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.TraitCollectionDidChange(UIKit.UITraitCollection previousTraitCollection) -> void +~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.ViewWillTransitionToSize(CoreGraphics.CGSize toSize, UIKit.IUIViewControllerTransitionCoordinator coordinator) -> void override Microsoft.Maui.Controls.Platform.Compatibility.ShellTableViewController.LoadView() -> void ~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void +override Microsoft.Maui.Controls.ScrollView.OnParentSet() -> void override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? propertyName = null) -> void ~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void -Microsoft.Maui.Controls.IndicatorView.~IndicatorView() -> void -override Microsoft.Maui.Controls.Handlers.Items.MauiCollectionView.LayoutSubviews() -> void -~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.TraitCollectionDidChange(UIKit.UITraitCollection previousTraitCollection) -> void -~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.ViewWillTransitionToSize(CoreGraphics.CGSize toSize, UIKit.IUIViewControllerTransitionCoordinator coordinator) -> void diff --git a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt index 2a202a66463f..b3f00d0e2e2e 100644 --- a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt @@ -1,8 +1,9 @@ -#nullable enable -override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? propertyName = null) -> void -~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void -override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void -override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void +#nullable enable Microsoft.Maui.Controls.IndicatorView.~IndicatorView() -> void -~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void Microsoft.Maui.Controls.Label.~Label() -> void +override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void +~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void +override Microsoft.Maui.Controls.ScrollView.OnParentSet() -> void +override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? propertyName = null) -> void +~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void +override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index 829590bf5521..60fc330f277b 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -122,6 +122,13 @@ bool IsElementTargetGeometryReady() => // them. Non-visual containers' own visibility semantics (a hidden tab, say) are // deliberately not consulted: parking keeps the scroll correct if that container is // ever shown, and a handler detach still completes the task. + // Evaluated when a request is parked, at every retry, and again when this view's own + // IsVisible or parent changes (OnIsVisibleChanged/OnParentSet) so a request parked + // while visible is released if the view is collapsed or reparented into a collapsed + // branch before it arranges. An *ancestor* collapsing after the park sends no signal + // down the tree; that request is released when the branch is shown again or the + // handler detaches, and watching every ancestor for it would cost more than it + // covers. bool WillArrange() { for (Element element = this; element is not null; element = element.RealParent) @@ -135,6 +142,33 @@ bool WillArrange() return true; } + internal override void OnIsVisibleChanged(bool oldValue, bool newValue) + { + base.OnIsVisibleChanged(oldValue, newValue); + + // A request parked while visible loses its arrange callbacks if this view is + // collapsed before layout runs: release the caller now (the request stays parked + // and replays if the view is shown again, exactly as a request parked while + // already collapsed does) + ReleaseParkedRequestIfCollapsed(); + } + + protected override void OnParentSet() + { + base.OnParentSet(); + + // Same for a parked request carried into a collapsed branch by reparenting + ReleaseParkedRequestIfCollapsed(); + } + + void ReleaseParkedRequestIfCollapsed() + { + if (_pendingScrollToRequested is not null && !WillArrange()) + { + SendScrollFinished(); + } + } + void SendPendingScrollToRequest() { if (Handler is null || _pendingScrollToRequested is not { } pending) diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index 7c6c2ba10758..b6426990f32a 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -549,6 +549,59 @@ public void DeferredElementRequestOnCollapsedAncestorReleasesCallerOnAttach() Assert.Equal(450, request.VerticalOffset); } + [Fact] + public void ElementRequestParkedWhileVisibleIsReleasedWhenCollapsedBeforeArrange() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // Parked while visible with geometry not ready — the OnAppearing ordering + var task = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); + Assert.False(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // Collapsing before the first arrange removes the callbacks that would retry + // it: the caller must be released rather than left pending forever, and the + // request must stay parked instead of resolving against unarranged geometry + scrollView.IsVisible = false; + Assert.True(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // Shown again, the first arrange still replays it against real geometry: + // End = 450 - 100 + 50 + scrollView.IsVisible = true; + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(400, request.VerticalOffset); + } + + [Fact] + public void ElementRequestParkedWhileVisibleIsReleasedWhenReparentedIntoCollapsedBranch() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // Moving the ScrollView under a collapsed parent means no arrange is coming + // from there either: the parent change must release the caller + _ = new StackLayout { IsVisible = false, Children = { scrollView } }; + Assert.True(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + } + [Fact] public void CollapsedShellAncestorBeyondNonVisualLinksIsDetected() { From 74e9c16c54b3fcb26a0f184e60c90a6596214fd1 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Sat, 15 Aug 2026 19:08:35 +0200 Subject: [PATCH 08/13] Give a parked element request one contract: it lives exactly as long as its task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The visibility-based release added over the previous rounds (WillArrange and its OnIsVisibleChanged/OnParentSet re-checks) used "no collapsed VisualElement ancestor" as a proxy for "an arrange is coming", and every proxy for that leaks: a view removed from the tree keeps its handler (LayoutHandler.Remove detaches only the platform view) so its parked request was never retried and the task hung; an unselected Shell tab is never arranged while Shell.IsVisible is true; and — the real problem — releasing the task while leaving the request parked meant a completed await could scroll minutes later when the branch was next shown, with no way for the caller to cancel it. Replace all of it with a single invariant. A parked element request leaves the park in exactly two ways and no other: the arrange arrives and it is replayed against real geometry (the task completes with the scroll), or the view's lifecycle ends — its handler goes away or it is removed from the tree — and it is dropped (the task completes without a scroll). Nothing releases the task while a request is still parked, so a completed await never scrolls later; nothing but a lifecycle end drops the request, so a merely hidden view (collapsed branch, unselected tab) still scrolls to the element once shown. A view that stays attached and is never arranged keeps the task pending — that is the contract, not a leak. Removal from the tree is the one new lifecycle end covered, via the private-protected OnParentChangedCore hook so no public API surface is added. The completion source now runs continuations asynchronously, so a caller's await never resumes inline on the stack of the mutation that completed it — a handler change, a child removal, or a platform scroll callback — and cannot re-enter a half-finished parenting or property change. The task itself still completes synchronously. Verified: ScrollViewUnitTests 26/26 (three tests pin the contract's edges, one of them proven to fail without RunContinuationsAsynchronously; 27 consecutive green runs under load), Issue36801 + Issue36801DeferredElement + ShellFlyoutHeaderScrollViewContent iOS UI suites 12/12 on the simulator. Co-Authored-By: Claude Fable 5 --- .../PublicAPI/net/PublicAPI.Unshipped.txt | 3 +- .../src/Core/ScrollView/ScrollView.cs | 147 +++++++----------- .../Core.UnitTests/ScrollViewUnitTests.cs | 130 ++++++---------- 3 files changed, 102 insertions(+), 178 deletions(-) diff --git a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt index 0fbb1c723ed9..e0464b360b74 100644 --- a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt @@ -1,10 +1,9 @@ #nullable enable Microsoft.Maui.Controls.IndicatorView.~IndicatorView() -> void Microsoft.Maui.Controls.Label.~Label() -> void +Microsoft.Maui.Controls.ShellContent.~ShellContent() -> void override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void ~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void -override Microsoft.Maui.Controls.ScrollView.OnParentSet() -> void override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? propertyName = null) -> void ~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void -Microsoft.Maui.Controls.ShellContent.~ShellContent() -> void diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index 60fc330f277b..3a4c93239158 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -43,6 +43,17 @@ public Rect LayoutAreaOverride ScrollToRequestedEventArgs _pendingScrollToRequested; bool _replayPendingScrollToRequestedEvent; + // A parked element request lives exactly as long as the task the caller is awaiting. + // It leaves the park in one of two ways, and no other: the arrange arrives and the + // request is replayed against real geometry (the task completes with the scroll), + // or the view's lifecycle ends — its handler goes away or it is removed from the + // tree — and the request is dropped (the task completes without a scroll). Nothing + // releases the task while the request is still parked, so a completed await never + // scrolls later; and nothing but a lifecycle end drops the request, so a view that is + // merely hidden (a collapsed branch, an unselected tab) still scrolls to the element + // when it is eventually arranged. A view that stays attached and is never arranged + // keeps the task pending — that is the contract, not a leak: the task completes when + // the scroll happens or the view is torn down. private protected override void OnHandlerChangedCore() { base.OnHandlerChangedCore(); @@ -50,46 +61,60 @@ private protected override void OnHandlerChangedCore() if (Handler is null) { // The handler went away with a request still queued, so nothing will ever - // dispatch it. Release the caller rather than leaving its task pending - // forever; Core does the same for its own pending request on disconnect. - if (_pendingScrollToRequested is not null) - { - _pendingScrollToRequested = null; - SendScrollFinished(); - } - + // dispatch it; Core does the same for its own pending request on disconnect + DropPendingScrollToRequest(); return; } DispatchPendingScrollToRequest(); } - void DispatchPendingScrollToRequest() + private protected override void OnParentChangedCore() { - if (Handler is null || _pendingScrollToRequested is not { } pending) + base.OnParentChangedCore(); + + // Removed from the tree: no arrange will come from a parent that no longer + // exists, and the handler is not necessarily disconnected by the removal, so + // this is the other lifecycle end that drops a parked request + if (RealParent is null) { - return; + DropPendingScrollToRequest(); } + } - if (pending.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady()) + void DropPendingScrollToRequest() + { + if (_pendingScrollToRequested is null) { - if (!WillArrange()) - { - // A collapsed branch is skipped by layout, so no callback is coming to - // resolve the target. Release the caller now, but keep the request - // parked: if the branch is later shown, the first arrange replays it - // against real geometry so the scroll still lands on the element - // (a newer request supersedes it as usual). - SendScrollFinished(); - } + return; + } + + _pendingScrollToRequested = null; + // A stale replay flag from a pre-handler park must not carry over to a later request + _replayPendingScrollToRequestedEvent = false; - // The request has to wait; OnSizeAllocated and ContentSizeChanged retry it. + // Safe to complete here even though this runs from a lifecycle mutation: the + // completion source runs continuations asynchronously, so the caller never + // resumes inline on this stack (see CheckTaskCompletionSource) + SendScrollFinished(); + } + + void DispatchPendingScrollToRequest() + { + if (Handler is null || _pendingScrollToRequested is not { } pending) + { return; } if (pending.Mode == ScrollToMode.Element) { - // The geometry callbacks run while the pass that produced the sizes is still + if (!IsElementTargetGeometryReady()) + { + // The request has to wait; OnSizeAllocated and ContentSizeChanged retry it. + return; + } + + // Those callbacks run while the pass that produced the sizes is still // arranging children, so resolve on the next tick, once positions are final. // Posting on every retry is deliberate: SendPendingScrollToRequest is a no-op // once the request has been sent or superseded, so a dropped callback cannot @@ -111,64 +136,6 @@ void DispatchPendingScrollToRequest() bool IsElementTargetGeometryReady() => Width >= 0 && Height >= 0 && Content is not ({ Width: < 0 } or { Height: < 0 }); - // A parked element request is retried only from OnSizeAllocated/ContentSizeChanged, - // which fire when this view gets arranged — and a view inside a collapsed - // (IsVisible=false) branch is skipped by layout entirely, so a request parked there - // has no callback coming. In that state the awaiting caller is released immediately - // while the request stays parked: showing the branch arranges it and replays the - // scroll against real geometry (see DispatchPendingScrollToRequest). - // The walk checks IsVisible on every VisualElement ancestor, skipping over non-visual - // links in the chain (e.g. Shell's ShellContent/ShellSection) rather than stopping at - // them. Non-visual containers' own visibility semantics (a hidden tab, say) are - // deliberately not consulted: parking keeps the scroll correct if that container is - // ever shown, and a handler detach still completes the task. - // Evaluated when a request is parked, at every retry, and again when this view's own - // IsVisible or parent changes (OnIsVisibleChanged/OnParentSet) so a request parked - // while visible is released if the view is collapsed or reparented into a collapsed - // branch before it arranges. An *ancestor* collapsing after the park sends no signal - // down the tree; that request is released when the branch is shown again or the - // handler detaches, and watching every ancestor for it would cost more than it - // covers. - bool WillArrange() - { - for (Element element = this; element is not null; element = element.RealParent) - { - if (element is VisualElement { IsVisible: false }) - { - return false; - } - } - - return true; - } - - internal override void OnIsVisibleChanged(bool oldValue, bool newValue) - { - base.OnIsVisibleChanged(oldValue, newValue); - - // A request parked while visible loses its arrange callbacks if this view is - // collapsed before layout runs: release the caller now (the request stays parked - // and replays if the view is shown again, exactly as a request parked while - // already collapsed does) - ReleaseParkedRequestIfCollapsed(); - } - - protected override void OnParentSet() - { - base.OnParentSet(); - - // Same for a parked request carried into a collapsed branch by reparenting - ReleaseParkedRequestIfCollapsed(); - } - - void ReleaseParkedRequestIfCollapsed() - { - if (_pendingScrollToRequested is not null && !WillArrange()) - { - SendScrollFinished(); - } - } - void SendPendingScrollToRequest() { if (Handler is null || _pendingScrollToRequested is not { } pending) @@ -570,7 +537,12 @@ void CheckTaskCompletionSource() { _scrollCompletionSource.TrySetCanceled(); } - _scrollCompletionSource = new TaskCompletionSource(); + // The task can be completed from inside a lifecycle mutation (a handler change, the + // view leaving the tree) as well as from platform scroll callbacks. The caller's + // await continuation must never resume on that stack — it could re-enter a + // half-finished parenting or property change — so continuations always run + // asynchronously. The task itself still transitions to completed synchronously. + _scrollCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); } double GetCoordinate(Element item, string coordinateName, double coordinate) @@ -601,17 +573,10 @@ void OnScrollToRequested(ScrollToRequestedEventArgs e) // OnAppearing): resolving the element target now would compute against the -1 // never-arranged sentinels. Park it for the layout callbacks instead — the // subscribers were already notified above, so the replay must not re-raise. + // It stays parked until the arrange arrives or the view's lifecycle ends + // (see OnHandlerChangedCore); a hidden view scrolls once it is shown. _pendingScrollToRequested = e; _replayPendingScrollToRequestedEvent = false; - - if (!WillArrange()) - { - // A collapsed branch gets no layout callbacks, so parking alone would - // hang the caller. Complete the task now and leave the request parked: - // showing the branch arranges it and replays the scroll against real - // geometry. - SendScrollFinished(); - } } else { diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index b6426990f32a..8cc72a265043 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -488,7 +488,7 @@ public void ElementRequestWithHandlerAttachedWaitsForArrange() } [Fact] - public void ElementRequestOnCollapsedScrollViewReleasesCallerAndReplaysWhenShown() + public void ElementRequestOnHiddenScrollViewWaitsAndScrollsOnceShown() { var item = new View(); var layout = new StackLayout { Children = { item } }; @@ -497,131 +497,91 @@ public void ElementRequestOnCollapsedScrollViewReleasesCallerAndReplaysWhenShown var handler = new ViewportProviderHandlerStub(); scrollView.Handler = handler; - // A collapsed ScrollView is skipped by layout, so no geometry callback is coming: - // the caller must be released immediately instead of hanging forever... + // A hidden view is skipped by layout, so no arrange is coming yet. The request + // stays parked and the task stays pending: completing it here would either + // scroll to a target computed from unarranged geometry or leave a request alive + // after its await returned — the task means "the scroll happened or the view is + // gone", nothing in between. var task = scrollView.ScrollToAsync(item, ScrollToPosition.Center, false); - Assert.True(task.IsCompleted); - - // ...but the request must not resolve against the -1 never-arranged sentinels — - // it stays parked instead of clamping a garbage target to the origin + Assert.False(task.IsCompleted); Assert.Empty(handler.ScrollToRequests); - // Showing the branch arranges it; the first arrange replays the parked request - // against real geometry so the scroll still lands on the element + // Showing the view arranges it; the first arrange replays the request against + // real geometry so the scroll lands on the element: Center = 450 - 100/2 + 50/2 scrollView.IsVisible = true; item.Layout(new Graphics.Rect(0, 450, 100, 50)); layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); - // Center: 450 - 100/2 + 50/2 var request = Assert.Single(handler.ScrollToRequests); Assert.Equal(425, request.VerticalOffset); - } - - [Fact] - public void DeferredElementRequestOnCollapsedAncestorReleasesCallerOnAttach() - { - var item = new View(); - var layout = new StackLayout { Children = { item } }; - var scrollView = new ScrollView { Content = layout }; - var parent = new StackLayout { IsVisible = false, Children = { scrollView } }; - // Parked because the handler is missing, not because of visibility - var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); - - var handler = new ViewportProviderHandlerStub(); - scrollView.Handler = handler; - - // On attach the collapsed ancestor means no arrange is coming: release the - // caller, but keep the request parked rather than resolving it against the - // never-arranged sentinels + scrollView.SendScrollFinished(); Assert.True(task.IsCompleted); - Assert.Empty(handler.ScrollToRequests); - - // Showing the ancestor arranges the branch and replays the request for real - parent.IsVisible = true; - item.Layout(new Graphics.Rect(0, 450, 100, 50)); - layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); - scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); - - // Start aligns the element's top with the viewport top - var request = Assert.Single(handler.ScrollToRequests); - Assert.Equal(450, request.VerticalOffset); } [Fact] - public void ElementRequestParkedWhileVisibleIsReleasedWhenCollapsedBeforeArrange() + public async Task ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree() { var item = new View(); var layout = new StackLayout { Children = { item } }; var scrollView = new ScrollView { Content = layout }; + var parent = new StackLayout { Children = { scrollView } }; var handler = new ViewportProviderHandlerStub(); scrollView.Handler = handler; - // Parked while visible with geometry not ready — the OnAppearing ordering + // Parked with the handler attached, waiting for the arrange var task = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); Assert.False(task.IsCompleted); - Assert.Empty(handler.ScrollToRequests); - // Collapsing before the first arrange removes the callbacks that would retry - // it: the caller must be released rather than left pending forever, and the - // request must stay parked instead of resolving against unarranged geometry - scrollView.IsVisible = false; + // Removing the view from the tree ends its lifecycle for layout purposes: no + // arrange will ever come from a parent it no longer has, and the removal does + // not disconnect its handler — so the request must be dropped and the caller + // released here rather than left pending forever + parent.Children.Remove(scrollView); + + await task.WaitAsync(TimeSpan.FromSeconds(5)); Assert.True(task.IsCompleted); Assert.Empty(handler.ScrollToRequests); - // Shown again, the first arrange still replays it against real geometry: - // End = 450 - 100 + 50 - scrollView.IsVisible = true; + // And dropped means dropped: re-attaching and arranging later must not resurrect + // the stale request into a scroll the caller no longer expects + parent.Children.Add(scrollView); item.Layout(new Graphics.Rect(0, 450, 100, 50)); layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); - - var request = Assert.Single(handler.ScrollToRequests); - Assert.Equal(400, request.VerticalOffset); - } - - [Fact] - public void ElementRequestParkedWhileVisibleIsReleasedWhenReparentedIntoCollapsedBranch() - { - var item = new View(); - var layout = new StackLayout { Children = { item } }; - var scrollView = new ScrollView { Content = layout }; - - var handler = new ViewportProviderHandlerStub(); - scrollView.Handler = handler; - - var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); - Assert.False(task.IsCompleted); - - // Moving the ScrollView under a collapsed parent means no arrange is coming - // from there either: the parent change must release the caller - _ = new StackLayout { IsVisible = false, Children = { scrollView } }; - Assert.True(task.IsCompleted); Assert.Empty(handler.ScrollToRequests); } [Fact] - public void CollapsedShellAncestorBeyondNonVisualLinksIsDetected() + public async Task ScrollCompletionNeverResumesTheCallerInlineOnTheMutationStack() { var item = new View(); - var layout = new StackLayout { Children = { item } }; - var scrollView = new ScrollView { Content = layout }; - var page = new ContentPage { Content = scrollView }; - var shell = new Shell { IsVisible = false }; - shell.Items.Add(new ShellContent { Content = page }); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var parent = new StackLayout { Children = { scrollView } }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + // The task is completed from inside a lifecycle mutation (child removal). The + // caller's continuation must not run inline on that mutation's stack, where it + // could re-enter a half-finished parenting operation. A continuation that asks + // to run synchronously would do exactly that if the completion source allowed + // it, so it must instead land on a different thread than the one mutating. + var mutatingThread = Environment.CurrentManagedThreadId; + var continuationThread = -1; + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + var continuation = task.ContinueWith( + _ => continuationThread = Environment.CurrentManagedThreadId, + TaskContinuationOptions.ExecuteSynchronously); - var handler = new ViewportProviderHandlerStub(); - scrollView.Handler = handler; + parent.Children.Remove(scrollView); - // The ancestor chain crosses non-VisualElement links (ShellContent/ShellSection) - // before reaching the collapsed Shell: the visibility walk must skip over them - // rather than stop, so the caller is released immediately instead of parking - // behind an arrange that never comes - var task = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); + // The task itself completes synchronously with the drop... Assert.True(task.IsCompleted); - Assert.Empty(handler.ScrollToRequests); + + // ...but the continuation was pushed off the mutating thread's stack + await continuation.WaitAsync(TimeSpan.FromSeconds(5)); + Assert.NotEqual(mutatingThread, continuationThread); } [Fact] From 6ec1c5a74250849daa6938ac12481191bab6e154 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Sat, 15 Aug 2026 19:43:56 +0200 Subject: [PATCH 09/13] Restore the PublicAPI files to the merge target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The branch adds no public API (the tree-removal hook uses the private-protected OnParentChangedCore), so both PublicAPI.Unshipped.txt files are checked out from upstream/inflight/current verbatim — removing the BOM, re-sort, duplicated entries and stale OnParentSet line that an earlier formatter pass had introduced. Co-Authored-By: Claude Fable 5 --- .../Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt | 5 ----- .../src/Core/PublicAPI/net/PublicAPI.Unshipped.txt | 12 ++++++------ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt index dc346e1805e9..8c21c75515e6 100644 --- a/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt @@ -1,11 +1,9 @@ #nullable enable -Microsoft.Maui.Controls.IndicatorView.~IndicatorView() -> void Microsoft.Maui.Controls.Label.~Label() -> void override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void override Microsoft.Maui.Controls.Handlers.Compatibility.EntryCellRenderer.EntryCellTableViewCell.Dispose(bool disposing) -> void override Microsoft.Maui.Controls.Handlers.Compatibility.TableViewModelRenderer.Dispose(bool disposing) -> void override Microsoft.Maui.Controls.Handlers.Items.MauiCollectionView.AddSubview(UIKit.UIView! view) -> void -override Microsoft.Maui.Controls.Handlers.Items.MauiCollectionView.LayoutSubviews() -> void override Microsoft.Maui.Controls.Handlers.Items.TemplatedCell.Dispose(bool disposing) -> void override Microsoft.Maui.Controls.Handlers.Items2.CarouselViewController2.Dispose(bool disposing) -> void ~override Microsoft.Maui.Controls.Handlers.Items2.CollectionViewHandler2.DisconnectHandler(UIKit.UIView platformView) -> void @@ -15,11 +13,8 @@ override Microsoft.Maui.Controls.Platform.Compatibility.ShellFlyoutContentRender ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellFlyoutRenderer.ViewWillTransitionToSize(CoreGraphics.CGSize toSize, UIKit.IUIViewControllerTransitionCoordinator coordinator) -> void override Microsoft.Maui.Controls.Platform.Compatibility.ShellItemRenderer.ViewDidAppear(bool animated) -> void ~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.DidMoveToParentViewController(UIKit.UIViewController parent) -> void -~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.TraitCollectionDidChange(UIKit.UITraitCollection previousTraitCollection) -> void -~override Microsoft.Maui.Controls.Platform.Compatibility.ShellSectionRenderer.ViewWillTransitionToSize(CoreGraphics.CGSize toSize, UIKit.IUIViewControllerTransitionCoordinator coordinator) -> void override Microsoft.Maui.Controls.Platform.Compatibility.ShellTableViewController.LoadView() -> void ~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void -override Microsoft.Maui.Controls.ScrollView.OnParentSet() -> void override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? propertyName = null) -> void ~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void diff --git a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt index e0464b360b74..9a3bc227e04a 100644 --- a/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt @@ -1,9 +1,9 @@ -#nullable enable +#nullable enable +override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? propertyName = null) -> void +~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void +override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void +override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void Microsoft.Maui.Controls.IndicatorView.~IndicatorView() -> void +~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void Microsoft.Maui.Controls.Label.~Label() -> void Microsoft.Maui.Controls.ShellContent.~ShellContent() -> void -override Microsoft.Maui.Controls.GraphicsView.OnBindingContextChanged() -> void -~override Microsoft.Maui.Controls.RadioButton.OnPropertyChanged(string propertyName = null) -> void -override Microsoft.Maui.Controls.Shapes.Shape.OnPropertyChanged(string? propertyName = null) -> void -~override Microsoft.Maui.Controls.SwipeItems.OnPropertyChanged(string propertyName = null) -> void -override Microsoft.Maui.Controls.TitleBar.OnBindingContextChanged() -> void From c56e736bb04cbd38e851f10ffdab73a703901531 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Sun, 16 Aug 2026 12:22:39 +0200 Subject: [PATCH 10/13] Scope the deferred completion to the lifecycle-drop path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RunContinuationsAsynchronously on the shared completion source changed the timing of every await ScrollToAsync(...) on every platform: the normal platform-callback completion used to resume the caller inline on the UI thread, and with the flag it was re-posted a dispatcher turn later, reordering user code behind any queued UI work. The re-entrancy requirement is narrow — only DropPendingScrollToRequest completes the task from inside a lifecycle mutation — so only that path now defers, by posting SendScrollFinished through the dispatcher. The completion source construction is back to its pre-PR form and ordinary continuations keep their existing timing. The regression test now asserts the property directly and deterministically: it captures dispatcher posts, checks the task is still pending after the removing mutation returns with exactly one post queued, and that running the post completes it — proven to fail when the drop completes inline. The previous thread-identity assertion depended on thread-pool scheduling and could fail a correct implementation under a saturated pool. Verified: ScrollViewUnitTests 26/26 (10 consecutive runs), Issue36801 + Issue36801DeferredElement + ShellFlyoutHeaderScrollViewContent iOS UI suites 12/12 on the simulator. Co-Authored-By: Claude Fable 5 --- .../src/Core/ScrollView/ScrollView.cs | 18 +++--- .../Core.UnitTests/ScrollViewUnitTests.cs | 59 +++++++++++-------- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index 3a4c93239158..aa8f13b759c1 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -93,10 +93,13 @@ void DropPendingScrollToRequest() // A stale replay flag from a pre-handler park must not carry over to a later request _replayPendingScrollToRequestedEvent = false; - // Safe to complete here even though this runs from a lifecycle mutation: the - // completion source runs continuations asynchronously, so the caller never - // resumes inline on this stack (see CheckTaskCompletionSource) - SendScrollFinished(); + // This runs from inside a lifecycle mutation (a handler change, the view leaving + // the tree), and the caller's await continuation must not resume in the middle of + // it — it could re-enter a half-finished parenting or property change. Post the + // completion instead of raising it inline. Only this path defers: the platform + // scroll callbacks complete the task exactly as before, so ordinary + // `await ScrollToAsync(...)` continuations keep their existing timing. + Dispatcher.Dispatch(SendScrollFinished); } void DispatchPendingScrollToRequest() @@ -537,12 +540,7 @@ void CheckTaskCompletionSource() { _scrollCompletionSource.TrySetCanceled(); } - // The task can be completed from inside a lifecycle mutation (a handler change, the - // view leaving the tree) as well as from platform scroll callbacks. The caller's - // await continuation must never resume on that stack — it could re-enter a - // half-finished parenting or property change — so continuations always run - // asynchronously. The task itself still transitions to completed synchronously. - _scrollCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _scrollCompletionSource = new TaskCompletionSource(); } double GetCoordinate(Element item, string coordinateName, double coordinate) diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index 8cc72a265043..a429c0a8c88f 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -1,5 +1,6 @@ using System; using System.Threading.Tasks; +using Microsoft.Maui.UnitTests; using NSubstitute; using Xunit; @@ -555,33 +556,39 @@ public async Task ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree() } [Fact] - public async Task ScrollCompletionNeverResumesTheCallerInlineOnTheMutationStack() + public void DroppedRequestCompletesThroughTheDispatcherNotInlineOnTheMutationStack() { - var item = new View(); - var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; - var parent = new StackLayout { Children = { scrollView } }; - scrollView.Handler = new ViewportProviderHandlerStub(); - - // The task is completed from inside a lifecycle mutation (child removal). The - // caller's continuation must not run inline on that mutation's stack, where it - // could re-enter a half-finished parenting operation. A continuation that asks - // to run synchronously would do exactly that if the completion source allowed - // it, so it must instead land on a different thread than the one mutating. - var mutatingThread = Environment.CurrentManagedThreadId; - var continuationThread = -1; - var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); - var continuation = task.ContinueWith( - _ => continuationThread = Environment.CurrentManagedThreadId, - TaskContinuationOptions.ExecuteSynchronously); - - parent.Children.Remove(scrollView); - - // The task itself completes synchronously with the drop... - Assert.True(task.IsCompleted); - - // ...but the continuation was pushed off the mutating thread's stack - await continuation.WaitAsync(TimeSpan.FromSeconds(5)); - Assert.NotEqual(mutatingThread, continuationThread); + // Capture dispatcher posts instead of running them, so the test can observe + // exactly when the completion is raised relative to the mutation. The option is + // thread-static and this capture swallows posts, so it must be restored. + var posted = new System.Collections.Generic.List(); + DispatcherProviderStubOptions.InvokeOnMainThread = posted.Add; + try + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var parent = new StackLayout { Children = { scrollView } }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // The drop runs from inside a lifecycle mutation (child removal). Completing + // the task inline there would resume the caller's continuation in the middle + // of a half-finished parenting operation, so the completion must be posted: + // after Remove returns the task is still pending and exactly one post is queued + parent.Children.Remove(scrollView); + Assert.False(task.IsCompleted); + var completion = Assert.Single(posted); + + // Running the post — what the real dispatcher does on its next turn — completes it + completion(); + Assert.True(task.IsCompleted); + } + finally + { + DispatcherProviderStubOptions.InvokeOnMainThread = null; + } } [Fact] From fa03bc0477bea6caac62cdda151b08677ef7fc22 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Mon, 17 Aug 2026 00:59:29 +0200 Subject: [PATCH 11/13] Complete a dropped request inline, bound to the request being dropped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Posting the drop's completion through the dispatcher opened a window in which a newer ScrollToAsync could replace _scrollCompletionSource before the post ran: the post then released the new caller's task while its request was still parked (a completed await that scrolls later — the exact thing the contract forbids) and the dropped caller's task was never completed. Reproduced: T1.completed=false, T2.completed=true. The deferral only ever existed to keep a caller's continuation off the lifecycle-mutation stack, and that is not the convention here: the already-shipped handler-detach drop and Core's DisconnectHandler both complete the task inline from their lifecycle hooks. Completing inline at the drop binds the release to the request being dropped by construction — no window, no captured identity, no interleaving — and matches the shipped path exactly. The completion source construction is unchanged from before this PR. Enumerating the deferral state machine for coverage also surfaced one real hole in the geometry gate: with Content set to null while an element request was parked, `Content is not ({Width:<0} or {Height:<0})` was trivially true for null, so the ScrollView's own layout dispatched a target computed against no content. The gate now requires arranged content; with none there is nothing to resolve the element against, so the request waits for content or a lifecycle end. Nine tests pin the remaining combinations: drop-then-new-request on both lifecycle ends (T1 completes, T2 untouched and scrolls), consecutive and doubled lifecycle ends, pre-handler parks (element and offset mode) leaving the tree, supersede while geometry-parked, content replacement, and content removal. Verified: ScrollViewUnitTests 34/34 (10 consecutive runs), Issue36801 + Issue36801DeferredElement + ShellFlyoutHeaderScrollViewContent iOS UI suites 12/12 on the simulator. Co-Authored-By: Claude Fable 5 --- .../src/Core/ScrollView/ScrollView.cs | 21 +- .../Core.UnitTests/ScrollViewUnitTests.cs | 273 +++++++++++++++--- 2 files changed, 248 insertions(+), 46 deletions(-) diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index aa8f13b759c1..d3150162efda 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -93,13 +93,14 @@ void DropPendingScrollToRequest() // A stale replay flag from a pre-handler park must not carry over to a later request _replayPendingScrollToRequestedEvent = false; - // This runs from inside a lifecycle mutation (a handler change, the view leaving - // the tree), and the caller's await continuation must not resume in the middle of - // it — it could re-enter a half-finished parenting or property change. Post the - // completion instead of raising it inline. Only this path defers: the platform - // scroll callbacks complete the task exactly as before, so ordinary - // `await ScrollToAsync(...)` continuations keep their existing timing. - Dispatcher.Dispatch(SendScrollFinished); + // Complete inline, at the moment the request is dropped. Deferring the completion + // would open a window in which a newer ScrollToAsync could swap the completion + // source, so a deferred completion would release the wrong task and orphan this + // one. Completing here binds the release to the request being dropped by + // construction. This is also the convention already in place: the handler-detach + // drop and Core's own DisconnectHandler both complete the task inline from their + // lifecycle hooks. + SendScrollFinished(); } void DispatchPendingScrollToRequest() @@ -135,9 +136,11 @@ void DispatchPendingScrollToRequest() // then is garbage. The content check must be "not yet arranged" rather than "arranged // to nothing": content can legitimately arrange to a zero size (a collapsed container), // and that raises no further callbacks — gating on the size would hang the caller's - // task forever, while dispatching just clamps the target to the origin. + // task forever, while dispatching just clamps the target to the origin. No content at + // all is different again: there is nothing to resolve the element against, so the + // request keeps waiting (for content to be set and arranged, or a lifecycle end). bool IsElementTargetGeometryReady() => - Width >= 0 && Height >= 0 && Content is not ({ Width: < 0 } or { Height: < 0 }); + Width >= 0 && Height >= 0 && Content is { Width: >= 0, Height: >= 0 }; void SendPendingScrollToRequest() { diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index a429c0a8c88f..780174df7867 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -1,6 +1,5 @@ using System; using System.Threading.Tasks; -using Microsoft.Maui.UnitTests; using NSubstitute; using Xunit; @@ -522,7 +521,7 @@ public void ElementRequestOnHiddenScrollViewWaitsAndScrollsOnceShown() } [Fact] - public async Task ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree() + public void ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree() { var item = new View(); var layout = new StackLayout { Children = { item } }; @@ -539,10 +538,8 @@ public async Task ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree() // Removing the view from the tree ends its lifecycle for layout purposes: no // arrange will ever come from a parent it no longer has, and the removal does // not disconnect its handler — so the request must be dropped and the caller - // released here rather than left pending forever + // released here, at the moment of removal, rather than left pending forever parent.Children.Remove(scrollView); - - await task.WaitAsync(TimeSpan.FromSeconds(5)); Assert.True(task.IsCompleted); Assert.Empty(handler.ScrollToRequests); @@ -556,39 +553,241 @@ public async Task ElementRequestIsDroppedAndCompletedWhenTheViewLeavesTheTree() } [Fact] - public void DroppedRequestCompletesThroughTheDispatcherNotInlineOnTheMutationStack() + public void DropCompletesTheDroppedTaskAndOnlyThatTask() { - // Capture dispatcher posts instead of running them, so the test can observe - // exactly when the completion is raised relative to the mutation. The option is - // thread-static and this capture swallows posts, so it must be restored. - var posted = new System.Collections.Generic.List(); - DispatcherProviderStubOptions.InvokeOnMainThread = posted.Add; - try - { - var item = new View(); - var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; - var parent = new StackLayout { Children = { scrollView } }; - scrollView.Handler = new ViewportProviderHandlerStub(); - - var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); - Assert.False(task.IsCompleted); - - // The drop runs from inside a lifecycle mutation (child removal). Completing - // the task inline there would resume the caller's continuation in the middle - // of a half-finished parenting operation, so the completion must be posted: - // after Remove returns the task is still pending and exactly one post is queued - parent.Children.Remove(scrollView); - Assert.False(task.IsCompleted); - var completion = Assert.Single(posted); - - // Running the post — what the real dispatcher does on its next turn — completes it - completion(); - Assert.True(task.IsCompleted); - } - finally - { - DispatcherProviderStubOptions.InvokeOnMainThread = null; - } + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var parent = new StackLayout { Children = { scrollView } }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + // T1 parks (handler attached, geometry not ready) + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task1.IsCompleted); + + // The view leaves the tree: T1's request is dropped and T1 completes at that + // moment — bound to the request being dropped, with no window in which anything + // else could be released in its place + parent.Children.Remove(scrollView); + Assert.True(task1.IsCompleted); + + // Re-attached, a new request T2 parks. It is a different request with its own + // task: nothing about the earlier drop may touch it + parent.Children.Add(scrollView); + var task2 = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); + Assert.False(task2.IsCompleted); + + // T2 stays pending until its own scroll: the arrange replays it and lands on the + // element (End = 450 - 100 + 50), and only then does its task complete + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + + var handler = (ViewportProviderHandlerStub)scrollView.Handler; + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(400, request.VerticalOffset); + Assert.False(task2.IsCompleted); + + scrollView.SendScrollFinished(); + Assert.True(task2.IsCompleted); + } + + [Fact] + public void ConsecutiveDropsEachCompleteTheirOwnTask() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var parent = new StackLayout { Children = { scrollView } }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + parent.Children.Remove(scrollView); + Assert.True(task1.IsCompleted); + + // A second park-and-drop cycle on the same view: the second drop must complete + // the second task, and the first drop must have had no effect on it + parent.Children.Add(scrollView); + var task2 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task2.IsCompleted); + + parent.Children.Remove(scrollView); + Assert.True(task2.IsCompleted); + } + + [Fact] + public void NewerRequestSupersedesAGeometryParkedElementRequest() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // T1 parks for geometry with the handler attached + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.Empty(handler.ScrollToRequests); + + // A direct request while T1 is parked wins: it is sent now, and T1's request is + // cleared so the arrange cannot replay the stale target on top of it + var task2 = scrollView.ScrollToAsync(0, 100, false); + var direct = Assert.Single(handler.ScrollToRequests); + Assert.Equal(100, direct.VerticalOffset); + + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.Single(handler.ScrollToRequests); + + // The scroll completes the current (latest) request's task + scrollView.SendScrollFinished(); + Assert.True(task2.IsCompleted); + } + + [Fact] + public void HandlerDetachDropCompletesOnlyTheDroppedTaskAndDoesNotResurrect() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task1.IsCompleted); + + // Handler detach is the other lifecycle end: T1 completes at that moment + scrollView.Handler = null; + Assert.True(task1.IsCompleted); + + // A new handler and a new request: T2 is its own request, unaffected by the drop + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + var task2 = scrollView.ScrollToAsync(item, ScrollToPosition.End, false); + Assert.False(task2.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // The arrange replays exactly one request — T2's — and the dropped T1 request is + // not resurrected alongside it + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(400, request.VerticalOffset); + Assert.False(task2.IsCompleted); + + scrollView.SendScrollFinished(); + Assert.True(task2.IsCompleted); + } + + [Fact] + public void DoubleLifecycleEndOnOneRequestCompletesItOnceAndLeavesLaterRequestsAlone() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var parent = new StackLayout { Children = { scrollView } }; + scrollView.Handler = new ViewportProviderHandlerStub(); + + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + + // Both lifecycle ends fire for the same parked request: the first drops and + // completes it, the second must be a no-op (nothing left to drop) + parent.Children.Remove(scrollView); + Assert.True(task1.IsCompleted); + scrollView.Handler = null; + + // A later request on the revived view is untouched by either earlier end + parent.Children.Add(scrollView); + scrollView.Handler = new ViewportProviderHandlerStub(); + var task2 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task2.IsCompleted); + } + + [Fact] + public void PreHandlerParkedRequestIsDroppedWhenTheViewLeavesTheTree() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var parent = new StackLayout { Children = { scrollView } }; + + // Parked because there is no handler yet + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // Leaving the tree is a lifecycle end whether or not a handler ever attached + parent.Children.Remove(scrollView); + Assert.True(task.IsCompleted); + + // And a handler attaching afterwards must find nothing to replay + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void PreHandlerParkedOffsetRequestIsDroppedWhenTheViewLeavesTheTree() + { + var scrollView = new ScrollView { Content = new StackLayout() }; + var parent = new StackLayout { Children = { scrollView } }; + + // The contract is about parked requests of any mode, not only element mode + var task = scrollView.ScrollToAsync(0, 100, false); + Assert.False(task.IsCompleted); + + parent.Children.Remove(scrollView); + Assert.True(task.IsCompleted); + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void ParkedElementRequestSurvivesContentReplacementAndDrainsOnTheNewContent() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // The content is swapped while the request is parked. The retry is hooked to + // the content's SizeChanged, so it must be re-hooked to the new content — the + // element still belongs to the tree via the old layout, but the ScrollView's + // geometry callbacks now come from the new one + var newLayout = new StackLayout { Children = { item } }; + scrollView.Content = newLayout; + Assert.False(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + newLayout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(450, request.VerticalOffset); + } + + [Fact] + public void ParkedElementRequestWithContentRemovedWaitsThenCompletesOnLifecycleEnd() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + + // With no content there is no geometry to resolve against, so the request must + // keep waiting rather than dispatch a target computed from nothing + scrollView.Content = null; + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.False(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // It is still bound to the view's lifecycle: a detach releases it + scrollView.Handler = null; + Assert.True(task.IsCompleted); } [Fact] From 386214eb93b976b6f4f6a79087421fb1c1fa8efe Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Tue, 18 Aug 2026 10:06:14 +0200 Subject: [PATCH 12/13] Gate an element request on the geometry its target actually needs; cancel on reparent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Making the geometry gate require arranged content (previous commit) regressed a valid input: ScrollToAsync(scrollView, ...) targets the ScrollView itself, resolves to the origin without touching Content, and was previously dispatched on a content-less ScrollView — the stricter gate parked it forever. The gate is now per-target: the ScrollView as target needs only its own geometry; any other target sits inside the content and needs the content arranged too. That also resolves the null-content case properly instead of by proxy. A descendant target validated at request time can be orphaned while parked (the content it hung off is removed or replaced with content that no longer contains it); no arrange of this ScrollView can ever give it a position, so waiting would be waiting for nothing — the request is dropped and completed at the moment of orphaning, from the Content setter, rather than at a later arrange that may never come. Content replaced with a layout that still contains the target keeps waiting for that content's arrange as before. Removal from the tree includes the transient removal of a reparent: a request made against a tree position that no longer exists is cancelled and its task completes at the removal, and it is not carried into the new parent. That is now stated as the contract (the previous comment justified the drop with a claim that does not hold for a reparent) and pinned by a test. The drop is deliberately not deferred to watch for a re-add — a window between drop and completion is exactly what produced the task-identity bug fixed in the previous commit. The invariant comment now names orphaning alongside lifecycle end as a terminal condition, so it describes every exit from the park. Verified: ScrollViewUnitTests 37/37 (10 consecutive runs; self-target, orphan-by-removal, orphan-by-replacement and reparent-cancel each pinned), Issue36801 + Issue36801DeferredElement + ShellFlyoutHeaderScrollViewContent iOS UI suites 12/12 on the simulator. Co-Authored-By: Claude Fable 5 --- .../src/Core/ScrollView/ScrollView.cs | 86 +++++++++++++----- .../Core.UnitTests/ScrollViewUnitTests.cs | 91 ++++++++++++++++++- 2 files changed, 148 insertions(+), 29 deletions(-) diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index d3150162efda..5cec536cf7ed 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -46,14 +46,16 @@ public Rect LayoutAreaOverride // A parked element request lives exactly as long as the task the caller is awaiting. // It leaves the park in one of two ways, and no other: the arrange arrives and the // request is replayed against real geometry (the task completes with the scroll), - // or the view's lifecycle ends — its handler goes away or it is removed from the - // tree — and the request is dropped (the task completes without a scroll). Nothing - // releases the task while the request is still parked, so a completed await never - // scrolls later; and nothing but a lifecycle end drops the request, so a view that is - // merely hidden (a collapsed branch, an unselected tab) still scrolls to the element - // when it is eventually arranged. A view that stays attached and is never arranged - // keeps the task pending — that is the contract, not a leak: the task completes when - // the scroll happens or the view is torn down. + // or there is nothing left that could ever satisfy it and the request is dropped + // (the task completes without a scroll). The latter happens when the view's + // lifecycle ends — its handler goes away or it is removed from the tree — and when + // the target is orphaned from this ScrollView's content (see IsElementTargetOrphaned). + // Nothing releases the task while the request is still parked, so a completed await + // never scrolls later; and nothing but one of those terminal conditions drops the + // request, so a view that is merely hidden (a collapsed branch, an unselected tab) + // still scrolls to the element when it is eventually arranged. A view that stays + // attached and is never arranged keeps the task pending — that is the contract, not + // a leak: the task completes when the scroll happens or nothing can make it happen. private protected override void OnHandlerChangedCore() { base.OnHandlerChangedCore(); @@ -73,9 +75,16 @@ private protected override void OnParentChangedCore() { base.OnParentChangedCore(); - // Removed from the tree: no arrange will come from a parent that no longer - // exists, and the handler is not necessarily disconnected by the removal, so - // this is the other lifecycle end that drops a parked request + // Removed from the tree — the handler is not necessarily disconnected by the + // removal, so this is the other lifecycle end that drops a parked request. This + // includes the transient removal of a reparent: a request made against a tree + // position that no longer exists is cancelled and its task completes at the + // removal, and it is not carried over to the new parent (its arrange does not + // replay it). The drop is deliberately not deferred to "see whether a re-add + // follows": that would put a window between the drop and the completion, in + // which a newer request could be released in the old one's place. A caller that + // moves a ScrollView with a pending element scroll re-requests it in the new + // location. if (RealParent is null) { DropPendingScrollToRequest(); @@ -112,7 +121,17 @@ void DispatchPendingScrollToRequest() if (pending.Mode == ScrollToMode.Element) { - if (!IsElementTargetGeometryReady()) + if (IsElementTargetOrphaned(pending.Element)) + { + // The target no longer hangs off this ScrollView's content (the content + // was replaced or removed under a parked request), so no arrange can ever + // give it a position: nothing is left to wait for. Terminal, like a + // lifecycle end. + DropPendingScrollToRequest(); + return; + } + + if (!IsElementTargetGeometryReady(pending.Element)) { // The request has to wait; OnSizeAllocated and ContentSizeChanged retry it. return; @@ -130,17 +149,31 @@ void DispatchPendingScrollToRequest() SendPendingScrollToRequest(); } - // An element target is resolved against this ScrollView's geometry and the element's - // position inside the arranged content. Before the first layout pass Width/Height are - // still -1 (the never-arranged sentinel, for the content too), so a target computed - // then is garbage. The content check must be "not yet arranged" rather than "arranged - // to nothing": content can legitimately arrange to a zero size (a collapsed container), - // and that raises no further callbacks — gating on the size would hang the caller's - // task forever, while dispatching just clamps the target to the origin. No content at - // all is different again: there is nothing to resolve the element against, so the - // request keeps waiting (for content to be set and arranged, or a lifecycle end). - bool IsElementTargetGeometryReady() => - Width >= 0 && Height >= 0 && Content is { Width: >= 0, Height: >= 0 }; + // An element target is resolved against the geometry it actually depends on. Before + // the first layout pass Width/Height are still -1 (the never-arranged sentinel), so a + // target computed then is garbage. The ScrollView itself is a valid target and needs + // only its own geometry; any other target sits inside the content, so its position + // is meaningful only once the content has been arranged too. That content check must + // be "not yet arranged" rather than "arranged to nothing": content can legitimately + // arrange to a zero size (a collapsed container), and that raises no further + // callbacks — gating on the size would hang the caller's task forever, while + // dispatching just clamps the target to the origin. + bool IsElementTargetGeometryReady(Element target) + { + if (Width < 0 || Height < 0) + { + return false; + } + + return target == this || Content is { Width: >= 0, Height: >= 0 }; + } + + // A target validated at request time as belonging to this ScrollView can stop + // belonging to it while parked: the content it hung off was replaced or removed. Its + // coordinates then no longer relate to this ScrollView and no arrange of this + // ScrollView can change that, so waiting would be waiting for nothing. + bool IsElementTargetOrphaned(Element target) => + target != this && !CheckElementBelongsToScrollViewer(target); void SendPendingScrollToRequest() { @@ -320,6 +353,11 @@ public View Content OnPropertyChanged(); Handler?.UpdateValue(nameof(Content)); + + // A parked element request may have just been orphaned (its target hung off + // the old content) — resolve that now rather than at the next arrange, which + // may not come if this ScrollView is already laid out + DispatchPendingScrollToRequest(); } } @@ -568,7 +606,7 @@ void OnScrollToRequested(ScrollToRequestedEventArgs e) _pendingScrollToRequested = e; _replayPendingScrollToRequestedEvent = true; } - else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady()) + else if (e.Mode == ScrollToMode.Element && !IsElementTargetGeometryReady(e.Element)) { // The handler exists but layout has not run yet (e.g. ScrollToAsync from // OnAppearing): resolving the element target now would compute against the -1 diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index 780174df7867..8502d82a2abf 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -769,7 +769,7 @@ public void ParkedElementRequestSurvivesContentReplacementAndDrainsOnTheNewConte } [Fact] - public void ParkedElementRequestWithContentRemovedWaitsThenCompletesOnLifecycleEnd() + public void ParkedElementRequestWhoseTargetIsOrphanedByContentRemovalIsDroppedAndCompleted() { var item = new View(); var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; @@ -777,19 +777,100 @@ public void ParkedElementRequestWithContentRemovedWaitsThenCompletesOnLifecycleE scrollView.Handler = handler; var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); - // With no content there is no geometry to resolve against, so the request must - // keep waiting rather than dispatch a target computed from nothing + // Removing the content orphans the target: it no longer hangs off this ScrollView, + // so no arrange of this ScrollView can ever give it a position. There is nothing + // left to wait for — the request is dropped and the caller released at that + // moment (not at some later arrange that may never come), and no target computed + // against nothing is dispatched scrollView.Content = null; + Assert.True(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // A later arrange has nothing to replay scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void ParkedElementRequestWhoseTargetIsOrphanedByContentReplacementIsDroppedAndCompleted() + { + var item = new View(); + var scrollView = new ScrollView { Content = new StackLayout { Children = { item } } }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); Assert.False(task.IsCompleted); + + // Replacing the content with a layout that does not contain the target orphans it + // just as removal does: dropped and completed at the replacement. (Replacement + // with content that still contains the target instead keeps waiting for that + // content's arrange — see ParkedElementRequestSurvivesContentReplacement...) + var unrelated = new StackLayout(); + scrollView.Content = unrelated; + Assert.True(task.IsCompleted); Assert.Empty(handler.ScrollToRequests); - // It is still bound to the view's lifecycle: a detach releases it - scrollView.Handler = null; + unrelated.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.Empty(handler.ScrollToRequests); + } + + [Fact] + public void SelfTargetOnAContentlessScrollViewNeedsOnlyItsOwnGeometry() + { + // ScrollToAsync(scrollView, ...) is a valid request that resolves to the origin + // without touching Content, so a ScrollView with no content must not park it + // forever waiting for content that may never come + var scrollView = new ScrollView(); + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(scrollView, ScrollToPosition.Start, false); + + // Waits only for the ScrollView's own arrange... + Assert.False(task.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // ...then dispatches against its own geometry + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(0, request.VerticalOffset); + + scrollView.SendScrollFinished(); Assert.True(task.IsCompleted); } + [Fact] + public void ReparentingCancelsAPendingElementScroll() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var parent1 = new StackLayout { Children = { scrollView } }; + var parent2 = new StackLayout(); + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + var task = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + Assert.False(task.IsCompleted); + + // Moving the ScrollView passes through a removal. A request made against a tree + // position that no longer exists is cancelled at that removal: the task completes + // (without a scroll) and the request is not carried into the new parent — a + // caller that moves a ScrollView with a pending scroll re-requests it there + parent1.Children.Remove(scrollView); + Assert.True(task.IsCompleted); + parent2.Children.Add(scrollView); + + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + Assert.Empty(handler.ScrollToRequests); + } + [Fact] public void DeferredRequestReplaysEventForSubscribersAttachedWithTheHandler() { From efdf0d980cd6dbe69200fcf915143a5af47a8bbd Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Tue, 18 Aug 2026 11:02:46 +0200 Subject: [PATCH 13/13] Let a request made from inside the replayed ScrollToRequested event win over the replay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enumerating the re-entrancy surface (the mechanism is UI-thread-affine, so re-entrant callbacks are its only race surface) found one hole: a ScrollToRequested subscriber that issues a new ScrollToAsync from inside the replayed event — as a compatibility renderer scrolling would — had its request sent immediately, and then the replay continued and sent the stale request on top of it, landing the scroll on the old target. Reproduced: handler received [250, 100] for a re-entrant request of 250. The replay now yields when a request was made during the event. Every request creates a fresh completion source, so a changed source after the raise is the exact signal — the same identity discipline that fixed the dropped-task bug. Non-re-entrant subscribers see no change: the replay proceeds exactly as before. The drop path was checked for the symmetric hazard (a caller re-requesting from the continuation the inline completion runs) and is correct as is: state is cleared before the completion, so the re-entrant request parks as a fresh one; that is now pinned too. Both tests fail without the fix. Memory: the mechanism holds only value-typed native state, the parked target is already reachable through the tree, and the handler/platform view hold the virtual view weakly — no new retention. Verified: ScrollViewUnitTests 39/39 (10 consecutive runs), Issue36801 + Issue36801DeferredElement + ShellFlyoutHeaderScrollViewContent iOS UI suites 12/12 on the simulator. Co-Authored-By: Claude Fable 5 --- .../src/Core/ScrollView/ScrollView.cs | 11 ++++ .../Core.UnitTests/ScrollViewUnitTests.cs | 66 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/src/Controls/src/Core/ScrollView/ScrollView.cs b/src/Controls/src/Core/ScrollView/ScrollView.cs index 5cec536cf7ed..3764318aafb0 100644 --- a/src/Controls/src/Core/ScrollView/ScrollView.cs +++ b/src/Controls/src/Core/ScrollView/ScrollView.cs @@ -194,7 +194,18 @@ void SendPendingScrollToRequest() if (_replayPendingScrollToRequestedEvent) { _replayPendingScrollToRequestedEvent = false; + + // A subscriber may issue a new ScrollToAsync from inside the event (a + // compatibility renderer scrolling, say). That newer request wins: it has + // already been sent, and sending the stale replay after it would land the + // scroll on the old target. Every request creates a fresh completion source, + // so a changed source is the exact signal that one was made. + var replayed = _scrollCompletionSource; ScrollToRequested?.Invoke(this, pending); + if (!ReferenceEquals(_scrollCompletionSource, replayed)) + { + return; + } } Handler.Invoke(nameof(IScrollView.RequestScrollTo), ConvertRequestMode(pending).ToRequest()); diff --git a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs index 8502d82a2abf..d9d3bcb909de 100644 --- a/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs +++ b/src/Controls/tests/Core.UnitTests/ScrollViewUnitTests.cs @@ -871,6 +871,72 @@ public void ReparentingCancelsAPendingElementScroll() Assert.Empty(handler.ScrollToRequests); } + [Fact] + public void ReentrantRequestFromTheDropContinuationParksCleanly() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + var parent = new StackLayout { Children = { scrollView } }; + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + // The caller re-requests from its own continuation, which the inline drop runs + // synchronously in the middle of the removal. The mechanism must already have + // cleared its state by then, so the re-entrant request parks as a fresh T2 + // instead of being dropped by the same removal, double-completed, or lost. + Task task2 = null; + var task1 = scrollView.ScrollToAsync(item, ScrollToPosition.Start, false); + task1.ContinueWith(_ => task2 = scrollView.ScrollToAsync(item, ScrollToPosition.End, false), + TaskContinuationOptions.ExecuteSynchronously); + + parent.Children.Remove(scrollView); + Assert.True(task1.IsCompleted); + Assert.NotNull(task2); + Assert.False(task2.IsCompleted); + Assert.Empty(handler.ScrollToRequests); + + // T2 belongs to the new lifecycle: re-attached and arranged, it scrolls + parent.Children.Add(scrollView); + item.Layout(new Graphics.Rect(0, 450, 100, 50)); + layout.Layout(new Graphics.Rect(0, 0, 100, 1000)); + scrollView.Layout(new Graphics.Rect(0, 0, 100, 100)); + var request = Assert.Single(handler.ScrollToRequests); + Assert.Equal(400, request.VerticalOffset); + } + + [Fact] + public void ReentrantRequestFromTheReplayedEventSupersedesTheReplay() + { + var item = new View(); + var layout = new StackLayout { Children = { item } }; + var scrollView = new ScrollView { Content = layout }; + + // Parked before the handler exists, so the attach replays ScrollToRequested + _ = scrollView.ScrollToAsync(0, 100, false); + + // A subscriber (as a compatibility renderer would be) that issues a new request + // from inside the replayed event. The replay has already cleared the pending + // request before raising, so the re-entrant request is not clobbered — and it + // must win: the replay must not send the stale request to the handler on top of it. + Task reentrant = null; + var reentered = false; + ((IScrollViewController)scrollView).ScrollToRequested += (_, _) => + { + if (reentered) return; + reentered = true; + reentrant = scrollView.ScrollToAsync(0, 250, false); + }; + + var handler = new ViewportProviderHandlerStub(); + scrollView.Handler = handler; + + Assert.NotNull(reentrant); + // Latest request wins; exactly what was sent, in order: the re-entrant one first + // (sent immediately, handler present) — the stale replay must not follow it + Assert.Equal(new[] { 250d }, handler.ScrollToRequests.ConvertAll(r => r.VerticalOffset)); + } + [Fact] public void DeferredRequestReplaysEventForSubscribersAttachedWithTheHandler() {