From 457119fec386eef2a755c38aa6137933abedfc88 Mon Sep 17 00:00:00 2001 From: yetone Date: Fri, 9 Oct 2026 12:13:57 +0800 Subject: [PATCH] gui: text selected in a dialog and let go outside it no longer closes the dialog; only a press and a release both on the backdrop do (#1373, xiaozhu1337) Dragging a selection out of a field (to bring in the part past its end) and letting go over the backdrop gave the click to the press's and release's nearest shared ancestor, the backdrop, so every backdrop handler that checked only e.target closed the dialog. backdropClick(el, on) in app.js records where the pointer was pressed and let go, and a click counts as one on the backdrop only when both were. Every dialog that closes from its backdrop now asks it: - the shared #modal (provider and other editors, cancelEdit), - the confirmation that borrows #modal (closeConfirmAsk), - the library's dialog that borrows #modal (closeLibModal, library.js), - confirmAction's (its ::backdrop is the dialog outside its box). Menus and pickers close on mousedown outside and were never affected. Verified: - new internal/gui/tests/dialog-backdrop-drag.test.cjs, Chromium and WebKit, en and zh, 1100px and 440px: 8/8 pass. With the change reverted all 8 fail ("a selection dragged out keeps the dialog"); with only confirmAction reverted they fail on "a selection dragged out of the confirmation keeps it". - the whole GUI suite in both engines: the only failures are purpose-filter in WebKit (fails the same without this change, #1291), rate-limited-sink "says who is at the back" in WebKit (fails the same without this change) and library-servers-bulk in Chromium (failed under the full run only, passes alone). - real GUI: a magpie built from this tree (`magpie web` under a sandbox HOME on 127.0.0.1:3486), Chromium and WebKit, zh and en, 440px and 980px: a drag from the provider editor's Name field out over the backdrop selects the text and leaves the editor open; a click on the backdrop still closes it. - go test -tags nogui ./internal/gui under a temp HOME; GOOS=linux and GOOS=windows go vet -tags nogui ./internal/gui. --- internal/gui/assets/app.js | 25 ++++- internal/gui/assets/library.js | 2 +- .../gui/tests/dialog-backdrop-drag.test.cjs | 103 ++++++++++++++++++ 3 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 internal/gui/tests/dialog-backdrop-drag.test.cjs diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index f5967d8c..4bbf616f 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -6900,6 +6900,18 @@ function providerDirty() { return !!draft && draft === providerDraftRef && (providerDraftValue() !== providerDraftBase || (active?.matches(".editor .mprice input") && active.value !== active.defaultValue)); } +// backdropClick(el, on) says whether a click on el is one on its backdrop: +// pressed and let go both where on(e) says the backdrop is. A press inside +// the dialog let go out over the backdrop, a selection dragged out of a +// field (#1373), clicks the two's nearest shared ancestor, the backdrop, so +// the click's own target can't tell it apart. Every dialog that closes from +// its backdrop asks this. +function backdropClick(el, on = (e) => e.target === el) { + let down = false, up = false; + el.addEventListener("pointerdown", (e) => { down = on(e); up = false; }, true); + el.addEventListener("pointerup", (e) => { up = down && on(e); }, true); + return (e) => up && on(e); +} let confirmationPending = null; function confirmAction(title, message, action) { if (confirmationPending) return Promise.resolve(false); @@ -6927,10 +6939,12 @@ function confirmAction(title, message, action) { cancel.onclick = () => finish(false); accept.onclick = () => finish(true); dialog.addEventListener("cancel", (event) => { event.preventDefault(); finish(false); }); - dialog.addEventListener("click", (event) => { + // the dialog's ::backdrop is the dialog itself, outside its box + const onBackdrop = backdropClick(dialog, (event) => { const rect = dialog.getBoundingClientRect(); - if (event.target === dialog && (event.clientX < rect.left || event.clientX > rect.right || event.clientY < rect.top || event.clientY > rect.bottom)) finish(false); + return event.target === dialog && (event.clientX < rect.left || event.clientX > rect.right || event.clientY < rect.top || event.clientY > rect.bottom); }); + dialog.addEventListener("click", (event) => { if (onBackdrop(event)) finish(false); }); // Keep the underlying editor's Escape handlers from seeing this dialog. dialog.addEventListener("keydown", (event) => { event.stopPropagation(); @@ -7649,7 +7663,10 @@ function closeModal() { }, () => {}); return done; } -$("#modal").onclick = (e) => { if (e.target === e.currentTarget) cancelEdit(); }; +// the providers page's dialog; the library and a confirmation that borrow it +// ask the same of its backdrop +const modalBackdrop = backdropClick($("#modal")); +$("#modal").onclick = (e) => { if (modalBackdrop(e)) cancelEdit(); }; // ---------- sliding thumb ---------- // Pills (the nav, every segmented control) have one thumb that glides to the @@ -14707,7 +14724,7 @@ function closeConfirmAsk() { // the dialog is the providers page's: while this asks, its backdrop and // Escape closes only the confirmation (in the panel it would hide the window) $("#modal").addEventListener("click", (e) => { - if (confirmAsk && e.target === e.currentTarget) { e.stopImmediatePropagation(); closeConfirmAsk(); } + if (confirmAsk && modalBackdrop(e)) { e.stopImmediatePropagation(); closeConfirmAsk(); } }, true); document.addEventListener("keydown", (e) => { if (!confirmationPending && confirmAsk && e.key === "Escape") { e.preventDefault(); e.stopImmediatePropagation(); closeConfirmAsk(); } diff --git a/internal/gui/assets/library.js b/internal/gui/assets/library.js index 7cc21563..452ba8a6 100644 --- a/internal/gui/assets/library.js +++ b/internal/gui/assets/library.js @@ -3924,7 +3924,7 @@ // The dialog is the providers page's; while the library has it, its // backdrop and Escape close it here. $("#modal").addEventListener("click", (e) => { - if (modal && e.target === e.currentTarget) { e.stopImmediatePropagation(); closeLibModal(); } + if (modal && modalBackdrop(e)) { e.stopImmediatePropagation(); closeLibModal(); } }, true); document.addEventListener("keydown", (e) => { if (!confirmationPending && modal && e.key === "Escape") { e.preventDefault(); e.stopImmediatePropagation(); closeLibModal(); } diff --git a/internal/gui/tests/dialog-backdrop-drag.test.cjs b/internal/gui/tests/dialog-backdrop-drag.test.cjs new file mode 100644 index 00000000..77b2b1b7 --- /dev/null +++ b/internal/gui/tests/dialog-backdrop-drag.test.cjs @@ -0,0 +1,103 @@ +// Run with Node's test runner and Playwright on the module path; see README.md. +// #1373 (xiaozhu1337): text selected in a dialog by dragging, with the mouse +// let go outside the dialog, closed the dialog. Only a press and a release +// both on the backdrop close it: a drag out of a field, or in from the +// backdrop, leaves it open. The same holds for the provider editor and for +// the confirmation over it. No backend, the API is faked here. +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 = path.resolve(__dirname, "../assets"); + +function fixture(lang) { + const settings = { theme: "light", lang, tray: "panel", currency: "usd", textSize: 100, version: "0.1.700", redactWords: [], lanURLs: [], otel: {} }; + const relay = { id: "relay", name: "A relay whose name runs on well past the width of its own field", icon: "generic", host: "relay.test", chat: "https://relay.test/v1", + responses: "", anthropic: "", models: [{ id: "fixture-model", name: "Fixture model", on: true }], agents: [], fallback: [], headers: {}, + key: { set: true, masked: "sk-…1234" }, keyList: [], ready: true }; + const providers = { providers: [relay], presets: [], excluded: [], gateway: { running: true, window: true } }; + 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 = ${JSON.stringify({ lang, theme: "light", textSize: 100, web: true })};` }); + if (url.pathname === "/api/state") return json({ agents: [], profiles: [], settings }); + if (url.pathname === "/api/settings") return json(settings); + if (url.pathname === "/api/providers") return json(providers); + if (url.pathname === "/api/groups") return json({ groups: [], models: [] }); + if (url.pathname === "/api/usage/quotas") return json([]); + if (url.pathname === "/api/plugins") return json({ plugins: [] }); + if (url.pathname.startsWith("/api/")) return json({}); + const file = path.join(assets, url.pathname === "/" ? "index.html" : url.pathname); + try { await route.fulfill({ body: await fs.readFile(file), contentType: { ".html": "text/html", ".js": "text/javascript", ".css": "text/css", ".svg": "image/svg+xml", ".png": "image/png" }[path.extname(file)] }); } + catch { await route.fulfill({ status: 404, body: "" }); } + }; +} + +// press at one point, move in steps (a selection follows), let go at another +async function drag(page, from, to) { + await page.mouse.move(from.x, from.y); + await page.mouse.down(); + await page.mouse.move(to.x, to.y, { steps: 8 }); + await page.mouse.up(); +} +const middle = async (locator) => { const b = await locator.boundingBox(); return { x: b.x + b.width / 2, y: b.y + b.height / 2 }; }; +const out = { x: 2, y: 2 }; + +for (const engine of process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"]) { + for (const lang of ["en", "zh"]) { + for (const width of [1100, 440]) { + test(`${engine} ${lang} ${width}px: a selection dragged out of a dialog doesn't close it`, async (t) => { + const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" })); + t.after(() => browser.close()); + const page = await browser.newPage({ viewport: { width, height: 800 }, reducedMotion: "reduce" }); + page.setDefaultTimeout(5000); + const errors = []; + page.on("pageerror", (e) => errors.push(e.message)); + await page.route("**/*", fixture(lang)); + await page.goto("http://magpie.test/?view=providers"); + const row = page.locator('#providers .row.provider[data-id="relay"]'); + const modal = page.locator("#modal"); + await row.click(); + await page.getByRole("dialog").waitFor(); + const field = modal.locator("input[type=text], input:not([type])").first(); + await field.waitFor(); + const name = await field.inputValue(); + + // #1373: from inside the name field, out past the dialog, let go + const start = await field.boundingBox(); + // as the reporter does, past the field's end to bring the rest in + const beside = { x: width - 3, y: start.y + start.height / 2 }; + assert.equal(await page.evaluate(({ x, y }) => document.elementFromPoint(x, y)?.id, beside), "modal", "the drag ends over the backdrop"); + await drag(page, { x: start.x + 6, y: beside.y }, beside); + assert.ok(await field.evaluate((e) => e.selectionEnd > e.selectionStart), "the drag selected text"); + await page.waitForTimeout(300); + assert.equal(await modal.isVisible(), true, "a selection dragged out keeps the dialog"); + // a press on the backdrop let go inside the dialog is no backdrop click either + await drag(page, out, await middle(modal.locator(".dialog"))); + await page.waitForTimeout(300); + assert.equal(await modal.isVisible(), true, "a press from the backdrop let go inside keeps the dialog"); + assert.equal(await field.inputValue(), name, "the field is as it was"); + // a real click on the backdrop still closes it + await page.mouse.click(out.x, out.y); + await modal.waitFor({ state: "hidden" }); + + // the confirmation over an edited dialog keeps to the same rule + await row.click(); + const url = modal.locator('input[type="url"]').first(); + await url.fill("https://changed.test/v1"); + await page.mouse.click(out.x, out.y); + const ask = page.getByRole("alertdialog"); + await ask.waitFor(); + await drag(page, await middle(ask.locator("p")), out); + await page.waitForTimeout(300); + assert.equal(await ask.isVisible(), true, "a selection dragged out of the confirmation keeps it"); + await page.mouse.click(out.x, out.y); + await ask.waitFor({ state: "detached" }); + assert.equal(await modal.isVisible(), true, "the backdrop's click on the confirmation keeps the edit"); + assert.equal(await url.inputValue(), "https://changed.test/v1"); + assert.deepEqual(errors, []); + }); + } + } +}