diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index 4e4a06d6..cbc336e1 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -114,6 +114,19 @@ function el(tag, cls, text) { if (text !== undefined) e.textContent = text; return e; } +// replaceKeeping is parent.replaceChildren(...nodes) that leaves a node +// already in parent where it is, rather than taking it out and putting it +// back. A filter redrawn from its own input event is one: taken out of the +// page while an IME commits, it gets the text twice (#1055: 1, 2 → 1122). +function replaceKeeping(parent, nodes) { + const keep = new Set(nodes); + for (const n of [...parent.childNodes]) if (!keep.has(n)) n.remove(); + let at = parent.firstChild; + for (const n of nodes) { + if (n === at) at = at.nextSibling; + else parent.insertBefore(n, at); + } +} function svg(d, size = 12, stroke = 1.6) { const s = document.createElementNS("http://www.w3.org/2000/svg", "svg"); s.setAttribute("viewBox", "0 0 16 16"); @@ -11755,11 +11768,16 @@ function keyFoldRows(p, fold) { q.className = "keys-filter"; q.dataset.provider = p.id; q.setAttribute("aria-label", q.placeholder); + // typing redraws the keys list around the filter, which stays in the + // page: taken out and put back, an IME's text lands twice (#1055) q.oninput = () => { keysOpen[p.id].q = q.value; - const at = q.selectionStart; - renderProviders(); - focusKeyFilter(p.id, at); + const list = q.closest(".accts.keys"), fresh = renderKeyAccounts(p); + const fbar = fresh.querySelector(".keys-tools"), fq = fbar?.querySelector(".keys-filter"); + if (!list || !fq) { renderProviders(); return; } + replaceKeeping(bar, [...fbar.childNodes].map((n) => n === fq ? q : n)); + list.className = fresh.className; + replaceKeeping(list, [...fresh.childNodes].map((n) => n === fbar ? bar : n)); }; q.onkeydown = (e) => { e.stopPropagation(); if (e.key === "Escape") { if (q.value) q.oninput(q.value = ""); else { delete keysOpen[p.id]; renderProviders(); } } }; const count = el("span", "hint keys-count", fold.matched.length === all.length ? t("{n} keys", { n: all.length }) : t("{n} of {m} keys", { n: fold.matched.length, m: all.length })); @@ -11791,11 +11809,8 @@ function keyFoldRows(p, fold) { else if (fold.matched.length > fold.shown.length) after.push(el("div", "hint keys-rest", t("{n} more — type to narrow them down", { n: fold.matched.length - fold.shown.length }))); return { before: [bar], after }; } -function focusKeyFilter(id, at) { - const q = document.querySelector(`.keys-filter[data-provider="${CSS.escape(id)}"]`); - if (!q) return; - q.focus({ preventScroll: true }); - if (at != null) q.setSelectionRange(at, at); +function focusKeyFilter(id) { + document.querySelector(`.keys-filter[data-provider="${CSS.escape(id)}"]`)?.focus({ preventScroll: true }); } // splitKeys is the keys in text pasted at once, as provider.SplitKeys diff --git a/internal/gui/assets/plugins.js b/internal/gui/assets/plugins.js index 86c15a26..7257ec44 100644 --- a/internal/gui/assets/plugins.js +++ b/internal/gui/assets/plugins.js @@ -101,13 +101,6 @@ for (const l of listings || []) if (npm[l.package]) l.npm = npm[l.package]; for (const e of mine?.plugins || []) if (onNPM(e.spec) && npm[name(e.spec)]?.version) e.latest = npm[name(e.spec)].version; } - // redrawn as a part comes: the search field keeps its focus - function redraw() { - const q = page.querySelector(".pm-find input"); - const typing = q && document.activeElement === q; - draw(); - if (typing) page.querySelector(".pm-find input")?.focus({ preventScroll: true }); - } async function loadMine() { try { mine = await api("plugins"); @@ -162,8 +155,8 @@ async function load() { if (!mine && !listings) draw(); const parts = [ - loadMine().then(() => { redraw(); window.renderPluginDot?.(); }), - loadListings().then(redraw), + loadMine().then(() => { draw(); window.renderPluginDot?.(); }), + loadListings().then(draw), loadTagged().then(drawBody), providers ? null : loadProviders().then(drawBody, () => {}), ]; @@ -471,8 +464,31 @@ return box; } + // the head and its search field are made once and drawn again in + // place: the field is never taken out of the page, or a key an IME + // commits while it is out lands twice (#1055) + const headBox = el("div", "lib-head pm-head"); + const find = el("label", "pm-find"); + const q = el("input"); + q.type = "search"; + q.spellcheck = false; + q.autocomplete = "off"; + q.oninput = () => { + query = q.value; + if (tab !== "discover") { tab = "discover"; draw(); } + clearTimeout(searchTimer); + const s = query.trim(); + if (s.length < 2) hits = null; + else { + hits = { q: s, loading: true }; + searchTimer = setTimeout(() => searchNPM(s), 350); + } + drawBody(); + }; + q.onkeydown = (ev) => { ev.stopPropagation(); if (ev.key === "Escape" && q.value) { q.value = ""; q.oninput(); } }; + find.append(glyph(SEARCH, 13, 1.6), q); + function head() { - const h = el("div", "lib-head pm-head"); const n = mine?.plugins?.length || 0; const tabs = segs([["discover", t("Discover")], ["installed", t("Installed") + (n ? " · " + n : "")]], tab, (id) => { tab = id; @@ -480,32 +496,12 @@ drawBody(); }); tabs.classList.add("lib-tabs"); - const find = el("label", "pm-find"); - find.append(glyph(SEARCH, 13, 1.6)); - const q = el("input"); - q.type = "search"; q.placeholder = t("Search plugins and npm…"); - q.value = query; - q.spellcheck = false; - q.autocomplete = "off"; + if (q.value !== query) q.value = query; q.setAttribute("aria-label", t("Search plugins")); - q.oninput = () => { - query = q.value; - if (tab !== "discover") { tab = "discover"; draw(); page.querySelector(".pm-find input")?.focus(); } - clearTimeout(searchTimer); - const s = query.trim(); - if (s.length < 2) hits = null; - else { - hits = { q: s, loading: true }; - searchTimer = setTimeout(() => searchNPM(s), 350); - } - drawBody(); - }; - q.onkeydown = (ev) => { ev.stopPropagation(); if (ev.key === "Escape" && q.value) { q.value = ""; q.oninput(); } }; - find.append(q); - h.append(tabs, el("span", "grow"), mirrorSwitch(), find); - h.classList.toggle("stuck", page.scrollTop > 0); - return h; + replaceKeeping(headBox, [tabs, el("span", "grow"), mirrorSwitch(), find]); + headBox.classList.toggle("stuck", page.scrollTop > 0); + return headBox; } // 「国内镜像」: the list, npm's packages and answers, and Bun asked of @@ -533,13 +529,13 @@ const r = await api("plugins/mirror", { on: !on }); mine.mirror = r.mirror; // the list again, now from the mirror, and npm's answers with it - if (r.mirror) loadListings().then(() => { redraw(); askNPM(); }); + if (r.mirror) loadListings().then(() => { draw(); askNPM(); }); } catch (e) { mine.mirror = on; status(t(e.message), "err"); } mirrorSaving = false; - redraw(); + draw(); }; return b; } @@ -559,9 +555,8 @@ let body = null; function draw() { const scroll = page.scrollTop; - page.replaceChildren(head()); body = el("div", "lib-body pm-body"); - page.append(body); + replaceKeeping(page, [head(), body]); drawBody(); page.scrollTop = scroll; } diff --git a/internal/gui/assets/routing.js b/internal/gui/assets/routing.js index cf2b679f..14d21e50 100644 --- a/internal/gui/assets/routing.js +++ b/internal/gui/assets/routing.js @@ -2651,8 +2651,8 @@ // Discord: they could only be removed one at a time); null otherwise let gSel = null; // gQ: the groups filtered by name and by the models in them, as many - // as there may be (PAMI on Discord); kept across redraws, and focused - // again when one comes while typing + // as there may be (PAMI on Discord); kept across redraws, never taken + // out of the page while it is there const gQ = el("input", "sess-filter rt-gfilter"); gQ.type = "search"; gQ.spellcheck = false; @@ -2677,7 +2677,6 @@ newBtn.append(svg(PLUS, 11, 1.8), el("span", "", t("New group"))); newBtn.onclick = () => newGroup(); const head = [el("span", "label", t("Routing groups")), el("span", "grow"), el("span", "note", t("models agents pick as one"))]; - const typing = document.activeElement === gQ, [a, b] = [gQ.selectionStart, gQ.selectionEnd]; if (all.length > 1 || gQ.value) { gQ.placeholder = t("Filter groups and models"); gQ.setAttribute("aria-label", gQ.placeholder); @@ -2696,8 +2695,9 @@ } head.push(newBtn); } - gHead.replaceChildren(...head); - if (typing && gQ.isConnected) { gQ.focus({ preventScroll: true }); try { gQ.setSelectionRange(a, b); } catch {} } + // the filter stays in the page as the rest of the row is drawn again, + // so the key being typed into it lands once (#1055) + replaceKeeping(gHead, head); drawFound(); drawNames(); const rows = []; diff --git a/internal/gui/tests/README.md b/internal/gui/tests/README.md index 0b25af7c..2f1c0006 100644 --- a/internal/gui/tests/README.md +++ b/internal/gui/tests/README.md @@ -2082,3 +2082,18 @@ unimportable entries, unreadable sources, stable fingerprints despite ID collisions, changed configuration fingerprints, excluding unticked collision and disabled Alma entries while preserving the add-key case, and keeping credentials server-side. + +## Filters typed through an IME + +`filter-ime.test.cjs` covers #1055: a digit typed into the Routing page's +group filter went in twice (1, 2 → 1122), because the filter's own input +event drew its row again and took the field out of the page while an IME +committed. It types into the routing group filter, a provider's keys filter +and the plugins search, through an IME in Chromium (CDP's +`Input.imeSetComposition`, then `Input.insertText`) and by key in WebKit, +and checks one character a key, the focus, the caret, and that the field +never leaves the page. The rows around each still follow it: the groups and +keys it matches, the routing header in the other language after +`setLocale`, the plugins listings arriving while the reader types. In +English, Chinese, Japanese and German, at 1100px and 440px. `MAGPIE_FILTER_ASSETS` points it +at another assets folder, to see it fail on the old code. diff --git a/internal/gui/tests/filter-ime.test.cjs b/internal/gui/tests/filter-ime.test.cjs new file mode 100644 index 00000000..66281c24 --- /dev/null +++ b/internal/gui/tests/filter-ime.test.cjs @@ -0,0 +1,209 @@ +// Run with Node's test runner and Playwright on the module path; see README.md. +// #1055: a digit typed into the Routing page's group filter went in twice +// (1, 2 → 1122). The filter's own input event drew its row again with +// replaceChildren, which took the focused field out of the page and put it +// back; an IME committing its text while the field was out wrote it twice. +// The provider editor's keys filter and the plugins search were made the +// same way. Each is typed into here, through an IME in Chromium (CDP's +// imeSetComposition, then insertText) and by key in both engines, and the +// field must keep one character a key, its focus and its caret, and never +// leave the page. The row around it still follows the filter and the +// language. In English, Chinese, Japanese and German, wide and at 440px. +const assert = require("node:assert/strict"); +const fs = require("node:fs/promises"); +const path = require("node:path"); +const { test } = require("node:test"); +const { chromium, webkit } = require("playwright"); + +const assets = process.env.MAGPIE_FILTER_ASSETS || path.resolve(__dirname, "../assets"); + +const models = [ + { id: "a/one", name: "one", providerName: "A", icon: "generic" }, + { id: "a/two", name: "two", providerName: "A", icon: "generic" }, + { id: "a/m12", name: "m12", providerName: "A", icon: "generic" }, +]; +const groups = [ + { id: "g", name: "G", members: ["a/one"], off: [], routing: "order", ready: true, memberInfo: [{ id: "a/one", ready: true }] }, + { id: "o", name: "O", members: ["a/two"], off: [], routing: "order", ready: true, memberInfo: [{ id: "a/two", ready: true }] }, + { id: "v12", name: "v12", members: ["a/m12"], off: [], routing: "order", ready: true, memberInfo: [{ id: "a/m12", ready: true }] }, +]; +const N = 300; +const idOf = (i) => i.toString(16).padStart(10, "0"); +const relay = () => ({ + id: "relay", name: "Relay", icon: "generic", chat: "https://relay.example.com/v1", responses: "", anthropic: "", catalog: "", + models: [{ id: "big", name: "", on: true }], agents: [], fallback: [], headers: {}, + key: { set: true, masked: "sk-…0000" }, balanceToken: { takes: false, set: false }, proxy: "", + keyList: Array.from({ length: N }, (_, i) => ({ id: idOf(i), name: "", masked: "sk-…" + String(i).padStart(4, "0"), on: true, active: i === 0 })), +}); +const listings = [ + { package: "@magpie-community/opencode-zed-auth", name: "Zed", icon: "zed", providers: ["zed"], community: true, + summary: { en: "Your Zed plan.", zh: "Zed 订阅。" }, npm: { version: "0.1.0", weekly: 120 } }, + { package: "opencode-gemini-auth", name: "Gemini", icon: "gemini-color", providers: ["google"], + summary: { en: "Gemini with a Google account.", zh: "用 Google 账号使用 Gemini。" }, npm: { version: "1.4.0", weekly: 9100 } }, +]; + +// serve answers the page from fixtures; held, when given, keeps the plugin +// listings back until it is resolved, as a slow npm mirror would +function serve(lang, held) { + return async (route) => { + const url = new URL(route.request().url()); + const json = (data) => route.fulfill({ json: data }); + if (url.pathname === "/boot.js") return route.fulfill({ contentType: "text/javascript", body: `window.bootPrefs = {lang:"${lang}",theme:"light",web:true};` }); + if (url.pathname === "/wails/runtime.js") return route.fulfill({ contentType: "text/javascript", body: "export const Window = {};" }); + if (url.pathname === "/api/state") return json({ agents: [], profiles: [], settings: { lang, theme: "light" } }); + if (url.pathname === "/api/gateway/trace") { + if (url.searchParams.get("wait")) return new Promise(() => {}); + return json({ mine: true, now: new Date().toISOString(), seq: 1, totals: { requests: 0, rerouted: 0, errors: 0 }, routes: [] }); + } + if (url.pathname === "/api/gateway/history") return json({ cut: false, days: [], routes: [] }); + if (url.pathname.startsWith("/api/groups")) return json({ models, pools: [], groups }); + if (url.pathname === "/api/providers") return json({ providers: [relay()], presets: [], excluded: [], gateway: { running: true, window: true }, plugins: [] }); + if (url.pathname === "/api/plugins") return json({ bun: true, bunVersion: "1.3.0", plugins: [{ spec: "opencode-gemini-auth@1.4.0", providers: ["Gemini"], version: "1.4.0" }] }); + if (url.pathname === "/api/plugins/listings") { + if (held) await held; + return json({ listings }); + } + if (url.pathname === "/api/plugins/search") return json({ hits: [] }); + if (url.pathname.startsWith("/api/")) return json({}); + const file = path.join(assets, url.pathname === "/" ? "index.html" : url.pathname); + const contentType = { ".html": "text/html", ".js": "text/javascript", ".css": "text/css", ".svg": "image/svg+xml", ".png": "image/png" }[path.extname(file)]; + try { await route.fulfill({ body: await fs.readFile(file), contentType }); } catch { await route.fulfill({ status: 404, body: "" }); } + }; +} + +// watch counts each time the field, or a box it sits in, is taken out of +// the page from now on +const watch = (page, sel) => page.evaluate((sel) => { + const q = document.querySelector(sel); + window.__field = q; + window.__out = 0; + new MutationObserver((recs) => { + for (const r of recs) for (const n of r.removedNodes) if (n === q || n.contains?.(q)) window.__out++; + }).observe(document.body, { childList: true, subtree: true }); +}, sel); +const field = (page) => page.evaluate(() => { + const q = window.__field, now = document.activeElement; + return { out: window.__out, same: now === q, value: q.value, caret: [q.selectionStart, q.selectionEnd], connected: q.isConnected }; +}); + +// ime types each character the way an input method commits it: a +// composition, then its text (Chromium only, through CDP) +async function ime(page, text) { + const c = await page.context().newCDPSession(page); + for (const ch of text) { + await c.send("Input.imeSetComposition", { text: ch, selectionStart: 1, selectionEnd: 1 }); + await c.send("Input.insertText", { text: ch }); + await page.waitForTimeout(30); + } + await c.detach(); +} + +// other: the language the routing header is switched to, and its words +const words = { + en: { label: "Routing groups", installed: "Installed", of: "1 of 300 keys", other: ["zh", "路由组", "筛选路由组和模型"] }, + zh: { label: "路由组", installed: "已安装", of: "300 个密钥中的 1 个", other: ["en", "Routing groups", "Filter groups and models"] }, + ja: { label: "ルーティンググループ", installed: "インストール済み", of: "300 個中 1 個のキー", other: ["en", "Routing groups", "Filter groups and models"] }, + de: { label: "Weiterleitungsgruppen", installed: "Installiert", of: "1 von 300 Schlüsseln", other: ["en", "Routing groups", "Filter groups and models"] }, +}; + +for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) { + for (const lang of ["en", "zh", "ja", "de"]) { + for (const width of [1100, 440]) { + const w = words[lang]; + const open = async (t, view, held) => { + const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" })); + t.after(() => browser.close()); + const page = await (await browser.newContext({ viewport: { width, height: 900 }, reducedMotion: "reduce" })).newPage(); + page.setDefaultTimeout(5000); + const errors = []; + page.on("pageerror", (e) => errors.push(e.message)); + await page.route("**/*", serve(lang, held)); + await page.goto("http://magpie.test/?view=" + view); + return { page, errors }; + }; + // typeIn: through an IME where there is one to drive, by key elsewhere + const typeIn = (page, text) => engine === "chromium" ? ime(page, text) : page.keyboard.type(text); + + test(`${engine} ${lang} ${width}px: the routing group filter takes one character a key`, async (t) => { + const { page, errors } = await open(t, "routing"); + const q = page.locator("input.rt-gfilter"); + await q.waitFor(); + await watch(page, "input.rt-gfilter"); + await q.focus(); + await typeIn(page, "12"); + assert.deepEqual(await field(page), { out: 0, same: true, value: "12", caret: [2, 2], connected: true }, "the filter after 1, 2"); + // the groups follow it: only v12 has a 12 in it + await page.waitForFunction(() => [...document.querySelectorAll(".rt-groups .rt-group")].map((r) => r.dataset.id).join() === "v12"); + assert.equal(await page.locator(".rt-gsec .row-head").first().locator(".label").textContent(), w.label); + // a key in the middle goes where the caret is + await page.keyboard.press("ArrowLeft"); + await typeIn(page, "5"); + assert.deepEqual(await field(page), { out: 0, same: true, value: "152", caret: [2, 2], connected: true }, "the filter after ←, 5"); + await page.keyboard.press("Backspace"); + await page.keyboard.press("ArrowRight"); + await page.keyboard.press("Backspace"); + assert.deepEqual(await field(page), { out: 0, same: true, value: "1", caret: [1, 1], connected: true }, "the filter after two Backspaces"); + // the row around it is still drawn again: in the other language + // its words change, and the filter stays where it was + await page.evaluate((l) => setLocale(l), w.other[0]); + await page.waitForFunction((s) => document.querySelector(".rt-gsec .row-head .label")?.textContent === s, w.other[1]); + assert.equal(await q.getAttribute("placeholder"), w.other[2]); + const f = await field(page); + assert.equal(f.out, 0, "switching the language took the filter out"); + assert.equal(f.value, "1"); + // Select takes the buttons beside it away and leaves the filter + await q.fill(""); + await page.locator(".rt-gselect").click(); + await page.locator(".rt-gsel").waitFor(); + assert.equal(await page.locator(".rt-gsec .row-head .rt-gselect, .rt-gsec .row-head .rt-gnew").count(), 0); + assert.equal(await page.locator(".rt-gsec .row-head input.rt-gfilter").count(), 1); + assert.deepEqual(errors, []); + }); + + test(`${engine} ${lang} ${width}px: the provider keys filter takes one character a key`, async (t) => { + const { page, errors } = await open(t, "providers"); + await page.locator(".row.provider", { hasText: "Relay" }).first().click(); + await page.locator(".editor .keys-more").click(); + const q = page.locator(".keys-tools .keys-filter"); + await q.waitFor(); + await watch(page, ".keys-tools .keys-filter"); + await q.focus(); + await typeIn(page, "12"); + assert.deepEqual(await field(page), { out: 0, same: true, value: "12", caret: [2, 2], connected: true }, "the filter after 1, 2"); + await typeIn(page, "9"); + assert.deepEqual(await field(page), { out: 0, same: true, value: "129", caret: [3, 3], connected: true }, "the filter after 9"); + // the keys, and the count beside the filter, follow it: only sk-…0129 + await page.waitForFunction(() => document.querySelectorAll(".editor .accts .acc[data-account-id]").length === 1); + assert.equal((await page.locator(".keys-count").textContent()).trim(), w.of); + assert.equal(await page.locator(".keys-tools").count(), 1); + await page.keyboard.press("Escape"); + assert.deepEqual(await field(page), { out: 0, same: true, value: "", caret: [0, 0], connected: true }, "Escape empties the filter"); + await page.waitForFunction(() => document.querySelectorAll(".editor .accts .acc[data-account-id]").length === 50); + assert.deepEqual(errors, []); + }); + + test(`${engine} ${lang} ${width}px: the plugins search takes one character a key`, async (t) => { + let release; + const held = new Promise((r) => { release = r; }); + const { page, errors } = await open(t, "plugins", held); + const q = page.locator(".pm-find input"); + await q.waitFor(); + // from Installed, the first key goes to Discover + await page.locator(".lib-tabs .opt", { hasText: w.installed }).click(); + await watch(page, ".pm-find input"); + await q.focus(); + await typeIn(page, "1"); + assert.deepEqual(await field(page), { out: 0, same: true, value: "1", caret: [1, 1], connected: true }, "the search after 1 from Installed"); + // the listings come while the reader types + release(); + await page.locator(".pm-sec").first().waitFor(); + await typeIn(page, "2"); + assert.deepEqual(await field(page), { out: 0, same: true, value: "12", caret: [2, 2], connected: true }, "the search after the listings came, and 2"); + await page.keyboard.press("Escape"); + assert.equal(await q.inputValue(), ""); + assert.equal((await field(page)).out, 0); + assert.deepEqual(errors, []); + }); + } + } +}