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 <dialog> (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.
This commit is contained in:
1 parent
be2037285a
commit
457119fec3
3 files changed
+125
-5
No files matched your search
@@ -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(); }
|
||||
|
||||
@@ -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(); }
|
||||
|
||||
@@ -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, []);
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user