fix(ssr-overlay): make the masked page interactive + skip overlay under WebDriver (close #725)

Addresses an expert review of the DOM-settle change: DOM-settle alone shortened but did not
eliminate issue #725, because the masking window still left the page non-interactive (the opaque
overlay sat over a visibility:hidden <ds-app>, so clicks landed on nothing) and still produced
duplicate DOM that breaks strict-mode E2E locators.

Changes:
- index.html: drop the `ds-app[data-dspace-ssr-hidden]{visibility:hidden}` rule and give the overlay
  min-height:100vh. The overlay is already opaque + pointer-events:none, so it still masks the CSR
  rebuild visually, but clicks now pass THROUGH it to the real, still-visible <ds-app> underneath.
  The page is therefore interactive the entire time it is masked -> #725 ("looks rendered but
  nothing is clickable") cannot recur, even if removal rides the 10s cap. The snapshot is marked
  aria-hidden so AT (and the duplicate-node concern) target the live app, not the snapshot.
- index.html: also bypass the overlay for WebDriver runners (navigator.webdriver), mirroring the
  existing Cypress guard, so Playwright/Selenium see no overlay and no duplicate DOM (fixes the
  #725 strict-mode locator failure).
- app.component.ts: exclude the admin-sidebar subtree from the DOM-settle MutationObserver (its long
  :enter/:leave animations would keep re-arming the quiet window and push admin logins toward the
  cap); add OnDestroy + takeUntil + observer/timer teardown.
- Fix stale `ApplicationRef.isStable` / `removeSsrOverlayWhenStable` comments (index.html, typings.d.ts).

Verified against the live build (Playwright, CPU/network throttled):
- WebDriver run: overlay absent (no duplicate DOM).
- webdriver spoofed false (real-user path): a navbar click WHILE the snapshot is still shown reaches
  the live <ds-app> (interactive under mask).
- anon reveal settled, CLS after reveal = 0; admin reveal settled (reason "settled", not cap),
  CLS after reveal = 0.

Refs: dspace-customers#725, PR #1288, PR #1317, PR #1318, PR #1321

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: milanmajchrak

src/app/app.component.ts

@@ -1,4 +1,4 @@
-import { distinctUntilChanged, filter, first, take, withLatestFrom, delay } from 'rxjs/operators';
+import { distinctUntilChanged, filter, first, take, takeUntil, withLatestFrom, delay } from 'rxjs/operators';
 import { DOCUMENT, isPlatformBrowser } from '@angular/common';
 import {
   AfterViewInit,
@@ -7,6 +7,7 @@ import {
   HostListener,
   Inject,
   NgZone,
+  OnDestroy,
   OnInit,
   PLATFORM_ID,
 } from '@angular/core';
@@ -17,7 +18,7 @@ import {
   Router,
 } from '@angular/router';
 
-import { BehaviorSubject, combineLatest, Observable } from 'rxjs';
+import { BehaviorSubject, combineLatest, Observable, Subject } from 'rxjs';
 import { select, Store } from '@ngrx/store';
 import { NgbModal, NgbModalConfig } from '@ng-bootstrap/ng-bootstrap';
 import { TranslateService } from '@ngx-translate/core';
@@ -39,10 +40,20 @@ import { distinctNext } from './core/shared/distinct-next';
   styleUrls: ['./app.component.scss'],
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class AppComponent implements OnInit, AfterViewInit {
+export class AppComponent implements OnInit, AfterViewInit, OnDestroy {
   notificationOptions;
   models;
 
+  /**
+   * Emits on destroy to tear down the overlay-removal gate subscription / DOM-settle watcher.
+   * AppComponent is the root component (lives for the app's lifetime) so this is mostly defensive
+   * + test hygiene, but it guarantees no MutationObserver/timer outlives the component.
+   */
+  private destroyed$ = new Subject<void>();
+
+  /** Set while the DOM-settle watcher is pending; disconnects the observer + clears its timers. */
+  private cancelOverlaySettle: (() => void) | null = null;
+
   /**
    * Whether or not the authentication is currently blocking the UI
    */
@@ -127,6 +138,7 @@ export class AppComponent implements OnInit, AfterViewInit {
         this.themeService.isThemeLoading$,
       ]).pipe(
         filter(([blocking, themeLoading]: [boolean, boolean]) => !blocking && !themeLoading),
+        takeUntil(this.destroyed$),
         first(),
       ).subscribe(() => {
         this.removeSsrOverlayWhenDomSettles(w);
@@ -177,6 +189,7 @@ export class AppComponent implements OnInit, AfterViewInit {
         return;
       }
       done = true;
+      this.cancelOverlaySettle = null;
       if (quietTimer !== null) { clearTimeout(quietTimer); }
       if (capTimer !== null) { clearTimeout(capTimer); }
       try { observer.disconnect(); } catch (e) { /* noop */ }
@@ -203,9 +216,22 @@ export class AppComponent implements OnInit, AfterViewInit {
       }, SETTLE_QUIET_MS);
     };
 
+    // The admin sidebar (left chrome) runs long :enter/:leave height animations that add/remove DOM
+    // well after the routed page has rendered; counting those would keep re-arming the quiet window
+    // (and can push admin logins toward the cap). The sidebar is not part of the above-the-fold
+    // content whose late pop-in users perceive as flicker, so exclude its subtree from the signal.
+    const inAdminSidebar = (target: Node | null): boolean => {
+      const el: Element | null = target && target.nodeType === 1
+        ? (target as Element)
+        : (target ? target.parentElement : null);
+      return !!(el && typeof el.closest === 'function' && el.closest('ds-themed-admin-sidebar, ds-admin-sidebar'));
+    };
     const observer = new MutationObserver((mutations: MutationRecord[]) => {
       for (const m of mutations) {
         if (m.type === 'childList' && (m.addedNodes.length > 0 || m.removedNodes.length > 0)) {
+          if (inAdminSidebar(m.target)) {
+            continue;
+          }
           let elementChanged = false;
           m.addedNodes.forEach((n: Node) => { if (n.nodeType === 1) { elementChanged = true; } });
           m.removedNodes.forEach((n: Node) => { if (n.nodeType === 1) { elementChanged = true; } });
@@ -221,9 +247,24 @@ export class AppComponent implements OnInit, AfterViewInit {
       observer.observe(app, { childList: true, subtree: true });
     }
     capTimer = setTimeout(finish, SETTLE_MAX_MS);
+    // expose teardown so ngOnDestroy can cancel a still-pending settle (and for test hygiene)
+    this.cancelOverlaySettle = () => {
+      if (quietTimer !== null) { clearTimeout(quietTimer); }
+      if (capTimer !== null) { clearTimeout(capTimer); }
+      try { observer.disconnect(); } catch (e) { /* noop */ }
+    };
     armQuietTimer();
   }
 
+  ngOnDestroy(): void {
+    this.destroyed$.next();
+    this.destroyed$.complete();
+    if (this.cancelOverlaySettle) {
+      this.cancelOverlaySettle();
+      this.cancelOverlaySettle = null;
+    }
+  }
+
   ngOnInit() {
     /** Implement behavior for interface {@link ModalBeforeDismiss} */
     this.modalConfig.beforeDismiss = async function () {

src/index.html

@@ -9,23 +9,31 @@
   <meta http-equiv="cache-control" content="no-store">
   <style id="__dspace-ssr-overlay-style">
     /* Overlay used to mask the Angular 15 bootstrap re-render of the SSR DOM.
-       See src/index.html bootstrap script + AppComponent.removeSsrOverlayWhenStable.
-       The overlay element holds the SSR-rendered children moved out of <ds-app>,
-       so it keeps every original Angular view-encapsulation attribute (_ngcontent-scXXX),
-       inline style, and lifecycle context — i.e. it looks pixel-identical to what SSR sent. */
+       See src/index.html bootstrap script + AppComponent.removeSsrOverlayWhenContentVisible
+       (-> removeSsrOverlayWhenDomSettles). The overlay element holds the SSR-rendered children
+       moved out of <ds-app>, so it keeps every original Angular view-encapsulation attribute
+       (_ngcontent-scXXX), inline style, and lifecycle context — i.e. it looks pixel-identical to
+       what SSR sent.
+
+       Interactivity: the overlay is opaque (background:#fff) and on top (z-index), but
+       pointer-events:none, AND the real <ds-app> underneath is NO LONGER visibility:hidden. So the
+       overlay only MASKS the CSR rebuild visually — clicks pass straight through it to the live,
+       already-built app underneath. That is what keeps the page interactive while masked and stops
+       the masking window from reproducing issue #725 ("looks rendered but nothing is clickable").
+       min-height:100vh guarantees the opaque mask always covers the viewport so the rebuilding app
+       cannot peek through (the live <ds-app> only ever grows up to the snapshot's height while it
+       rebuilds, so it stays behind the overlay until removal). */
     #__dspace_ssr_overlay {
       position: absolute;
       top: 0;
       left: 0;
       right: 0;
       width: 100%;
+      min-height: 100vh;
       z-index: 10000;
       background: #fff;
       pointer-events: none;
     }
-    /* Keep <ds-app> taking layout space (height of CSR result) but hidden, so the page
-       does not collapse the moment we drop the overlay. */
-    ds-app[data-dspace-ssr-hidden] { visibility: hidden; }
   </style>
 </head>
 <body>
@@ -40,18 +48,21 @@
   available before Angular 16, so on every browser load Angular tears down the entire SSR DOM and
   re-renders the component tree from scratch. The rebuild takes ~600-1500 ms on slow connections,
   during which the user sees the SSR view -> blank/half-built CSR view -> final CSR view.
-  This script captures the SSR DOM as a non-interactive snapshot the moment it's parsed (before
-  any module/main script runs - those are type=module and therefore deferred). While Angular
-  rebuilds the real <ds-app> invisibly, the snapshot keeps the page looking stable. AppComponent
-  removes the overlay once ApplicationRef.isStable settles.
+  This script captures the SSR DOM as a snapshot the moment it's parsed (before any module/main
+  script runs - those are type=module and therefore deferred). While Angular rebuilds the real
+  <ds-app> underneath (visually covered by the opaque snapshot, but still interactive), the snapshot
+  keeps the page looking stable. AppComponent removes the overlay once the routed CSR page has
+  finished rendering (its DOM has settled) - see AppComponent.removeSsrOverlayWhenDomSettles.
 */
 (function () {
   if (typeof window === 'undefined' || typeof document === 'undefined') return;
-  // Skip when Cypress is driving the page. The overlay duplicates SSR DOM (moved into the
-  // overlay) alongside the CSR DOM (rendered into <ds-app>) during the masking window — so
-  // any cy.get('#some-id').click() picks up two elements and fails. The overlay is a pure
-  // UX nicety, and Cypress E2E doesn't measure visual smoothness anyway; bail early.
+  // Skip when an E2E runner is driving the page. The overlay duplicates SSR DOM (moved into the
+  // overlay) alongside the CSR DOM (rendered into <ds-app>) during the masking window — so a
+  // strict-mode locator like cy.get('#x')/page.locator('#x') picks up two elements and fails. The
+  // overlay is a pure UX nicety and E2E doesn't measure visual smoothness, so bail early for both
+  // Cypress (window.Cypress) and any WebDriver-based runner (Playwright/Selenium: navigator.webdriver).
   if (typeof window.Cypress !== 'undefined') return;
+  if (typeof navigator !== 'undefined' && navigator.webdriver) return;
   try {
     var app = document.querySelector('ds-app');
     // If SSR was skipped for this route (excludePathPatterns), there are no children; nothing to mask.
@@ -78,25 +89,27 @@
     // so the overlay is pixel-identical to what the user already saw before Angular booted.
     // Cloning via innerHTML loses parent-context-dependent rendering.
     //
-    // Accessibility note: we deliberately do NOT set aria-hidden on the overlay. The overlay
-    // *is* the visible page during the masking window, so assistive technologies should read
-    // it. The original <ds-app> underneath gets visibility:hidden (via attribute + CSS rule),
-    // which removes both itself and its children from the accessibility tree.
+    // Accessibility: the snapshot is now a purely VISUAL mask, so we mark it aria-hidden. The real
+    // <ds-app> underneath is no longer visibility:hidden — it stays in the accessibility tree and
+    // is the interactive surface — so assistive tech (and mouse clicks) target the live, functional
+    // app rather than a soon-to-be-removed duplicate snapshot. This also avoids the duplicate
+    // a11y nodes the old (overlay-as-a11y-surface) approach produced during masking.
     var overlay = document.createElement('div');
     overlay.id = '__dspace_ssr_overlay';
+    overlay.setAttribute('aria-hidden', 'true');
     while (app.firstChild) {
       overlay.appendChild(app.firstChild);
     }
 
-    // Hide the now-empty <ds-app> so Angular can rebuild into it invisibly. We use an attribute
-    // (CSS in <head> targets it) rather than setting .style.visibility directly so Angular's
-    // template doesn't blow it away on first ChangeDetection.
+    // Mark <ds-app> as being masked. NOTE: this attribute is now only a state hook — it no longer
+    // hides the element (the visibility:hidden CSS rule was removed) so the live app stays visible
+    // (covered by the opaque overlay) and, crucially, INTERACTIVE while Angular rebuilds into it.
     app.setAttribute('data-dspace-ssr-hidden', '');
     document.body.appendChild(overlay);
 
     var removing = false;
     window.__dspaceRemoveSsrOverlay = function () {
-      // Re-entrancy guard: null the pointer up-front so a racing isStable + 15s safety
+      // Re-entrancy guard: null the pointer up-front so a racing DOM-settle removal + 15s safety
       // fallback cannot start two interleaving fade-out passes (which would re-remove
       // the kept styles from underneath the first pass).
       if (removing) return;

src/typings.d.ts

@@ -89,8 +89,9 @@ declare module '*.scss' {
 
 /**
  * Window global injected by the inline anti-flicker bootstrap script in `src/index.html`.
- * Called once by `AppComponent.removeSsrOverlayWhenStable()` when `ApplicationRef.isStable`
- * fires, to drop the SSR-mask overlay and let the freshly built CSR DOM become visible.
+ * Called once by `AppComponent.removeSsrOverlayWhenContentVisible()` (via
+ * `removeSsrOverlayWhenDomSettles()`) once the routed CSR page's DOM has settled, to drop the
+ * SSR-mask overlay and let the freshly built CSR DOM become visible.
  */
 interface Window {
   __dspaceRemoveSsrOverlay?: (() => void) | null;