[MWPW-205194] - Perf Router Marquee: stop background autoplay on hidden viewports & sync slide across breakpoints by DKos95 · Pull Request #6545 · adobecom/milo

DKos95 · 2026-08-24T11:30:46Z

Background

The router marquee builds all three viewports (mobile/tablet/desktop) into the DOM at once, showing one at a time via CSS, with a separate autoplay controller per viewport created lazily on first view. Two related defects came out of that design.

Issue 1 — Autoplay leak on hidden viewports

Switching breakpoints (window resize, devtools open/close, orientation change) never stopped the previous viewport's controller. initViewportAutoplay only guarded against re-initializing the same viewport — it never paused the outgoing one. So each viewport a session ever touched kept its 5s autoplay timer and hero <video> running forever inside a display:none container, accumulating across breakpoint crossings.

Verified: resizing 1200 → 1400 → 500 left both the tablet and desktop videos playing (paused: false) while hidden. Directly counter to sr-perf-dd's perf goals (decoding/playing invisible video, re-running the slide-advance cycle indefinitely).

Issue 2 — Slide index not synced across breakpoints

Each viewport tracked its own independent slide index, so switching breakpoints landed on a different slide than the one that had been on screen (e.g. desktop auto-advanced to slide 2, resize to tablet showed slide 1). The viewports were never in sync.

Fixes

One running controller at a time (leak fix). Replaced the initializedVps Set + autoplayControllers array with a controllersByVp Map. syncViewportAutoplay pauses the outgoing viewport's controller on every breakpoint change and resumes (or lazily creates) the incoming one, so only the visible viewport ever runs a timer/video. The scroll-based IntersectionObserver now pauses/resumes only the active viewport's controller.
Carry the slide index across (sync fix). Controllers expose getActive() and syncTo(index); the handoff carries the outgoing viewport's live index into the incoming one via an instant, no-animation jump (a new instant flag reusing activate's reduced-motion path). Breakpoints now stay in lockstep.
Skip first-frame gating for later viewports. The hero-video first-frame autoplay gate only matters for initial-load LCP, so a gateOnFirstFrame param applies it only to the very first viewport; viewports spun up later on resize start immediately.

Resolves: MWPW-205194

Test URLs:

…hanges Switching viewports (mobile/tablet/desktop) never stopped the previous viewport's autoplay controller, so its timer and hero video kept running forever inside a display:none container. Track one controller per viewport and pause the outgoing one / resume (or lazily create) the incoming one instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Each viewport (mobile/tablet/desktop) tracked its own slide index, so switching breakpoints landed on a different slide than the one on screen (e.g. desktop on slide 2 -> tablet showed slide 1). Controllers now expose getActive()/syncTo(), and the viewport handoff carries the outgoing viewport's active slide into the incoming one via an instant (no-animation) jump. Viewports created later on resize also skip the hero first-frame gate, which only matters for initial-load LCP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

aem-code-sync · 2026-08-24T11:30:49Z

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

Re-sync branch

Commits

d0814e2 ✅ (latest)
47d0723 ✅
d498713 ✅
878d70c ✅
d829f26 ✅
5986cfd ✅

rgclayton · 2026-08-24T23:18:29Z

The auto play timer doesn't seem to wait for the marquee to load/initialize. By the time it loads it's advancing to the 2nd slide. Is this expected or possibly a regression?

DKos95 · 2026-08-25T06:27:54Z

The auto play timer doesn't seem to wait for the marquee to load/initialize. By the time it loads it's advancing to the 2nd slide. Is this expected or possibly a regression?

It's doing it with the site-redesign-foundation branch also so I would say this is already existing.

resume() lacked the prefersReducedMotion() check that beginAutoplay() already has, so switching breakpoints back to a previously-created viewport (e.g. mobile -> desktop -> mobile) could restart autoplay for reduced-motion users. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

rgclayton · 2026-08-26T00:42:19Z

