From f814ff7468d03e700ba8c4c569250d150db7e0f5 Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Sun, 27 Sep 2026 17:57:15 +0800 Subject: [PATCH] =?UTF-8?q?fix(webui):=20=E4=BF=AE=207=20=E5=A4=84?= =?UTF-8?q?=E5=BC=B9=E7=AA=97=E5=85=B3=E9=97=AD=E9=94=99=E5=AF=B9=E8=B1=A1?= =?UTF-8?q?=20+=20=E6=A8=A1=E6=9D=BF=E7=AE=A1=E7=90=86=E5=99=A8=E5=8F=98?= =?UTF-8?q?=E9=87=8F=E9=81=AE=E8=94=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 上一提交只修了自己新加的两处弹窗，全站其余 7 处是同一缺陷：所有对话框 共用 id="modal-wrap"（CSS `#modal-wrap:not(:empty){display:flex}`）且可以叠 加（seed-key 提示就盖在密钥页上），而 `const w = $("#modal-wrap"); w.remove()` 移除的是**文档里第一个**，不是用户 刚提交的那一个。 逐处改为两种安全写法： - 能拿到按钮的（saveSource / saveTemplate / sortScopeSave / scopeSave / keyQuotaSave / createKey / downloadStatsCsv / downloadKeysCsv / scrAddFromForm）：`btn.closest("#modal-wrap")` - 拿不到按钮的：新增 `closeTopModal()` 取**最后一个**（用户看到的那个）， 并作为所有 `if (w) w.remove()` 之后的兜底 - 顺带把 sortScopeSave / downloadStatsCsv / downloadKeysCsv / scrAddFromForm 的签名补上 btn / this 参数 —— 否则 .closest 恒为 null，表单永远不关 同时修一个相邻的既有 bug：`openTemplateModal` 的 `.map((t) => ...)` 用 t 做 循环变量，模板字面量里又调 t("srcEdit")，t 被遮蔽成对象 ⇒ 打开模板管理器 直接抛 `t is not a function`，整个弹窗渲染失败（main 上就有，git show 确认）。 参数改名 tpl。修后模板管理器完整渲染（浏览器实测：DeepSeek / 智谱 / Kimi / SiliconFlow 各行 + Edit/Delete 按钮文案全部正常，零异常）。 判据从 2 处扩到全量（internal/gateway/ui_quota_contract_test.go，+3 例）： - 全文档扫描：任何 `.remove()` 配裸 `$("#modal-wrap")` 即失败 - 9 个关闭对话框的处理器必须走 .closest 或 closeTopModal - closeTopModal 必须取 all.length - 1（首尾颠倒就是原 bug） - 用 .closest 的处理器，其签名必须真的有 btn 参数 —— 否则查找恒为 null， 表单永远不关 5/5 变异全被抓：sortScopeSave 去 btn 参数、closeTopModal 取第一个、scopeSave 退回裸选择器、saveSource 退回裸选择器、downloadKeysCsv 删兜底。 浏览器实测（共享 Chromium CDP，真实进程，每处都插入一个「decoy」弹窗 占据文档首位，复现原 bug 的触发条件）： - scopeSave / keyQuotaSave / scrAddFromForm / saveSource / saveTemplate 五个处理器：自己的表单关、decoy 保留 ✓ - 零 JS 异常 ★ 测试自身踩了两个坑，都不是代码问题：① `document.querySelector(sel) && .click()` 在 CDP 里求值为 undefined，改成箭头函数；② 编辑模板时没填名字就点保存， saveTemplate 因 `if (!nm)` 早退、fetch 零调用 —— 一开始我把这个误读成 「修复失效」，加 fetch 拦截 + 读 #s-name 的值才定位到是测试数据缺失。 ⇒ 「点按钮没反应」要先分清是「事件没触发」「请求失败」还是「早退」。 (cherry picked from commit b5c3fea0bb81b11b52be0af20d71b63661a6ee3b) --- internal/gateway/ui/index.html | 61 ++++++++++++++-------- internal/gateway/ui_quota_contract_test.go | 61 ++++++++++++++++++++++ 2 files changed, 99 insertions(+), 23 deletions(-) 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