Enhanced navigation should not scroll before content renders

src/Components/Web.JS/src/Boot.Web.ts

@@ -15,6 +15,7 @@ import { shouldAutoStart } from './BootCommon';
 import { Blazor } from './GlobalExports';
 import { WebStartOptions } from './Platform/WebStartOptions';
 import { attachStreamingRenderingListener } from './Rendering/StreamingRendering';
+import { resetScrollIfNeeded, ScrollResetSchedule } from './Rendering/Renderer';
 import { NavigationEnhancementCallbacks, attachProgressivelyEnhancedNavigationListener } from './Services/NavigationEnhancement';
 import { WebRootComponentManager } from './Services/WebRootComponentManager';
 import { hasProgrammaticEnhancedNavigationHandler, performProgrammaticEnhancedNavigation } from './Services/NavigationUtils';
@@ -57,6 +58,7 @@ function boot(options?: Partial<WebStartOptions>) : Promise<void> {
     },
     documentUpdated: () => {
       rootComponentManager.onDocumentUpdated();
+      resetScrollIfNeeded(ScrollResetSchedule.AfterDocumentUpdate);
       jsEventRegistry.dispatchEvent('enhancedload', {});
     },
     enhancedNavigationCompleted() {

src/Components/Web.JS/src/Rendering/Renderer.ts

@@ -11,8 +11,15 @@ import { getAndRemovePendingRootComponentContainer } from './JSRootComponents';
 interface BrowserRendererRegistry {
   [browserRendererId: number]: BrowserRenderer;
 }
+
+export enum ScrollResetSchedule {
+  None,
+  AfterBatch,
+  AfterDocumentUpdate,
+}
+
 const browserRenderers: BrowserRendererRegistry = {};
-let shouldResetScrollAfterNextBatch = false;
+let pendingScrollResetTiming: ScrollResetSchedule = ScrollResetSchedule.None;
 
 export function attachRootComponentToLogicalElement(browserRendererId: number, logicalElement: LogicalElement, componentId: number, appendContent: boolean): void {
   let browserRenderer = browserRenderers[browserRendererId];
@@ -88,19 +95,28 @@ export function renderBatch(browserRendererId: number, batch: RenderBatch): void
     browserRenderer.disposeEventHandler(eventHandlerId);
   }
 
-  resetScrollIfNeeded();
+  resetScrollIfNeeded(ScrollResetSchedule.AfterBatch);
 }
 
-export function resetScrollAfterNextBatch(): void {
-  shouldResetScrollAfterNextBatch = true;
-}
+export function scheduleScrollReset(timing: ScrollResetSchedule): void {
+  if (timing != ScrollResetSchedule.AfterBatch) {
+      pendingScrollResetTiming = timing;
+      return;
+  }
 
-export function resetScrollIfNeeded() {
-  if (shouldResetScrollAfterNextBatch) {
-    shouldResetScrollAfterNextBatch = false;
+  if (pendingScrollResetTiming !== ScrollResetSchedule.AfterDocumentUpdate) {
+    pendingScrollResetTiming = ScrollResetSchedule.AfterBatch;
+  }
+}
 
-    // This assumes the scroller is on the window itself. There isn't a general way to know
-    // if some other element is playing the role of the primary scroll region.
-    window.scrollTo && window.scrollTo(0, 0);
+export function resetScrollIfNeeded(triggerTiming: ScrollResetSchedule) {
+  if (pendingScrollResetTiming !== triggerTiming) {
+    return;
   }
+
+  pendingScrollResetTiming = ScrollResetSchedule.None;
+
+  // This assumes the scroller is on the window itself. There isn't a general way to know
+  // if some other element is playing the role of the primary scroll region.
+  window.scrollTo && window.scrollTo(0, 0);
 }

src/Components/Web.JS/src/Services/NavigationEnhancement.ts

@@ -3,7 +3,7 @@
 
 import { synchronizeDomContent } from '../Rendering/DomMerging/DomSync';
 import { attachProgrammaticEnhancedNavigationHandler, handleClickForNavigationInterception, hasInteractiveRouter, isForSamePath, notifyEnhancedNavigationListeners, performScrollToElementOnTheSamePage, isSamePageWithHash } from './NavigationUtils';
-import { resetScrollAfterNextBatch, resetScrollIfNeeded } from '../Rendering/Renderer';
+import { scheduleScrollReset, ScrollResetSchedule } from '../Rendering/Renderer';
 
 /*
 In effect, we have two separate client-side navigation mechanisms:
@@ -81,7 +81,7 @@ function performProgrammaticEnhancedNavigation(absoluteInternalHref: string, rep
   }
 
   if (!isForSamePath(absoluteInternalHref, originalLocation)) {
-    resetScrollAfterNextBatch();
+    scheduleScrollReset(ScrollResetSchedule.AfterDocumentUpdate);
   }
 
   performEnhancedPageLoad(absoluteInternalHref, /* interceptedLink */ false);
@@ -108,8 +108,7 @@ function onDocumentClick(event: MouseEvent) {
       let isSelfNavigation = isForSamePath(absoluteInternalHref, originalLocation);
       performEnhancedPageLoad(absoluteInternalHref, /* interceptedLink */ true);
       if (!isSelfNavigation) {
-        resetScrollAfterNextBatch();
-        resetScrollIfNeeded();
+        scheduleScrollReset(ScrollResetSchedule.AfterDocumentUpdate);
       }
     }
   });

