diff --git a/internal/gateway/ui/index.html b/internal/gateway/ui/index.html index 02d5cfd..1a9f83a 100644 --- a/internal/gateway/ui/index.html +++ b/internal/gateway/ui/index.html @@ -1714,7 +1714,20 @@ `; document.body.appendChild(wrap); } - function downloadStatsCsv(from, to) { + // closeTopModal removes the topmost dialog. Several dialogs share the id + // "modal-wrap" and more than one can be open at once (the seed-key notice + // sits on top of the keys page), so `$("#modal-wrap")` resolves to + // whichever comes FIRST in the document — not necessarily the one the + // user is looking at. Handlers that receive the clicked button resolve + // their own dialog with .closest(); handlers with no button reference + // use this, which matches what the user sees. + function closeTopModal() { + const all = [...document.querySelectorAll("#modal-wrap")]; + const w = all[all.length - 1]; + if (w) w.remove(); + return w; + } + function downloadStatsCsv(from, to, btn) { const q = new URLSearchParams({ export: "csv", from: String(Math.floor(from)), @@ -1722,8 +1735,7 @@ }); if (statsKeyF) q.set("key", statsKeyF); location.href = "/api/stats?" + q.toString(); - const w = $("#modal-wrap"); - if (w) w.remove(); + (btn ? btn.closest("#modal-wrap") : null) || closeTopModal(); } // downloadStatsCsvFromForm reads the custom date range from the export // modal's date inputs and forwards to downloadStatsCsv. (Previously this @@ -1741,12 +1753,11 @@ if (!isFinite(from) || from <= 0) return toast(t("expRange")); downloadStatsCsv(from, to); } - function downloadKeysCsv() { + function downloadKeysCsv(btn) { location.href = "/api/stats?export=keys-csv" + (statsKeyF ? "&key=" + encodeURIComponent(statsKeyF) : ""); - const w = $("#modal-wrap"); - if (w) w.remove(); + (btn ? btn.closest("#modal-wrap") : null) || closeTopModal(); } async function paintStats() { try { @@ -2940,9 +2951,10 @@ body: JSON.stringify(payload), }); toast(t("toastSaved")); - const w = $("#modal-wrap"); + const w = btn && btn.closest("#modal-wrap"); if (w) w.remove(); else if (window._modal) window._modal.remove(); + else closeTopModal(); renderSources(); } catch (e) { toast(tFmt("toastSaveFail", e.message)); @@ -3099,11 +3111,11 @@ ? `
` + tpls .map( - (t) => - ` - - `, + (tpl) => + ` + + `, ) .join("") + "
${t("tName")}${t("tURL")}${t("tAdapter")}${t("tModels")}
${esc(t.name)}${esc(t.base_url)}${esc(t.adapter)}
${(t.models || []).map((m) => `${esc(m.id)}`).join("")}
-
${esc(tpl.name)}${esc(tpl.base_url)}${esc(tpl.adapter)}
${(tpl.models || []).map((m) => `${esc(m.id)}`).join("")}
+
" @@ -3203,8 +3215,9 @@ body: JSON.stringify({ name: nm, ...body }), }); toast(t("toastSaved")); - const w = $("#modal-wrap"); + const w = btn && btn.closest("#modal-wrap"); if (w) w.remove(); + else closeTopModal(); openTemplateModal(); } catch (e) { toast(e.message); @@ -3910,7 +3923,7 @@ -

+

`; document.body.appendChild(wrap); @@ -3920,7 +3933,7 @@ }); $("#a-model").focus(); } - function scrAddFromForm() { + function scrAddFromForm(btn) { const raw = $("#a-model").value.trim(); if (!raw) { toast(t("kName")); @@ -3943,14 +3956,14 @@ ], }); const li = sortState.lanes.length - 1; - const w = $("#modal-wrap"); - if (w) w.remove(); + if (btn) btn.closest("#modal-wrap")?.remove(); + else closeTopModal(); paintSort([li]); persistAuto() .then(() => toast(t("sortHintSave"))) .catch((e) => toast(e.message)); } - function sortScopeSave(li, ji) { + function sortScopeSave(li, ji, btn) { const it = sortState.lanes[li].models[ji]; if (!it) return; let q = parseInt($("#q-quota").value); @@ -3959,8 +3972,7 @@ let h = parseInt($("#q-hours").value); if (isNaN(h) || h < 1) h = 1; it.meta = { quota: q, period: p, hours: p || q ? h : 0 }; - const w = $("#modal-wrap"); - if (w) w.remove(); + (btn ? btn.closest("#modal-wrap") : null) || closeTopModal(); paintSort([li]); persistAuto() .then(() => toast(t("sortHintSave"))) @@ -4508,9 +4520,10 @@ }); // Close THIS modal, not whichever #modal-wrap comes first in the // document: another dialog (e.g. the seed-key notice) may already be - // open, and $("#modal-wrap") would remove that one and leave this - // form stranded on screen. + // open, and a bare $("#modal-wrap") would remove that one and leave + // this form stranded on screen. if (wrap) wrap.remove(); + else closeTopModal(); toast(t("kSaved")); await loadKeys(); } catch (e) { @@ -4668,8 +4681,9 @@ if (btn) btn.disabled = true; try { await putScope(key, scopes); - const w = $("#modal-wrap"); + const w = btn && btn.closest("#modal-wrap"); if (w) w.remove(); + else closeTopModal(); toast(t("kSaved")); await loadKeys(); } catch (e) { @@ -4898,6 +4912,7 @@ // would otherwise be the one that gets removed. const w = btn ? btn.closest("#modal-wrap") : null; if (w) w.remove(); + else closeTopModal(); $("#k-newbox").innerHTML = `
diff --git a/internal/gateway/ui_quota_contract_test.go b/internal/gateway/ui_quota_contract_test.go index 93c14c3..17c1f05 100644 --- a/internal/gateway/ui_quota_contract_test.go +++ b/internal/gateway/ui_quota_contract_test.go @@ -45,6 +45,67 @@ func TestUIDialogClosesItselfNotTheFirstModal(t *testing.T) { } } +// TestUIDialogClosuresGoThroughSafePaths pins the rule across the whole +// document by data flow rather than by pattern: every handler that closes a +// dialog must do it one of the two safe ways. A handler could contain a +// correct .closest() and still close the wrong dialog on another path. +func TestUIDialogClosuresGoThroughSafePaths(t *testing.T) { + src := stripJSComments(uiSource(t)) + for _, fn := range []string{ + "downloadStatsCsv", "downloadKeysCsv", "saveSource", "saveTemplate", + "scrAddFromForm", "sortScopeSave", "scopeSave", "keyQuotaSave", "createKey", + } { + body, ok := jsFunctionBody(src, fn) + if !ok { + t.Errorf("%s not found", fn) + continue + } + if !strings.Contains(body, `closest("#modal-wrap")`) && !strings.Contains(body, "closeTopModal()") { + t.Errorf("%s closes a dialog with neither .closest nor closeTopModal", fn) + } + } + if !strings.Contains(src, "function closeTopModal(") { + t.Error("closeTopModal helper is missing") + } +} + +// The helper must pick the LAST dialog (the topmost one the user sees), not the +// first — that inversion is the whole bug. +func TestUICloseTopModalTakesTheLast(t *testing.T) { + body, ok := jsFunctionBody(uiSource(t), "closeTopModal") + if !ok { + t.Fatal("closeTopModal not found") + } + if !strings.Contains(body, "all.length - 1") { + t.Errorf("closeTopModal does not take the last dialog:\n\t%s", oneLine(body)) + } +} + +// Handlers that resolve their dialog from a button must actually receive one: +// a signature without the parameter means the .closest() silently yields null +// and the save leaves its form stranded on screen. +func TestUIDialogHandlersReceiveTheirButton(t *testing.T) { + src := stripJSComments(uiSource(t)) + for _, fn := range []string{ + "downloadStatsCsv", "saveSource", "scrAddFromForm", "sortScopeSave", + "scopeSave", "keyQuotaSave", "createKey", + } { + body, ok := jsFunctionBody(src, fn) + if !ok { + t.Errorf("%s not found", fn) + continue + } + if !strings.Contains(body, "closest(\"#modal-wrap\")") { + continue // uses closeTopModal only + } + sig := body[:strings.Index(body, ")")+1] + if !strings.Contains(sig, "btn") { + t.Errorf("%s uses .closest(\"#modal-wrap\") but its signature %s has no button parameter —\n"+ + "the lookup would always be null and the form would never close", fn, oneLine(sig)) + } + } +} + // stripJSComments removes // line comments and /* block */ comments from JS // embedded in the UI document. It is deliberately simple (no string/regex // awareness beyond skipping quoted spans on the same line): the document is