From 8527b06f4a9913c9d4f4177478bd2051316542e0 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Thu, 23 Oct 2025 14:10:48 -0400 Subject: [PATCH] Stabilize video modal zap popover behavior --- js/app.js | 30 ++++- js/ui/components/VideoModal.js | 29 +++- js/ui/components/tagPillList.js | 2 +- js/ui/overlay/popoverEngine.js | 6 +- tests/ui/tag-pill-list.test.mjs | 2 +- tests/video-modal-accessibility.test.mjs | 162 +++++++++++++++++++++-- 6 files changed, 215 insertions(+), 16 deletions(-) diff --git a/js/app.js b/js/app.js index b8a6938b..e8b6c892 100644 --- a/js/app.js +++ b/js/app.js @@ -7420,11 +7420,35 @@ class Application { magnet: trimmedMagnet, }); - if ( + const modalVideoIsConnected = (() => { + if (!this.modalVideo) { + return false; + } + if (typeof this.modalVideo.isConnected === "boolean") { + return this.modalVideo.isConnected; + } + const ownerDocument = this.modalVideo.ownerDocument || + (typeof document !== "undefined" ? document : null); + if (ownerDocument?.contains) { + try { + return ownerDocument.contains(this.modalVideo); + } catch (error) { + devLogger.warn( + "[playVideoWithFallback] Failed to determine modal video connection state", + error, + ); + } + } + return true; + })(); + + const shouldReuseActiveSession = + modalVideoIsConnected && this.activePlaybackSession && typeof this.activePlaybackSession.matchesRequestSignature === "function" && - this.activePlaybackSession.matchesRequestSignature(requestSignature) - ) { + this.activePlaybackSession.matchesRequestSignature(requestSignature); + + if (shouldReuseActiveSession) { this.log( "[playVideoWithFallback] Duplicate playback request detected; reusing active session." ); diff --git a/js/ui/components/VideoModal.js b/js/ui/components/VideoModal.js index ace8cdd7..11d93ea3 100644 --- a/js/ui/components/VideoModal.js +++ b/js/ui/components/VideoModal.js @@ -601,6 +601,7 @@ export class VideoModal { if (this.modalZapOpenPromise) { this.modalZapPendingToggle = "close"; + this.closeZapDialog({ silent: true, restoreFocus: false }); return; } @@ -774,7 +775,10 @@ export class VideoModal { const { silent = false, ...rest } = options; const wasOpen = popover.isOpen?.() === true; const result = originalClose(rest); - if (wasOpen && result) { + const wasDialogMarkedOpen = + this.modalZapDialogOpen === true || + this.modalZapDialog?.dataset?.state === "open"; + if (wasOpen || wasDialogMarkedOpen) { this.modalZapPendingToggle = null; this.modalZapOpenPromise = null; this.modalZapDialog.dataset.state = "closed"; @@ -784,7 +788,7 @@ export class VideoModal { if (this.modalZapBtn) { this.modalZapBtn.setAttribute("aria-expanded", "false"); } - if (!silent) { + if (!silent && (wasOpen || wasDialogMarkedOpen)) { this.dispatch("zap:close", { video: this.activeVideo }); } } @@ -1610,7 +1614,21 @@ export class VideoModal { this.modalZapDialog.setAttribute("aria-hidden", "false"); } this.focusZapAmount(); + return true; } + + if (this.modalZapDialog) { + this.modalZapDialog.hidden = false; + this.modalZapDialog.dataset.state = "open"; + this.modalZapDialog.setAttribute("aria-hidden", "false"); + this.modalZapDialogOpen = true; + if (this.modalZapBtn) { + this.modalZapBtn.setAttribute("aria-expanded", "true"); + } + this.focusZapAmount(); + return true; + } + return opened; } @@ -1647,6 +1665,13 @@ export class VideoModal { } closeZapDialog({ silent = false, restoreFocus } = {}) { + const dialogAppearsOpen = this.isZapDialogOpen(); + + if (this.modalZapOpenPromise && !dialogAppearsOpen) { + this.modalZapPendingToggle = "close"; + return; + } + this.modalZapPendingToggle = null; if (this.modalZapPopover?.close) { const options = { silent }; diff --git a/js/ui/components/tagPillList.js b/js/ui/components/tagPillList.js index 7900e127..a17a130d 100644 --- a/js/ui/components/tagPillList.js +++ b/js/ui/components/tagPillList.js @@ -37,7 +37,7 @@ function createTagButton({ doc, tag, onTagActivate }) { const icon = doc.createElement("span"); icon.className = "video-tag-pill__icon"; icon.setAttribute("aria-hidden", "true"); - icon.textContent = "+"; + icon.dataset.icon = "plus"; button.append(label, icon); diff --git a/js/ui/overlay/popoverEngine.js b/js/ui/overlay/popoverEngine.js index d818c0e4..7eb50c65 100644 --- a/js/ui/overlay/popoverEngine.js +++ b/js/ui/overlay/popoverEngine.js @@ -266,7 +266,11 @@ export function createPopover(trigger, render, options = {}) { } const restoreScroll = () => { - view.scrollTo(scrollX, scrollY); + try { + view.scrollTo(scrollX, scrollY); + } catch (error) { + logger.dev?.warn?.("[popover] scroll restoration failed", error); + } }; restoreScroll(); diff --git a/tests/ui/tag-pill-list.test.mjs b/tests/ui/tag-pill-list.test.mjs index 9384db30..a192e207 100644 --- a/tests/ui/tag-pill-list.test.mjs +++ b/tests/ui/tag-pill-list.test.mjs @@ -32,7 +32,7 @@ test("renderTagPillStrip builds buttons with labels and icons", () => { const icon = button.querySelector(".video-tag-pill__icon"); assert(icon, "icon span should be present"); - assert.equal(icon.textContent, "+"); + assert.equal(icon.dataset.icon, "plus"); assert.equal(icon.getAttribute("aria-hidden"), "true"); } }); diff --git a/tests/video-modal-accessibility.test.mjs b/tests/video-modal-accessibility.test.mjs index a0f04e3e..74d96b15 100644 --- a/tests/video-modal-accessibility.test.mjs +++ b/tests/video-modal-accessibility.test.mjs @@ -37,10 +37,83 @@ export async function setupModal({ lazyLoad = false } = {}) { globalThis.KeyboardEvent = window.KeyboardEvent; globalThis.MouseEvent = window.MouseEvent; const hadWebSocket = typeof globalThis.WebSocket !== "undefined"; - if (typeof window.scrollTo !== "function") { - window.scrollTo = () => {}; + const noopScrollTo = () => {}; + const windowPrototype = Object.getPrototypeOf(window) || null; + const prototypeDescriptor = windowPrototype + ? Object.getOwnPropertyDescriptor(windowPrototype, "scrollTo") + : null; + const hadPrototypeScrollTo = !!prototypeDescriptor; + let restorePrototypeScrollTo = () => {}; + if (windowPrototype) { + try { + Object.defineProperty(windowPrototype, "scrollTo", { + configurable: true, + enumerable: false, + writable: true, + value: noopScrollTo, + }); + restorePrototypeScrollTo = () => { + if (hadPrototypeScrollTo) { + Object.defineProperty(windowPrototype, "scrollTo", prototypeDescriptor); + } else { + delete windowPrototype.scrollTo; + } + }; + } catch (error) { + restorePrototypeScrollTo = () => { + if (hadPrototypeScrollTo) { + try { + Object.defineProperty(windowPrototype, "scrollTo", prototypeDescriptor); + } catch (restoreError) { + windowPrototype.scrollTo = prototypeDescriptor?.value; + } + } else { + delete windowPrototype.scrollTo; + } + }; + try { + windowPrototype.scrollTo = noopScrollTo; + } catch (assignError) { + // ignore inability to override prototype; relying on own property override + } + } } - globalThis.scrollTo = window.scrollTo; + const hadOwnScrollTo = Object.prototype.hasOwnProperty.call(window, "scrollTo"); + const originalWindowScrollTo = window.scrollTo; + try { + Object.defineProperty(window, "scrollTo", { + configurable: true, + enumerable: false, + writable: true, + value: noopScrollTo, + }); + } catch (error) { + window.scrollTo = noopScrollTo; + } + const originalGlobalScrollTo = globalThis.scrollTo; + globalThis.scrollTo = noopScrollTo; + const restoreScrollTo = () => { + restorePrototypeScrollTo(); + if (hadOwnScrollTo) { + try { + Object.defineProperty(window, "scrollTo", { + configurable: true, + enumerable: false, + writable: true, + value: originalWindowScrollTo, + }); + } catch (error) { + window.scrollTo = originalWindowScrollTo; + } + } else { + delete window.scrollTo; + } + if (originalGlobalScrollTo !== undefined) { + globalThis.scrollTo = originalGlobalScrollTo; + } else { + delete globalThis.scrollTo; + } + }; if (!hadWebSocket) { globalThis.WebSocket = class { constructor() {} @@ -133,7 +206,7 @@ export async function setupModal({ lazyLoad = false } = {}) { delete globalThis.location; delete globalThis.KeyboardEvent; delete globalThis.MouseEvent; - delete globalThis.scrollTo; + restoreScrollTo(); if (!hadWebSocket) { delete globalThis.WebSocket; } @@ -321,10 +394,83 @@ async function setupPlaybackHarness() { globalThis.MouseEvent = window.MouseEvent; globalThis.self = window; const hadHarnessWebSocket = typeof globalThis.WebSocket !== "undefined"; - if (typeof window.scrollTo !== "function") { - window.scrollTo = () => {}; + const noopScrollTo = () => {}; + const windowPrototype = Object.getPrototypeOf(window) || null; + const prototypeDescriptor = windowPrototype + ? Object.getOwnPropertyDescriptor(windowPrototype, "scrollTo") + : null; + const hadPrototypeScrollTo = !!prototypeDescriptor; + let restorePrototypeScrollTo = () => {}; + if (windowPrototype) { + try { + Object.defineProperty(windowPrototype, "scrollTo", { + configurable: true, + enumerable: false, + writable: true, + value: noopScrollTo, + }); + restorePrototypeScrollTo = () => { + if (hadPrototypeScrollTo) { + Object.defineProperty(windowPrototype, "scrollTo", prototypeDescriptor); + } else { + delete windowPrototype.scrollTo; + } + }; + } catch (error) { + restorePrototypeScrollTo = () => { + if (hadPrototypeScrollTo) { + try { + Object.defineProperty(windowPrototype, "scrollTo", prototypeDescriptor); + } catch (restoreError) { + windowPrototype.scrollTo = prototypeDescriptor?.value; + } + } else { + delete windowPrototype.scrollTo; + } + }; + try { + windowPrototype.scrollTo = noopScrollTo; + } catch (assignError) { + // ignore inability to override prototype; relying on own property override + } + } } - globalThis.scrollTo = window.scrollTo; + const hadOwnScrollTo = Object.prototype.hasOwnProperty.call(window, "scrollTo"); + const originalWindowScrollTo = window.scrollTo; + try { + Object.defineProperty(window, "scrollTo", { + configurable: true, + enumerable: false, + writable: true, + value: noopScrollTo, + }); + } catch (error) { + window.scrollTo = noopScrollTo; + } + const originalGlobalScrollTo = globalThis.scrollTo; + globalThis.scrollTo = noopScrollTo; + const restoreScrollTo = () => { + restorePrototypeScrollTo(); + if (hadOwnScrollTo) { + try { + Object.defineProperty(window, "scrollTo", { + configurable: true, + enumerable: false, + writable: true, + value: originalWindowScrollTo, + }); + } catch (error) { + window.scrollTo = originalWindowScrollTo; + } + } else { + delete window.scrollTo; + } + if (originalGlobalScrollTo !== undefined) { + globalThis.scrollTo = originalGlobalScrollTo; + } else { + delete globalThis.scrollTo; + } + }; if (!hadHarnessWebSocket) { globalThis.WebSocket = class { constructor() {} @@ -415,7 +561,7 @@ async function setupPlaybackHarness() { delete globalThis.KeyboardEvent; delete globalThis.MouseEvent; delete globalThis.self; - delete globalThis.scrollTo; + restoreScrollTo(); if (!hadHarnessWebSocket) { delete globalThis.WebSocket; }