mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
modal: fix video player failing to open — missing-delegate TypeError on every open
Root cause of "clicking a video card no longer opens the player": coordinator methods run with `this` bound to the Application (bindCoordinator), and the zap-badge helpers were added as coordinator methods called via `this.subscribeModalZapTotal(...)` — but app.js only exposes coordinator methods through explicit delegates, which were never added. Every modal open threw "this.subscribeModalZapTotal is not a function" inside subscribeModalViewCount (and every close threw in teardown). Same delegate landmine that broke Most Zapped's loader. The helpers are now plain closures inside createModalCoordinator — immune to the delegate pattern by construction. Regression test drives the REAL bindCoordinator with only app.js's actual delegates, so any future cross-method `this.` call without a delegate fails in CI first. Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
1 parent
941580f227
commit
213cb97ee8
2 files changed
+174
-55
No files matched your search
+66
-55
@@ -34,6 +34,70 @@ export function createModalCoordinator(deps) {
|
||||
BITVID_WEBSITE_URL,
|
||||
} = deps;
|
||||
|
||||
// Zap-total badge helpers are LOCAL closures, NOT coordinator methods:
|
||||
// coordinator methods run with `this` bound to the Application, so a
|
||||
// cross-method `this.helper()` call only works when app.js also adds an
|
||||
// explicit delegate for it — a silent landmine (a missing delegate here
|
||||
// threw on EVERY modal open and broke video playback entirely).
|
||||
let modalZapTotalUnsub = null;
|
||||
|
||||
function teardownModalZapTotal() {
|
||||
if (typeof modalZapTotalUnsub === "function") {
|
||||
try {
|
||||
modalZapTotalUnsub();
|
||||
} catch (error) {
|
||||
devLogger.warn("[zapTotals] Failed to tear down modal listener:", error);
|
||||
}
|
||||
}
|
||||
modalZapTotalUnsub = null;
|
||||
const el =
|
||||
typeof document !== "undefined"
|
||||
? document.getElementById("videoZapTotal")
|
||||
: null;
|
||||
if (el) {
|
||||
el.classList.add("hidden");
|
||||
const text = el.querySelector("[data-zap-total-text]");
|
||||
if (text) {
|
||||
text.textContent = "";
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Orange sats badge on the modal's meta row (right side; views stay left).
|
||||
// Mirrors the card badge: cached total now (also schedules the batched
|
||||
// receipt fetch), re-rendered when a batch lands, hidden while zero.
|
||||
function subscribeModalZapTotal(pointer) {
|
||||
if (
|
||||
typeof requestVideoZapTotal !== "function" ||
|
||||
typeof getVideoZapTotalSnapshot !== "function" ||
|
||||
typeof onZapTotalsChanged !== "function"
|
||||
) {
|
||||
return;
|
||||
}
|
||||
const el =
|
||||
typeof document !== "undefined"
|
||||
? document.getElementById("videoZapTotal")
|
||||
: null;
|
||||
if (!el || !pointer) {
|
||||
return;
|
||||
}
|
||||
const render = () => {
|
||||
const sats = getVideoZapTotalSnapshot(pointer);
|
||||
const text = el.querySelector("[data-zap-total-text]");
|
||||
if (Number.isFinite(sats) && sats > 0) {
|
||||
if (text) {
|
||||
text.textContent = `${formatViewCount(sats)} sats`;
|
||||
}
|
||||
el.classList.remove("hidden");
|
||||
} else {
|
||||
el.classList.add("hidden");
|
||||
}
|
||||
};
|
||||
requestVideoZapTotal(pointer);
|
||||
render();
|
||||
modalZapTotalUnsub = onZapTotalsChanged(render);
|
||||
}
|
||||
|
||||
return {
|
||||
normalizeModalTrigger(candidate) {
|
||||
if (!candidate) {
|
||||
@@ -120,66 +184,13 @@ export function createModalCoordinator(deps) {
|
||||
}
|
||||
}
|
||||
this.modalViewCountUnsub = null;
|
||||
this.teardownModalZapTotal();
|
||||
teardownModalZapTotal();
|
||||
if (this.videoModal) {
|
||||
this.videoModal.updateViewCountLabel("\u2013 views");
|
||||
this.videoModal.setViewCountPointer(null);
|
||||
}
|
||||
},
|
||||
|
||||
teardownModalZapTotal() {
|
||||
if (typeof this.modalZapTotalUnsub === "function") {
|
||||
try {
|
||||
this.modalZapTotalUnsub();
|
||||
} catch (error) {
|
||||
devLogger.warn("[zapTotals] Failed to tear down modal listener:", error);
|
||||
}
|
||||
}
|
||||
this.modalZapTotalUnsub = null;
|
||||
const el =
|
||||
typeof document !== "undefined"
|
||||
? document.getElementById("videoZapTotal")
|
||||
: null;
|
||||
if (el) {
|
||||
el.classList.add("hidden");
|
||||
const text = el.querySelector("[data-zap-total-text]");
|
||||
if (text) {
|
||||
text.textContent = "";
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
// Orange sats badge on the modal's meta row (right side; views stay left).
|
||||
// Mirrors the card badge: cached total now (also schedules the batched
|
||||
// receipt fetch), re-rendered when a batch lands, hidden while zero.
|
||||
subscribeModalZapTotal(pointer) {
|
||||
if (typeof requestVideoZapTotal !== "function") {
|
||||
return;
|
||||
}
|
||||
const el =
|
||||
typeof document !== "undefined"
|
||||
? document.getElementById("videoZapTotal")
|
||||
: null;
|
||||
if (!el || !pointer) {
|
||||
return;
|
||||
}
|
||||
const render = () => {
|
||||
const sats = getVideoZapTotalSnapshot(pointer);
|
||||
const text = el.querySelector("[data-zap-total-text]");
|
||||
if (Number.isFinite(sats) && sats > 0) {
|
||||
if (text) {
|
||||
text.textContent = `${formatViewCount(sats)} sats`;
|
||||
}
|
||||
el.classList.remove("hidden");
|
||||
} else {
|
||||
el.classList.add("hidden");
|
||||
}
|
||||
};
|
||||
requestVideoZapTotal(pointer);
|
||||
render();
|
||||
this.modalZapTotalUnsub = onZapTotalsChanged(render);
|
||||
},
|
||||
|
||||
subscribeModalViewCount(pointer, pointerKey) {
|
||||
const viewEl = this.videoModal?.getViewCountElement() || null;
|
||||
if (!viewEl) {
|
||||
@@ -196,7 +207,7 @@ export function createModalCoordinator(deps) {
|
||||
this.videoModal.updateViewCountLabel("Loading views\u2026");
|
||||
this.videoModal.setViewCountPointer(pointerKey);
|
||||
}
|
||||
this.subscribeModalZapTotal(pointer);
|
||||
subscribeModalZapTotal(pointer);
|
||||
try {
|
||||
const token = subscribeToVideoViewCount(pointer, ({ total, status, partial }) => {
|
||||
const latestViewEl = this.videoModal?.getViewCountElement() || null;
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
// Regression for "clicking a video no longer opens the player": coordinator
|
||||
// methods run with `this` bound to the APPLICATION (bindCoordinator), so a
|
||||
// cross-method `this.helper()` call inside the coordinator only works when
|
||||
// app.js also adds an explicit delegate — the zap-badge helpers were plain
|
||||
// methods, `this.subscribeModalZapTotal` didn't exist on the app, and EVERY
|
||||
// modal open threw. This test drives subscribeModalViewCount through the REAL
|
||||
// bindCoordinator with ONLY the delegates app.js actually defines, so any
|
||||
// future cross-method `this.` call without a delegate fails here first.
|
||||
//
|
||||
// test_integrity_note:
|
||||
// change_type: ["new_tests"]
|
||||
// scenarios:
|
||||
// - id: SCN-modal-open-no-missing-delegate
|
||||
// given: "a coordinator bound to an app stub exposing only app.js's real modal delegates"
|
||||
// when: "subscribeModalViewCount runs (the modal-open path) and teardown runs (close path)"
|
||||
// then: "no TypeError; the zap badge renders from the shared store and clears on teardown"
|
||||
// observable_outcomes:
|
||||
// - "subscribeModalViewCount does not throw (the user-facing 'video will not open' bug)"
|
||||
// - "badge hidden at zero, revealed with sats after a receipt batch, cleared on teardown"
|
||||
// determinism_controls:
|
||||
// - "JSDOM; scripted subscription-manager double; store flushed manually"
|
||||
// anti_cheat_rationale:
|
||||
// prevents: ["over-mocking internal logic", "hard-coded return value"]
|
||||
// relaxation:
|
||||
// did_relax_any_assertion: false
|
||||
|
||||
import "./test-helpers/setup-localstorage.mjs";
|
||||
import test from "node:test";
|
||||
import { strict as assert } from "node:assert";
|
||||
import { JSDOM } from "jsdom";
|
||||
import { createModalCoordinator } from "../js/app/modalCoordinator.js";
|
||||
import bindCoordinator from "../js/app/bindCoordinator.js";
|
||||
import zapTotalsStore, {
|
||||
initZapTotals,
|
||||
requestVideoZapTotal,
|
||||
getVideoZapTotalSnapshot,
|
||||
onZapTotalsChanged,
|
||||
} from "../js/zapTotals.js";
|
||||
import { formatViewCount } from "../js/viewCounter.js";
|
||||
|
||||
const A1 = "30078:" + "c".repeat(64) + ":video-1";
|
||||
|
||||
test("modal open path survives binding and drives the zap badge", async () => {
|
||||
const dom = new JSDOM(
|
||||
'<!DOCTYPE html><body><span id="videoZapTotal" class="hidden"><span data-zap-total-text></span></span></body>',
|
||||
{ pretendToBeVisual: true },
|
||||
);
|
||||
global.document = dom.window.document;
|
||||
|
||||
let receipts = [];
|
||||
initZapTotals({
|
||||
nostrClient: {
|
||||
relays: ["wss://relay.example"],
|
||||
getSubscriptionManager: () => ({ list: async () => receipts }),
|
||||
},
|
||||
tools: { nip57: { getSatoshisAmountFromBolt11: () => NaN } },
|
||||
});
|
||||
|
||||
const coordinator = createModalCoordinator({
|
||||
devLogger: { warn: () => {}, log: () => {}, error: () => {} },
|
||||
subscribeToVideoViewCount: () => "token-1",
|
||||
unsubscribeFromVideoViewCount: () => {},
|
||||
formatViewCount,
|
||||
requestVideoZapTotal,
|
||||
getVideoZapTotalSnapshot,
|
||||
onZapTotalsChanged,
|
||||
});
|
||||
|
||||
// App stub with ONLY the delegates app.js really defines for this path.
|
||||
const app = {
|
||||
videoModal: {
|
||||
getViewCountElement: () => dom.window.document.createElement("span"),
|
||||
updateViewCountLabel: () => {},
|
||||
setViewCountPointer: () => {},
|
||||
},
|
||||
formatViewCountLabel: (n) => `${n} views`,
|
||||
};
|
||||
app._modal = bindCoordinator(app, coordinator);
|
||||
app.teardownModalViewCountSubscription = (...args) =>
|
||||
app._modal.teardownModalViewCountSubscription(...args);
|
||||
app.subscribeModalViewCount = (...args) =>
|
||||
app._modal.subscribeModalViewCount(...args);
|
||||
|
||||
const pointer = ["a", A1];
|
||||
// The bug: this line threw "this.subscribeModalZapTotal is not a function".
|
||||
app.subscribeModalViewCount(pointer, `a:${A1}`);
|
||||
|
||||
const badge = dom.window.document.getElementById("videoZapTotal");
|
||||
assert.equal(badge.classList.contains("hidden"), true, "zero total stays hidden");
|
||||
|
||||
receipts = [
|
||||
{
|
||||
id: "r1",
|
||||
kind: 9735,
|
||||
tags: [["a", A1], ["description", JSON.stringify({ tags: [["amount", "21000"]] })]],
|
||||
},
|
||||
];
|
||||
await zapTotalsStore.flush();
|
||||
assert.equal(badge.classList.contains("hidden"), false, "receipts reveal the badge");
|
||||
assert.match(
|
||||
badge.querySelector("[data-zap-total-text]").textContent,
|
||||
/21 sats/,
|
||||
);
|
||||
|
||||
// Modal close path (also used to throw via this.teardownModalZapTotal).
|
||||
app.teardownModalViewCountSubscription();
|
||||
assert.equal(badge.classList.contains("hidden"), true, "teardown clears the badge");
|
||||
});
|
||||
Reference in new issue
Block a user