ScrollView's ScrollToAsync doesn't complete when called thrice with the same value by KarthikRajaKalaimani · Pull Request #27300 · dotnet/maui

KarthikRajaKalaimani · 2025-01-23T12:53:02Z

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Issue Details:

When calling ScrollToAsync thrice with the same Y value on a ScrollView, the scrolling operation does not complete on iOS, Windows, and Catalyst platforms. This behavior is inconsistent with Android, where the scrolling operation completes as expected.

Root Cause:

ScrollToAsync hangs when called with the same position the scroll view is already at. On iOS, UIScrollView.SetContentOffset does not trigger ScrollAnimationEnded when the content offset doesn't actually so ScrollFinished() is never called and the Task never completes. On Windows, ScrollViewer.ChangeView has the same behavior when already at the target offset.change

Description of Change:

iOS (ScrollViewHandler.iOS.cs):
Before calling SetContentOffset, check whether the clamped target position already matches the current ContentOffset. If already at the target:

Skip the SetContentOffset call (no-op scroll)
Call ScrollFinished() directly to complete the pending Task
ScrollFinished) handles all other cases unchanged.

Windows (ScrollViewHandler.Windows.cs):
Before calling ChangeView, check whether the requested position matches the current VerticalOffset/HorizontalOffset. If already at the target, call ScrollFinished() and return early.

Tests (ScrollViewUITests.cs):
Removed three [FailsOn*WhenRunningOnXamarinUITest] attributes from ScrollToYTwice that were tracking this issue on iOS, Mac, and Windows.

Platform Affected

iOS (.ios.cs covers both iOS and MacCatalyst)
MacCatalyst (via ScrollViewHandler.iOS.cs)
Windows

Tested the behavior in the following platforms.

Android
Windows
iOS
Mac

Reference:

N/A

Issues Fixed:

Fixes #27250

Screenshots

Before	After
Before_27250.mov	After_27250.mov

KarthikRajaKalaimani · 2025-01-23T12:56:48Z

@dotnet-policy-service agree company="Syncfusion, Inc."

jsuarezruiz · 2025-01-24T07:18:09Z

/azp run

azure-pipelines · 2025-01-24T07:18:32Z

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz · 2025-01-28T10:36:26Z

/azp run

azure-pipelines · 2025-01-28T10:36:47Z

Azure Pipelines successfully started running 3 pipeline(s).

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

EmilienDup · 2025-06-19T07:34:28Z

Hi guys,

Any update on this PR?
We are also impacted by those never ending scrolling tasks.

Thank you.

jsuarezruiz · 2025-10-31T11:54:11Z

/rebase

jsuarezruiz · 2025-10-31T12:10:33Z

/azp run

azure-pipelines · 2025-10-31T12:10:57Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2026-01-30T16:04:08Z

/rebase