src/Components/Web.JS/src/Services/NavigationManager.ts

@@ -2,7 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 import '@microsoft/dotnet-js-interop';
-import { resetScrollAfterNextBatch } from '../Rendering/Renderer';
+import { scheduleScrollReset, ScrollResetSchedule } from '../Rendering/Renderer';
 import { EventDelegator } from '../Rendering/Events/EventDelegator';
 import { attachEnhancedNavigationListener, getInteractiveRouterRendererId, handleClickForNavigationInterception, hasInteractiveRouter, hasProgrammaticEnhancedNavigationHandler, isForSamePath, isSamePageWithHash, isWithinBaseUriSpace, performProgrammaticEnhancedNavigation, performScrollToElementOnTheSamePage, scrollToElement, setHasInteractiveRouter, toAbsoluteUri } from './NavigationUtils';
 import { WebRendererId } from '../Rendering/WebRendererId';
@@ -170,7 +170,7 @@ async function performInternalNavigation(absoluteInternalHref: string, intercept
   // To avoid ugly flickering effects, we don't want to change the scroll position until
   // we render the new page. As a best approximation, wait until the next batch.
   if (!isForSamePath(absoluteInternalHref, location.href)) {
-    resetScrollAfterNextBatch();
+    scheduleScrollReset(ScrollResetSchedule.AfterBatch);
   }
 
   saveToBrowserHistory(absoluteInternalHref, replace, state);

src/Components/test/E2ETest/Infrastructure/ScrollOverrideScope.cs

@@ -0,0 +1,181 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Linq;
+using OpenQA.Selenium;
+using Xunit.Sdk;
+
+namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
+
+internal sealed class ScrollOverrideScope : IDisposable
+{
+    private readonly IJavaScriptExecutor _executor;
+    private readonly bool _isActive;
+
+    public ScrollOverrideScope(IWebDriver browser, bool isActive)
+    {
+        _executor = (IJavaScriptExecutor)browser;
+        _isActive = isActive;
+
+        if (!_isActive)
+        {
+            return;
+        }
+
+        _executor.ExecuteScript(@"
+(function() {
+    if (window.__enhancedNavScrollOverride) {
+        if (window.__clearEnhancedNavScrollLog) {
+            window.__clearEnhancedNavScrollLog();
+        }
+        return;
+    }
+
+    const original = window.scrollTo.bind(window);
+    const log = [];
+
+    function resolvePage() {
+        const landing = document.getElementById('test-info-1');
+        if (landing && landing.textContent === 'Scroll tests landing page') {
+            return 'landing';
+        }
+
+        const next = document.getElementById('test-info-2');
+        if (next && next.textContent === 'Scroll tests next page') {
+            return 'next';
+        }
+
+        return 'other';
+    }
+
+    window.__enhancedNavScrollOverride = true;
+    window.__enhancedNavOriginalScrollTo = original;
+    window.__enhancedNavScrollLog = log;
+    window.__clearEnhancedNavScrollLog = () => { log.length = 0; };
+    window.__drainEnhancedNavScrollLog = () => {
+        const copy = log.slice();
+        log.length = 0;
+        return copy;
+    };
+
+    window.scrollTo = function(...args) {
+        log.push({
+            page: resolvePage(),
+            url: location.href,
+            time: performance.now(),
+            args
+        });
+
+        return original(...args);
+    };
+})();
+");
+
+        ClearLog();
+    }
+
+    public void ClearLog()
+    {
+        if (!_isActive)
+        {
+            return;
+        }
+
+        _executor.ExecuteScript("if (window.__clearEnhancedNavScrollLog) { window.__clearEnhancedNavScrollLog(); }");
+    }
+
+    public void AssertNoPrematureScroll(string expectedPage, string navigationDescription)
+    {
+        if (!_isActive)
+        {
+            return;
+        }
+
+        var entries = DrainLog();
+        if (entries.Length == 0)
+        {
+            return;
+        }
+
+        var unexpectedEntries = entries
+            .Where(entry => !string.Equals(entry.Page, expectedPage, StringComparison.Ordinal))
+            .ToArray();
+
+        if (unexpectedEntries.Length == 0)
+        {
+            return;
+        }
+
+        var details = string.Join(
+            ", ",
+            unexpectedEntries.Select(entry => $"page={entry.Page ?? "null"} url={entry.Url} time={entry.Time:F2}"));
+
+        throw new XunitException($"Detected a scroll reset while the DOM still displayed '{unexpectedEntries[0].Page ?? "unknown"}' during {navigationDescription}. Entries: {details}");
+    }
+
+    private ScrollInvocation[] DrainLog()
+    {
+        if (!_isActive)
+        {
+            return Array.Empty<ScrollInvocation>();
+        }
+
+        var result = _executor.ExecuteScript("return window.__drainEnhancedNavScrollLog ? window.__drainEnhancedNavScrollLog() : [];");
+        if (result is not IReadOnlyList<object> entries || entries.Count == 0)
+        {
+            return Array.Empty<ScrollInvocation>();
+        }
+
+        var resolved = new ScrollInvocation[entries.Count];
+        for (var i = 0; i < entries.Count; i++)
+        {
+            if (entries[i] is IReadOnlyDictionary<string, object> dict)
+            {
+                dict.TryGetValue("page", out var pageValue);
+                dict.TryGetValue("url", out var urlValue);
+                dict.TryGetValue("time", out var timeValue);
+
+                resolved[i] = new ScrollInvocation(
+                    pageValue as string,
+                    urlValue as string,
+                    timeValue is null ? 0D : Convert.ToDouble(timeValue, CultureInfo.InvariantCulture));
+                continue;
+            }
+
+            resolved[i] = new ScrollInvocation(null, null, 0D);
+        }
+
+        return resolved;
+    }
+
+    public void Dispose()
+    {
+        if (!_isActive)
+        {
+            return;
+        }
+
+        _executor.ExecuteScript(@"
+(function() {
+    if (!window.__enhancedNavScrollOverride) {
+        return;
+    }
+
+    if (window.__enhancedNavOriginalScrollTo) {
+        window.scrollTo = window.__enhancedNavOriginalScrollTo;
+        delete window.__enhancedNavOriginalScrollTo;
+    }
+
+    delete window.__enhancedNavScrollOverride;
+    delete window.__enhancedNavScrollLog;
+    delete window.__clearEnhancedNavScrollLog;
+    delete window.__drainEnhancedNavScrollLog;
+})();
+");
+    }
+
+    private readonly record struct ScrollInvocation(string Page, string Url, double Time);
+}

src/Components/test/E2ETest/Infrastructure/WebDriverExtensions/WebDriverExtensions.cs

@@ -1,8 +1,11 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
+using System.Threading;
 using OpenQA.Selenium;
 using OpenQA.Selenium.Support.UI;
+using Microsoft.AspNetCore.Components.E2ETests.ServerRenderingTests;
 
 namespace Microsoft.AspNetCore.Components.E2ETest;
 
@@ -64,4 +67,78 @@ public static long GetElementPositionWithRetry(this IWebDriver browser, string e
 
         throw new Exception($"Failed to get position for element '{elementId}' after {retryCount} retries. Debug log: {log}");
     }
+
+    internal static ScrollObservation BeginScrollObservation(this IWebDriver browser, IWebElement element, Func<IWebDriver, bool> domMutationPredicate)
+    {
+        ArgumentNullException.ThrowIfNull(browser);
+        ArgumentNullException.ThrowIfNull(element);
+        ArgumentNullException.ThrowIfNull(domMutationPredicate);
+
+        var initialScrollPosition = browser.GetScrollY();
+        return new ScrollObservation(element, initialScrollPosition, domMutationPredicate);
+    }
+
+    internal static ScrollObservationResult WaitForStaleDomOrScrollChange(this IWebDriver browser, ScrollObservation observation, TimeSpan? timeout = null, TimeSpan? pollingInterval = null)
+    {
+        ArgumentNullException.ThrowIfNull(browser);
+
+        var wait = new DefaultWait<IWebDriver>(browser)
+        {
+            Timeout = timeout ?? TimeSpan.FromSeconds(10),
+            PollingInterval = pollingInterval ?? TimeSpan.FromMilliseconds(50),
+        };
+        wait.IgnoreExceptionTypes(typeof(InvalidOperationException));
+
+        ScrollObservationOutcome? detectedOutcome = null;
+        wait.Until(driver =>
+        {
+            if (observation.DomMutationPredicate(driver))
+            {
+                detectedOutcome = ScrollObservationOutcome.DomUpdated;
+                return true;
+            }
+
+            if (observation.Element.IsStale())
+            {
+                detectedOutcome = ScrollObservationOutcome.DomUpdated;
+                return true;
+            }
+
+            if (browser.GetScrollY() != observation.InitialScrollPosition)
+            {
+                detectedOutcome = ScrollObservationOutcome.ScrollChanged;
+                return true;
+            }
+
+            return false;
+        });
+
+        var outcome = detectedOutcome ?? ScrollObservationOutcome.DomUpdated;
+
+        var finalScrollPosition = browser.GetScrollY();
+        return new ScrollObservationResult(outcome, observation.InitialScrollPosition, finalScrollPosition);
+    }
+
+    internal static bool IsStale(this IWebElement element)
+    {
+        try
+        {
+            _ = element.Enabled;
+            return false;
+        }
+        catch (StaleElementReferenceException)
+        {
+            return true;
+        }
+    }
+}
+
+internal readonly record struct ScrollObservation(IWebElement Element, long InitialScrollPosition, Func<IWebDriver, bool> DomMutationPredicate);
+
+internal readonly record struct ScrollObservationResult(ScrollObservationOutcome Outcome, long InitialScrollPosition, long FinalScrollPosition);
+
+internal enum ScrollObservationOutcome
+{
+    ScrollChanged,
+    DomUpdated,
 }