+  const getActive = () => active;
+  // Jump straight to a slide with no transition, so a hidden viewport can be lined up
+  // with the one the user is leaving. clearFill resets the outgoing card's progress bar.
+  const syncTo = (index) => {


No bounds check on index here. If one breakpoint has fewer authored slide rows than another and the user resizes over from a higher index, this calls activate() with an out-of-range index — newSlide (line 504) comes back undefined and newSlide.querySelector('video') throws.

Good catch, thanks - clamped it: Math.min(index, slides.length - 1) before activating, so resizing into a viewport with fewer authored slides can't push it out of range anymore.

rgclayton · 2026-08-26T00:42:19Z


  const resume = () => {
-    if (!paused) return;
+    if (!paused || prefersReducedMotion()) return;


This makes the Play button permanently inert for reduced-motion users — it stays visible and clickable, but clicking it now silently does nothing. Intentional, or should manual resume still work under reduced motion (just don't auto-advance)?

Fixed - resume() now takes a manual flag. A manual Play-button click still plays the video and flips the button under reduced motion, it just returns right after (before scheduling the auto-advance timer/progress-bar fill) so it doesn't auto-advance. System-triggered resumes (breakpoint switch, scroll back into view) are unaffected and still fully respect reduced motion.

rgclayton · 2026-08-26T00:42:19Z

+      controllersByVp.set(activeVp, controller);
+    }
+    if (carryIndex != null) controller.syncTo(carryIndex);
+    controller.resume();


This runs unconditionally on every breakpoint change, including for a controller the user explicitly paused before resizing. Doesn't this override an explicit user pause once they resize back? Might need a separate "user paused" flag that this checks before calling resume().

Fixed with a userPaused flag, set by every user-driven pause path (Play/Pause button, hover, swipe, reset/next buttons, click-through) and only cleared by a manual resume. A system resume (breakpoint switch or the scroll IntersectionObserver) now checks it and backs off instead of overriding an explicit pause.

rgclayton · 2026-08-26T00:42:19Z

+  if (gateOnFirstFrame && heroVideo && typeof heroVideo.requestVideoFrameCallback === 'function' && !prefersReducedMotion()) {
    let started = false;
    let fallbackTimer = null;
    const kick = () => {


This and resume() (line 587) both schedule their own timer without clearing the other's. Scenario: a resize pauses this controller before its first-frame gate resolves, then a second resize brings it back (calling resume(), which sets a timer) — the still-pending kick() can later fire beginAutoplay() and schedule a second timer. The first one never gets cleared, so it fires on its own later too, causing an extra/early advance().

Fixed - added clearTimeout(timer) here (and in resume()) before scheduling a new one, so a kick() that resolves after a resize has already resumed the controller clears the timer resume() set instead of leaving it to fire on its own later.

rgclayton · 2026-08-26T00:42:19Z

-  initViewportAutoplay();
+  syncViewportAutoplay();
  requestAnimationFrame(() => dynamicLayoutUpdates(el));
  let resizeRaf;


Nit: libs/utils/action.js already exports debounce(), used the same way in pdf-space.js:1389 and menu.js:108 — could reuse that instead of hand-rolling the rAF throttle here.

Took the suggestion, with one adjustment: only dynamicLayoutUpdates() (pure layout measurement) is debounced. syncViewportAutoplay()/loadViewportVideos() stay un-debounced and run directly on every resize event - getActiveViewport() is just two cheap matchMedia checks, and debouncing the pause/resume sync itself would let the outgoing viewport's hidden video/timer keep running for the whole drag during a continuous resize, which is the exact leak this PR fixes.

DKos95 · 2026-08-26T07:56:18Z

@rgclayton Thanks a lot for the detailed review, will revisit performance PR's once the wave stuff is cleared.

github-actions · 2026-09-03T03:37:45Z

This PR has not been updated recently and will be closed in 7 days if no action is taken. Please ensure all checks are passing, https://github.com/orgs/adobecom/discussions/997 provides instructions. If the PR is ready to be merged, please mark it with the "Ready for Stage" label.

Clamp syncTo() to the incoming viewport's slide count, let a manual Play click still play video under reduced motion (without scheduling auto-advance), make a standing user pause survive breakpoint/scroll resume via a userPaused flag, clear the autoplay timer before rescheduling it to avoid a duplicate advance() firing, and split the resize handler so viewport pause/resume sync stays immediate while only the layout-measurement work is debounced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

DKos95 · 2026-09-03T15:57:37Z

@rgclayton finally got some time to update these performance tickets, thanks again for the suggestions!

…utoplay-leak-fix

Dusan Kosanovic and others added 2 commits August 24, 2026 11:33

DKos95 requested a review from a team August 24, 2026 11:30

DKos95 added the Site Redesign label Aug 24, 2026

SilviuLCF assigned NadiiaSokolova Aug 24, 2026

zagi25 reviewed Aug 25, 2026

View reviewed changes

Comment thread libs/c2/blocks/router-marquee/router-marquee.js

NadiiaSokolova requested a review from a team August 25, 2026 10:54

DKos95 requested a review from zagi25 August 25, 2026 11:54

rgclayton reviewed Aug 26, 2026

View reviewed changes

DKos95 added the on hold label Aug 26, 2026

github-actions Bot added the Stale label Sep 3, 2026

DKos95 removed the Stale label Sep 3, 2026

DKos95 requested a review from rgclayton September 3, 2026 15:57

DKos95 removed the on hold label Sep 4, 2026

Merge remote-tracking branch 'origin/sr-perf-dd' into sr-perf-dd-rm-a…

47d0723

…utoplay-leak-fix

aem-code-sync Bot deployed to sr-perf-dd-rm-autoplay-leak-fix September 4, 2026 11:24 View deployment

Merge remote-tracking branch 'origin/sr-perf-dd' into sr-perf-dd-rm-a…

d0814e2

…utoplay-leak-fix

aem-code-sync Bot deployed to sr-perf-dd-rm-autoplay-leak-fix September 4, 2026 11:35 View deployment

Uh oh!

Conversation

DKos95 commented Aug 24, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Issue 1 — Autoplay leak on hidden viewports

Issue 2 — Slide index not synced across breakpoints

Fixes

Uh oh!

aem-code-sync Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgclayton commented Aug 24, 2026

Uh oh!

DKos95 commented Aug 25, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DKos95 commented Aug 26, 2026

Uh oh!

github-actions Bot commented Sep 3, 2026

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DKos95 commented Aug 24, 2026 •

edited by github-actions Bot

Loading

aem-code-sync Bot commented Aug 24, 2026 •

edited

Loading

DKos95 commented Sep 3, 2026 •

edited

Loading