…s extraction (#33813)  > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Summary This PR significantly enhances the PR agent workflow with improvements across **two development phases**: ### Phase 1: Original Enhancements (Commits 1-8) 1. **Multi-model try-fix exploration** - Uses 5 different AI models to explore alternative solutions 2. **Environment blocker handling** - Strict rules to stop and ask user when environment issues occur 3. **Review automation script** - PowerShell script that invokes Copilot CLI directly for PR reviews 4. **Branch safety rules** - Prevents agent from switching branches during reviews 5. **Template formatting rules** - Exact template adherence for downstream script compatibility ### Phase 2: Consolidation & Simplification (Commit 9) After multi-model review (5 models: Claude Sonnet 4.5, Claude Opus 4.5, GPT-5.2, GPT-5.2-Codex, Gemini 3 Pro), the following improvements were made: 6. **Shared rules extraction** - Created `SHARED-RULES.md` to eliminate duplication across files 7. **Simplified git policy** - Agent never runs git commands; always assumes correct branch 8. **State file handling** - Changed "commit" to "save" (state files are gitignored) 9. **Reduced verbosity** - Compressed cross-pollination section, converted PLAN-TEMPLATE to checklist --- ## Commits ### 1. `80d7e412c2` - Update pr agent with multi-model try-fix workflow **Why:** The original PR agent only used a single model for exploring fixes. Different AI models have different strengths and may find solutions others miss. **Changes:** - Added Phase 4 multi-model workflow using 5 models: - `claude-sonnet-4.5`, `claude-opus-4.5`, `gpt-5.2`, `gpt-5.2-codex`, `gemini-3-pro-preview` - Cross-pollination loop: Share results between models to spark new ideas - Continue until all models confirm "no more approaches to explore" --- ### 2. `69cc6af403` - Address Copilot review suggestions **Why:** Initial PR review feedback suggested improvements. **Changes:** - Minor formatting and clarity improvements to pr.md --- ### 3. `fe55c3fd21` - Add rules for template formatting and skill script usage **Why:** Downstream scripts depend on exact regex patterns in state files. Agents were "improving" templates by adding attributes like `open` which broke parsing. **Changes:** - Added "Follow Templates EXACTLY" rule - no adding attributes, no improving formats - Added "Use Skills' Scripts" rule - run provided PowerShell scripts, don't bypass with manual commands --- ### 4. `debbee608e` - Add 'Stop on Environment Blockers' rule to PR agent **Why:** Agent was continuing through phases when environment issues (missing Appium, WinAppDriver errors) prevented completion, leading to incomplete reviews. **Changes:** - Added explicit blocker handling section to pr.md - Common blockers: Appium drivers, WinAppDriver, Xcode, emulators, port conflicts - Must STOP, report the blocker, and ask user how to proceed - Never mark phase as blocked and continue to next phase --- ### 5. `ad29f6a796` - Add PR review plan template and Review-PR.ps1 script **Why:** Need a reusable template for consistent PR reviews and a script to automate invocation. **Changes:** - Created `.github/agents/pr/PLAN-TEMPLATE.md` - Reusable 5-phase review plan - Created `.github/scripts/Review-PR.ps1` - Script to prepare environment and invoke Copilot CLI --- ### 6. `886ea2aa8e` - Improve blocker handling and fix Review-PR.ps1 for Copilot CLI **Why:** During PR #27300 review, agent spent 10+ tool calls troubleshooting WinAppDriver instead of stopping after first failure. **Changes:** - Added strict retry limits table: | Blocker Type | Max Retries | Action | |--------------|-------------|--------| | Server errors (500, timeout) | 0 | STOP immediately | | Missing tools | 1 install | STOP and ask | | Port conflicts | 1 kill | STOP and ask | | WinAppDriver errors | 0 | STOP immediately | - Added "What I tried" section to blocker report template - New prohibitions: Never spend more than 2-3 tool calls on same blocker --- ### 7. `d67da75e85` - Update Review-PR.ps1 to invoke Copilot CLI directly **Why:** Initially thought Copilot CLI was interactive-only. Discovered it supports `-i <prompt>` and `-p <prompt>` for programmatic invocation. **Changes:** - Script now invokes `copilot --agent pr -i "<prompt>"` directly - Validates both `gh` CLI and `copilot` CLI are installed - New `-NoInteractive` switch for `-p` mode (exits after completion) - Dry run mode shows exactly what would be invoked --- ### 8. `ed74c574a5` - Add 'Do NOT Switch Branches' rule to pr agent **Why:** During PR review testing, the pr agent ran `git checkout`, `git stash`, and other branch-switching commands, causing loss of local changes and confusion about which code was being reviewed. **Changes:** - Added explicit "Do NOT Switch Branches" rule to both pr.md and PLAN-TEMPLATE.md - Forbidden commands: `git checkout`, `git switch`, `gh pr checkout`, `git stash` - Agent must work on current branch as-is, using `git diff` or `gh pr diff` to see PR changes - Fixed variable expansion in Review-PR.ps1 prompt (double backticks for here-strings) --- ### 9. `632bfb7155` - Extract shared rules, simplify git policy, reduce duplication **Why:** Multi-model review (5 AI models) identified significant duplication (~200 lines) across files, conflicting "commit" terminology, and overly verbose sections. The git checkout prohibition also conflicted with workflow steps that mentioned git checkout. **Changes:** - **Created `SHARED-RULES.md`** (167 lines) - Single source of truth for: - Phase Completion Protocol - Follow Templates EXACTLY - No Direct Git Commands (absolute - agent never runs git) - Use Skills' Scripts - Stop on Environment Blockers (with retry limits) - Multi-Model Configuration (5 models list) - Platform Selection guidance - **Simplified git policy** - Agent is ALWAYS on correct branch, never runs git commands: - Removed git fetch/checkout from Phase 1 Pre-Flight - Phase 5: User handles commit/push/PR creation - Changed all "State file committed" → "State file saved" (gitignored files) - **Compressed content**: - Cross-pollination ASCII box: 51 → 20 lines - PLAN-TEMPLATE.md: Full docs → Pure checklist (226 → 112 lines) - pr.md: 662 → 535 lines (-19%) - post-gate.md: 403 → 302 lines (-25%) - Total reduction: 1291 → 1116 lines (-14%) - **Eliminated duplication**: - Blocker handling was in 3 files → now in SHARED-RULES.md only - Phase Completion Protocol was in 2 files → now in SHARED-RULES.md only - Model list was in 3 files → now in SHARED-RULES.md only --- ## Files Changed | File | Purpose | |------|---------| | `.github/agents/pr.md` | Main PR agent instructions (Phases 1-3) | | `.github/agents/pr/post-gate.md` | Phase 4-5 instructions (multi-model try-fix) | | `.github/agents/pr/PLAN-TEMPLATE.md` | **NEW** - Reusable 5-phase review checklist | | `.github/agents/pr/SHARED-RULES.md` | **NEW** - Extracted shared rules (single source of truth) | | `.github/scripts/Review-PR.ps1` | **NEW** - Script to invoke Copilot CLI for PR review | --- ## Usage ```powershell # Interactive mode (default) - stays open for follow-up pwsh .github/scripts/Review-PR.ps1 -PRNumber 33687 # Non-interactive mode - exits when done pwsh .github/scripts/Review-PR.ps1 -PRNumber 33687 -NoInteractive # Specific platform pwsh .github/scripts/Review-PR.ps1 -PRNumber 33687 -Platform ios # Skip merge if already on branch pwsh .github/scripts/Review-PR.ps1 -PRNumber 33687 -SkipMerge # Dry run to preview pwsh .github/scripts/Review-PR.ps1 -PRNumber 33687 -DryRun ``` --- ## Multi-Model Validation The final changes (commit 9) were validated by 5 AI models: | Model | Verdict | Key Feedback | |-------|---------|--------------| | Claude Sonnet 4.5 | ✅ READY TO MERGE | "All git command instructions successfully removed" | | Claude Opus 4.5 | ✅ READY TO MERGE | "Excellent refactoring, no conflicting guidance" | | GPT-5.2 | ✅ READY TO MERGE | "Progressive disclosure maintained" | | GPT-5.2-Codex | ✅ READY TO MERGE | "No instructions to run git commands remain" | | Gemini 3 Pro | ✅ READY TO MERGE | "Agent is instructed to STOP and ask user for commits" | --- ## Testing Tested by reviewing PR #27300 (ScrollView ScrollToAsync fix): - Pre-Flight phase completed successfully - Tests phase verified test files exist - Gate phase encountered WinAppDriver blocker → agent correctly stopped and asked - Blocker handling rules validated through real-world usage - **Branch safety verified**: Agent stayed on branch instead of switching --------- Co-authored-by: Jakub Florkowski <kubaflo123@gmail.com>

jfversluis

Code Review — PR #27300

Thanks for the fix! The diagnosis is correct — ScrollToAsync hangs when the ScrollView is already at the target position because the platform scroll-complete event never fires. The approach of calling ScrollFinished() in this case is the right solution.

However, the position check needs to happen before the scroll call on both platforms, not after. Here's why and what to change:

🔴 Windows: Race condition with async `ChangeView`

ChangeView() is asynchronous — it returns immediately while the actual scroll happens later. Checking offsets after ChangeView reads potentially stale/intermediate values. Additionally, if the position is already at the target, ChangeView may still fire ViewChanged → double ScrollFinished() call.

Current code (problematic):

handler.PlatformView.ChangeView(request.HorizontalOffset, request.VerticalOffset, null, request.Instant);
if(request.VerticalOffset == handler.PlatformView.VerticalOffset && request.HorizontalOffset == handler.PlatformView.HorizontalOffset)
{
   handler.VirtualView.ScrollFinished();
}

Suggested fix — check before, early return:

if (args is ScrollToRequest request)
{
    if (request.VerticalOffset == handler.PlatformView.VerticalOffset 
        && request.HorizontalOffset == handler.PlatformView.HorizontalOffset)
    {
        handler.VirtualView.ScrollFinished();
        return;
    }
    handler.PlatformView.ChangeView(request.HorizontalOffset, request.VerticalOffset, null, request.Instant);
}

This avoids calling ChangeView entirely when already at target — no race, no double-fire.

🔴 iOS: `SetContentOffset` to same position has undefined animation callback behavior

Calling SetContentOffset with animated: true when already at the target position may or may not fire scrollViewDidEndScrollingAnimation depending on iOS version. This means ScrollFinished() could be called twice — once from your new check and once from the animation callback.

Suggested fix — check before, skip SetContentOffset:

var minScrollHorizontal = Math.Clamp(request.HorizontalOffset, 0, availableScrollWidth);
var minScrollVertical = Math.Clamp(request.VerticalOffset, 0, availableScrollHeight);

bool alreadyAtTarget = uiScrollView.ContentOffset.Y == minScrollVertical 
    && uiScrollView.ContentOffset.X == minScrollHorizontal;

if (!alreadyAtTarget)
    uiScrollView.SetContentOffset(new CGPoint(minScrollHorizontal, minScrollVertical), !request.Instant);

if (request.Instant || alreadyAtTarget)
{
    scrollView.ScrollFinished();
}

This skips the SetContentOffset call entirely when already at target, eliminating ambiguous UIKit behavior.

🟡 Minor: Floating-point equality

Both platforms use == on doubles for position comparison. This works for the common case (exact integer values like scrolling to Y=100 twice), but could produce false negatives at content boundaries due to floating-point precision. Consider using a tolerance:

Math.Abs(request.VerticalOffset - handler.PlatformView.VerticalOffset) < 0.5

🟡 Minor: Indentation

The Windows change uses 3-space indentation while the file uses tabs. Running dotnet format should fix this.

Summary

The fix direction is correct and the ScrollToYTwice test re-enablement is appropriate. Just needs the position checks moved before the platform scroll calls to avoid race conditions and double-fire scenarios. ScrollFinished() itself is safe to double-call (TrySetResult), but the cleaner pattern avoids the issue entirely.

…with the same value Fixed ScrollView's ScrollToAsync doesn't complete when called thrice with the same value.

ScrollView's ScrollToAsync doesn't complete when called thrice with the same value

Test case enabled for this issue

github-actions · 2026-02-27T05:47:28Z

🚀 Dogfood this PR with:

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

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

Or

Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 27300"

KarthikRajaKalaimani · 2026-02-27T07:57:06Z

Code Review — PR #27300

Thanks for the fix! The diagnosis is correct — ScrollToAsync hangs when the ScrollView is already at the target position because the platform scroll-complete event never fires. The approach of calling ScrollFinished() in this case is the right solution.

However, the position check needs to happen before the scroll call on both platforms, not after. Here's why and what to change:

🔴 Windows: Race condition with async ChangeView

ChangeView() is asynchronous — it returns immediately while the actual scroll happens later. Checking offsets after ChangeView reads potentially stale/intermediate values. Additionally, if the position is already at the target, ChangeView may still fire ViewChanged → double ScrollFinished() call.

Current code (problematic):
handler.PlatformView.ChangeView(request.HorizontalOffset, request.VerticalOffset, null, request.Instant);
if(request.VerticalOffset == handler.PlatformView.VerticalOffset && request.HorizontalOffset == handler.PlatformView.HorizontalOffset)
{
   handler.VirtualView.ScrollFinished();
}
Suggested fix — check before, early return:
if (args is ScrollToRequest request)
{
    if (request.VerticalOffset == handler.PlatformView.VerticalOffset 
        && request.HorizontalOffset == handler.PlatformView.HorizontalOffset)
    {
        handler.VirtualView.ScrollFinished();
        return;
    }
    handler.PlatformView.ChangeView(request.HorizontalOffset, request.VerticalOffset, null, request.Instant);
}
This avoids calling ChangeView entirely when already at target — no race, no double-fire.

🔴 iOS: SetContentOffset to same position has undefined animation callback behavior

Calling SetContentOffset with animated: true when already at the target position may or may not fire scrollViewDidEndScrollingAnimation depending on iOS version. This means ScrollFinished() could be called twice — once from your new check and once from the animation callback.

Suggested fix — check before, skip SetContentOffset:
var minScrollHorizontal = Math.Clamp(request.HorizontalOffset, 0, availableScrollWidth);
var minScrollVertical = Math.Clamp(request.VerticalOffset, 0, availableScrollHeight);

bool alreadyAtTarget = uiScrollView.ContentOffset.Y == minScrollVertical 
    && uiScrollView.ContentOffset.X == minScrollHorizontal;

if (!alreadyAtTarget)
    uiScrollView.SetContentOffset(new CGPoint(minScrollHorizontal, minScrollVertical), !request.Instant);

if (request.Instant || alreadyAtTarget)
{
    scrollView.ScrollFinished();
}
This skips the SetContentOffset call entirely when already at target, eliminating ambiguous UIKit behavior.

🟡 Minor: Floating-point equality

Both platforms use == on doubles for position comparison. This works for the common case (exact integer values like scrolling to Y=100 twice), but could produce false negatives at content boundaries due to floating-point precision. Consider using a tolerance:
Math.Abs(request.VerticalOffset - handler.PlatformView.VerticalOffset) < 0.5
🟡 Minor: Indentation

The Windows change uses 3-space indentation while the file uses tabs. Running dotnet format should fix this.

Summary

The fix direction is correct and the ScrollToYTwice test re-enablement is appropriate. Just needs the position checks moved before the platform scroll calls to avoid race conditions and double-fire scenarios. ScrollFinished() itself is safe to double-call (TrySetResult), but the cleaner pattern avoids the issue entirely.

I have modified the fix as per the suggestion.

kubaflo · 2026-03-03T01:30:43Z

🤖 AI Summary

📊 Expand Full Review

🔍 Pre-Flight — Context & Validation

📝 Review Session — Modified the fix · 4ce7857

Issue: #27250 - ScrollView's ScrollToAsync doesn't complete when called thrice with the same value
PR: #27300 - ScrollView's ScrollToAsync doesn't complete when called thrice with the same value
Author: KarthikRajaKalaimani (Syncfusion)
Platforms Affected: iOS, Windows, MacCatalyst
Files Changed: 2 implementation files, 1 test file (test re-enablement)

Issue Summary

ScrollToAsync hangs indefinitely when called multiple times with the same Y position on iOS, Windows, and MacCatalyst. The root cause is that the platform scroll-complete event never fires because the ScrollView is already at the target position — so the ScrollToAsync Task never completes.

Android works correctly because it apparently handles this case differently.

Key Findings

Root cause: Platform scroll callbacks (ScrollAnimationEnded on iOS, ViewChanged on Windows) never fire when the scroll position doesn't change, leaving the Task in a pending state.
Fix direction: Check if already at target position BEFORE calling the platform scroll method. If already there, call ScrollFinished() directly.
Prior review by jfversluis: Reviewer identified that the original fix checked position AFTER calling the scroll method (race condition) and suggested checking BEFORE. The PR author updated the code accordingly.
PR has s/agent-changes-requested label from a previous agent review.

Reviewer Discussion

Issue	Reviewer Says	Current Status
Windows race condition	Check before `ChangeView`, not after	✅ Fixed in current code
iOS double-fire risk	Skip `SetContentOffset` when already at target	✅ Fixed in current code
Floating-point equality	Use tolerance instead of `==`	⚠️ NOT addressed (still using `==`)
Indentation	Windows/iOS use mixed indentation	⚠️ Partially fixed (still mixed spaces/tabs)

Files Changed

Fix files:

src/Core/src/Handlers/ScrollView/ScrollViewHandler.Windows.cs - Early return if already at target before calling ChangeView
src/Core/src/Handlers/ScrollView/ScrollViewHandler.iOS.cs - alreadyAtTarget check skips SetContentOffset and calls ScrollFinished() directly

Test files:

src/Controls/tests/TestCases.Shared.Tests/Tests/ScrollViewUITests.cs - Removed 3 [FailsOn...] attributes from ScrollToYTwice test

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #27300	Check position before platform scroll; call ScrollFinished() if already at target	⏳ PENDING (Gate)	`ScrollViewHandler.iOS.cs`, `ScrollViewHandler.Windows.cs`	Updated per reviewer suggestions

🚦 Gate — Test Verification

📝 Review Session — Modified the fix · 4ce7857

Result: ✅ PASSED
Platform: ios
Mode: Full Verification
Test Filter: ScrollToYTwice

Tests FAIL without fix ✅ (bug correctly detected)
Tests PASS with fix ✅ (fix correctly resolves the bug)

Fix files validated:

src/Core/src/Handlers/ScrollView/ScrollViewHandler.iOS.cs
src/Core/src/Handlers/ScrollView/ScrollViewHandler.Windows.cs

🔧 Fix — Analysis & Comparison

📝 Review Session — Modified the fix · 4ce7857

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #27300	Pre-check platform offsets in iOS/Windows handlers; call ScrollFinished() if already at target; skip scroll call	✅ PASS (Gate)	`ScrollViewHandler.iOS.cs`, `ScrollViewHandler.Windows.cs`	Original PR - validated by Gate
1	claude-sonnet-4.6	Cross-platform fix in ScrollView.cs: check ScrollX/ScrollY before invoking handler	✅ PASS	`ScrollView.cs` (+12 lines)	Single fix covers all platforms
2	claude-opus-4.6	Timeout safety net (1s) on TaskCompletionSource in ScrollView.cs	✅ PASS	`ScrollView.cs` (+9 lines)	Adds up to 1s latency — not ideal for production
3	gpt-5.2	CATransaction completion block on iOS + ChangeView return value check on Windows	✅ PASS	iOS + Windows handlers	CATransaction completion fires even for no-op
4	gpt-5.3-codex	Post-call reconciliation: check if scroll position changed after calling scroll method	✅ PASS	iOS + Windows handlers	Similar to pre-Reviewer-suggestion original PR code
5	gemini-3-pro	Post-SetContentOffset DispatchAsync to inspect Layer.AnimationKeys	✅ PASS	`ScrollViewHandler.iOS.cs`	Has race condition risk; complex
6	gemini-3-pro	Force animated=false when target matches current position in ScrollView.cs	✅ PASS	`ScrollView.cs` (+7 lines)	Elegant variant of Attempt 1
7	gpt-5.3-codex	Coalesce duplicate requests: return same in-flight Task if same target	✅ PASS	`ScrollView.cs` (+16 lines)	Changes semantics: multiple callers share one Task
8	claude-sonnet-4.6	Per-request TCS embedded in ScrollToRequest; handlers complete that TCS directly	✅ PASS	`ScrollToRequest.cs`, `ScrollView.cs`, `ScrollViewHandler.iOS.cs`	Architecturally clean; large change
9	claude-opus-4.6	Replace SetContentOffset(animated:true) with UIView.Animate() - guaranteed callback	✅ PASS	`ScrollViewHandler.iOS.cs`	Changes scroll animation behavior on iOS
10	gpt-5.2	Post-SetContentOffset DispatchAsync stability observer	✅ PASS	`ScrollViewHandler.iOS.cs`	Variant of Attempt 5; same race risk
11	gemini-3-pro	Clamped target resolution: clamp offsets to valid range on BOTH iOS and Windows before comparing	✅ PASS	iOS + Windows handlers	Improves PR's Windows fix by clamping

Exhausted: Yes — Round 5 cross-pollination: 4/5 models said NO NEW IDEAS; 1 remaining model (gpt-5.2) proposed CADisplayLink and UIViewPropertyAnimator which are variants of Attempts 5/9 respectively.

Selected Fix: PR's fix — The PR's platform-handler approach is the most appropriate fix at the correct abstraction level. It directly addresses the root cause where the platform scroll callback doesn't fire, using actual platform scroll position values (more accurate than cross-platform ScrollX/ScrollY). The iOS fix is complete and correct with proper clamping.

Key Findings

All 11 alternative approaches passed, confirming the fix direction is sound
The PR's iOS fix correctly clamps the target value before comparing (avoiding floating-point issues)
The PR's Windows fix uses raw request values (not clamped) - minor potential edge case
Both platform files have indentation issues that need formatting

Cross-Pollination Summary

Round	claude-sonnet	claude-opus	gpt-5.2	gpt-5.3-codex	gemini
2	NEW IDEA ✅	NEW IDEA ✅	NEW IDEA ✅	NEW IDEA ✅	NEW IDEA ✅
3	NEW IDEA ✅	NO NEW IDEAS	NEW IDEA ✅	NEW IDEA ✅	NEW IDEA ✅
4	NO NEW IDEAS	NO NEW IDEAS	NEW IDEA	NO NEW IDEAS	NO NEW IDEAS
5	NO NEW IDEAS	NO NEW IDEAS	NO NEW IDEAS*	NO NEW IDEAS	NO NEW IDEAS

*gpt-5.2 Round 5 proposed CADisplayLink (variant of Attempt 10) and UIViewPropertyAnimator (variant of Attempt 9) — not genuinely new approaches.

📋 Report — Final Recommendation

📝 Review Session — Modified the fix · 4ce7857

⚠️ Final Recommendation: REQUEST CHANGES

Summary

PR #27300 fixes ScrollToAsync hanging when called multiple times with the same target position. The fix is correct and the test passes. However, there are code quality issues that should be addressed before merge.

Root Cause

When ScrollToAsync is called with the same position the ScrollView is already at:

On iOS: UIScrollView.SetContentOffset(animated:true) executes but no animation starts, so scrollViewDidEndScrollingAnimation never fires, leaving the TaskCompletionSource pending forever.
On Windows: ScrollViewer.ChangeView() completes but the ViewChanged event may not fire for no-op scrolls.

Fix Quality

The fix direction is correct: check if already at the target position BEFORE calling the platform scroll method, and call ScrollFinished() directly if so.

iOS fix (ScrollViewHandler.iOS.cs): ✅ Correct — properly clamps the target to valid scroll range, detects already-at-target, skips SetContentOffset, calls ScrollFinished().

Windows fix (ScrollViewHandler.Windows.cs): ⚠️ Two issues:

Uses raw request.VerticalOffset instead of clamped values — edge case: requesting Y=2000 when max is Y=1000 won't match ScrollViewer.VerticalOffset (1000), causing ChangeView to be called unnecessarily.
3-space indentation instead of tabs (file uses tabs elsewhere).

iOS indentation: ⚠️ The if (!alreadyAtTarget) block uses 4 spaces instead of tabs.

Code Review Findings

🔴 Indentation Issues (both files)

The Windows change uses 3-space indentation and the iOS change uses 4-space indentation, while both files use tabs elsewhere. Running dotnet format would fix this.

// Windows - should use tabs:
-			if(request.VerticalOffset == handler.PlatformView.VerticalOffset && ...)
-			{
-			   handler.VirtualView.ScrollFinished();   // 3-space indent, not tab
-			   return;
-			}
+			if (request.VerticalOffset == handler.PlatformView.VerticalOffset && ...)
+			{
+				handler.VirtualView.ScrollFinished();  // tab-aligned
+				return;
+			}

🟡 Windows: Missing clamped value comparison

The iOS fix correctly compares against minScrollVertical (the clamped value), but the Windows fix compares against the raw request.VerticalOffset. If a user tries to scroll beyond content bounds (e.g., Y=2000 when max is 1000), the ScrollViewer.VerticalOffset would be 1000 but request.VerticalOffset is 2000 — they won't match, so ChangeView is called unnecessarily.

Suggested improvement (mirrors Attempt 11 from try-fix):

var maxHorizontal = Math.Max(handler.PlatformView.ExtentWidth - handler.PlatformView.ViewportWidth, 0);
var maxVertical = Math.Max(handler.PlatformView.ExtentHeight - handler.PlatformView.ViewportHeight, 0);
var horizontalOffset = Math.Clamp(request.HorizontalOffset, 0, maxHorizontal);
var verticalOffset = Math.Clamp(request.VerticalOffset, 0, maxVertical);

if (horizontalOffset == handler.PlatformView.HorizontalOffset && verticalOffset == handler.PlatformView.VerticalOffset)
{
    handler.VirtualView.ScrollFinished();
    return;
}
handler.PlatformView.ChangeView(horizontalOffset, verticalOffset, null, request.Instant);

🟡 Missing space after if keyword (Windows)

if(request.VerticalOffset... should be if (request.VerticalOffset...

✅ What Looks Good

iOS fix correctly clamps to valid scroll range before comparing
Test re-enablement is appropriate — 3 FailsOn attributes removed
Fix approach is minimal and targeted to the problem

Title Suggestion

[iOS][Windows] ScrollView: Fix ScrollToAsync hanging when called with same target position

Description Update Needed

Missing required NOTE block at top
"Description of Change" should describe the final implementation (check BEFORE calling SetContentOffset, skip the call when already at target) — the current text references the old approach

📋 Expand PR Finalization Review

Title: ✅ Good

Current: ScrollView's ScrollToAsync doesn't complete when called thrice with the same value

Description: ⚠️ Needs Update

Description needs updates. See details below.
Missing Elements:

**
Required NOTE block at top1.
Description of Change should clarify: the fix checks BEFORE calling the platform scroll API, skips the call when already at target, and calls ScrollFinished() directly2.
MacCatalyst should be explicitly noted3.

Recommendation: Add NOTE block + update Description of Change to accurately reflect the implementation. See recommended-description.md.

Phase 2: Code Review

See code-review.md for full findings.

Summary:

- - iOS logic is correct and well-structured
Test re-enablement is appropriate (3 FailsOn* attributes removed)

✨ Suggested PR Description

[!NOTE]
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Root Cause

ScrollToAsync hangs when called with the same position the scroll view is already at. On iOS, UIScrollView.SetContentOffset does not trigger ScrollAnimationEnded when the content offset doesn't actually so ScrollFinished() is never called and the Task never completes. On Windows, ScrollViewer.ChangeView has the same behavior when already at the target offset.change

Description of Change

iOS (ScrollViewHandler.iOS.cs):
Before calling SetContentOffset, check whether the clamped target position already matches the current ContentOffset. If already at the target:

Skip the SetContentOffset call (no-op scroll)
Call ScrollFinished() directly to complete the pending Task

ScrollFinished) handles all other cases unchanged.

Windows (ScrollViewHandler.Windows.cs):
Before calling ChangeView, check whether the requested position matches the current VerticalOffset/HorizontalOffset. If already at the target, call ScrollFinished() and return early.

Tests (ScrollViewUITests.cs):
Removed three [FailsOn*WhenRunningOnXamarinUITest] attributes from ScrollToYTwice that were tracking this issue on iOS, Mac, and Windows.

Platforms Affected

iOS (.ios.cs covers both iOS and MacCatalyst)
MacCatalyst (via ScrollViewHandler.iOS.cs)
Windows

Issues Fixed

Fixes #27250

Platforms Tested

Android
Windows
iOS
Mac

Code Review: ✅ Passed

Code Review: PR #27300

Windows: Missing value clamping before position comparison

File: src/Core/src/Handlers/ScrollView/ScrollViewHandler.Windows.cs

The Windows fix compares raw request.VerticalOffset and request.HorizontalOffset against the actual scroll position, without first clamping to the valid scroll range:

// Current (problematic):
if(request.VerticalOffset == handler.PlatformView.VerticalOffset && 
   request.HorizontalOffset == handler.PlatformView.HorizontalOffset)

Problem: If the user calls ScrollToAsync(0, 2000) when maximum vertical scroll is 1000, ScrollViewer.VerticalOffset will be 1000 (clamped by the platform) but request.VerticalOffset is 2000. They won't match, so ChangeView is called unnecessarily, and on a subsequent identical call the same mismatch the task never early-returns as expected.occurs

The iOS fix correctly clamps first:

var minScrollVertical = Math.Clamp(request.VerticalOffset, 0, availableScrollHeight);
bool alreadyAtTarget = uiScrollView.ContentOffset.Y == minScrollVertical && ...

Recommended fix for Windows (mirrors iOS approach):

var maxHorizontal = Math.Max(handler.PlatformView.ExtentWidth - handler.PlatformView.ViewportWidth, 0);
var maxVertical = Math.Max(handler.PlatformView.ExtentHeight - handler.PlatformView.ViewportHeight, 0);
var horizontalOffset = Math.Clamp(request.HorizontalOffset, 0, maxHorizontal);
var verticalOffset = Math.Clamp(request.VerticalOffset, 0, maxVertical);

if (horizontalOffset == handler.PlatformView.HorizontalOffset && verticalOffset == handler.PlatformView.VerticalOffset)
{
    handler.VirtualView.ScrollFinished();
    return;
}
handler.PlatformView.ChangeView(horizontalOffset, verticalOffset, null, request.Instant);

Formatting: Mixed indentation in iOS file

File: src/Core/src/Handlers/ScrollView/ScrollViewHandler.iOS.cs

The new if (!alreadyAtTarget) block uses spaces for indentation inside, while the rest of the file uses tabs:

if (!alreadyAtTarget)
{
    uiScrollView. spaces mixed with tabsSetContentOffset(...);  // 
}

Run dotnet format Microsoft.Maui.sln --no-restore --exclude Templates/src --exclude-diagnostics CA1822 to fix.

Formatting: Missing space after `if` keyword (Windows)

File: src/Core/src/Handlers/ScrollView/ScrollViewHandler.Windows.cs

// Current:
if(request.VerticalOffset == ...

// Should be:
if (request.VerticalOffset == ...

Looks Good

iOS logic is correct: Clamped values used for comparison, SetContentOffset skipped when unnecessary, ScrollFinished() called for both instant and already-at-target cases.
Test re-enablement is appropriate: Three [FailsOn*WhenRunningOnXamarinUITest] attributes correctly removed from these were tracking this exact bug.ScrollToYTwice
Minimal, targeted change: No unrelated modifications. The fix is well-scoped to the problem.
MacCatalyst covered: ScrollViewHandler.iOS.cs compiles for both iOS and MacCatalyst (.ios.cs convention), so the fix covers all three affected platforms from a single file.

dotnet-policy-service bot added the community ✨ Community Contribution label Jan 23, 2025

jsuarezruiz added the area-controls-scrollview ScrollView label Jan 23, 2025

karthikraja-arumugam added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Jan 24, 2025

KarthikRajaKalaimani marked this pull request as ready for review January 28, 2025 09:46

Copilot AI review requested due to automatic review settings January 28, 2025 09:46

KarthikRajaKalaimani requested a review from a team as a code owner January 28, 2025 09:46

KarthikRajaKalaimani requested review from mattleibow and rmarinho January 28, 2025 09:46

Copilot AI reviewed Apr 1, 2025

View reviewed changes

MartyIX added platform/macos macOS / Mac Catalyst platform/windows platform/android platform/ios labels Jun 19, 2025

github-actions bot force-pushed the fix-27250 branch from 306ee52 to 60d08d1 Compare October 31, 2025 11:55

KarthikRajaKalaimani force-pushed the fix-27250 branch from 60d08d1 to 11a48e6 Compare December 8, 2025 12:00

KarthikRajaKalaimani force-pushed the fix-27250 branch from 11a48e6 to 44d1865 Compare December 23, 2025 13:17

This was referenced Dec 23, 2025

CI UItests failure System.TimeoutException : Timed out waiting for element... #33220

Open

System.TimeoutException #33245

Open

github-actions bot force-pushed the fix-27250 branch from 44d1865 to ce1d567 Compare January 30, 2026 16:04

PureWeen removed the platform/macos macOS / Mac Catalyst label Jan 30, 2026

PureWeen removed the platform/android label Jan 30, 2026

build-analysis bot mentioned this pull request Jan 30, 2026

Project build failure causing test run abort in RunOniOS_BlazorRelease on mac_runios_blazor_release job #33744

Closed

PureWeen added the s/ai-reproduction-failed label Jan 31, 2026

PureWeen mentioned this pull request Jan 31, 2026

Enhance PR agent: multi-model workflow, blocker handling, shared rules extraction #33813

Merged

PureWeen added s/ai-reproduction-confirmed and removed s/ai-reproduction-failed labels Feb 1, 2026

kubaflo removed the s/ai-reproduction-confirmed label Feb 10, 2026

jfversluis mentioned this pull request Feb 24, 2026

Fix the OnNavigatedTo event and the OnAppearing event to return the ScrollToAsync method call #16619

Closed

jfversluis reviewed Feb 24, 2026

View reviewed changes

jfversluis added s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/pr-needs-author-input PR needs an update from the author labels Feb 24, 2026

KarthikRajaKalaimani added 3 commits February 27, 2026 11:17

Fixed ScrollView's ScrollToAsync doesn't complete when called thrice …

bc0f3ec

…with the same value Fixed ScrollView's ScrollToAsync doesn't complete when called thrice with the same value.

Modified the fix

150f7b4

ScrollView's ScrollToAsync doesn't complete when called thrice with the same value

Test case enabled for this issue

18ee050

Test case enabled for this issue

KarthikRajaKalaimani force-pushed the fix-27250 branch from ce1d567 to 18ee050 Compare February 27, 2026 05:47

Modified the fix

4ce7857

dotnet deleted a comment from PureWeen Mar 2, 2026

kubaflo added s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates labels Mar 3, 2026

Conversation

KarthikRajaKalaimani commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue Details:

Root Cause:

Description of Change:

Reference:

Issues Fixed:

Screenshots

Uh oh!

KarthikRajaKalaimani commented Jan 23, 2025

Uh oh!

jsuarezruiz commented Jan 24, 2025

Uh oh!

azure-pipelines bot commented Jan 24, 2025

Uh oh!

jsuarezruiz commented Jan 28, 2025

Uh oh!

azure-pipelines bot commented Jan 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

EmilienDup commented Jun 19, 2025

Uh oh!

jsuarezruiz commented Oct 31, 2025

Uh oh!

jsuarezruiz commented Oct 31, 2025

Uh oh!

azure-pipelines bot commented Oct 31, 2025

Uh oh!

PureWeen commented Jan 30, 2026

Uh oh!

jfversluis left a comment

Choose a reason for hiding this comment

Code Review — PR #27300

🔴 Windows: Race condition with async ChangeView

🔴 iOS: SetContentOffset to same position has undefined animation callback behavior

🟡 Minor: Floating-point equality

🟡 Minor: Indentation

Summary

Uh oh!

github-actions bot commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KarthikRajaKalaimani commented Feb 27, 2026

Code Review — PR #27300

🔴 Windows: Race condition with async ChangeView

🔴 iOS: SetContentOffset to same position has undefined animation callback behavior

🟡 Minor: Floating-point equality

🟡 Minor: Indentation

Summary

Uh oh!

kubaflo commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 AI Summary

Issue Summary

Key Findings

Reviewer Discussion

Files Changed

Fix Candidates

Fix Candidates

Key Findings

Cross-Pollination Summary

⚠️ Final Recommendation: REQUEST CHANGES

Summary

Root Cause

Fix Quality

Code Review Findings

Title Suggestion

Description Update Needed

Phase 2: Code Review

✨ Suggested PR Description

Root Cause

Description of Change

Platforms Affected

Issues Fixed

Platforms Tested

Code Review: PR #27300

Windows: Missing value clamping before position comparison

KarthikRajaKalaimani commented Jan 23, 2025 •

edited

Loading

🔴 Windows: Race condition with async `ChangeView`

🔴 iOS: `SetContentOffset` to same position has undefined animation callback behavior

github-actions bot commented Feb 27, 2026 •

edited

Loading

🔴 Windows: Race condition with async `ChangeView`

🔴 iOS: `SetContentOffset` to same position has undefined animation callback behavior

kubaflo commented Mar 3, 2026 •

edited

Loading

Formatting: Missing space after `if` keyword (Windows)