From 062a2d649b92b970545d5f799abc77a23f1b3abc Mon Sep 17 00:00:00 2001 From: napalm255 Date: Sun, 27 Sep 2026 09:30:23 -0400 Subject: [PATCH 1/5] fix: refuse Shift alone with keys that editing text needs A binding whose only modifier is Shift was accepted whenever its key typed no visible character. That let Shift+Left, Shift+Home, Shift+Tab, Shift+Return and Shift+dead keys through, and grabbing any of them globally breaks text selection, focus movement or typing in every application. Shift alone is now also refused for GNOME Settings' own forbidden_keyvals (gnome-control-center, panels/keyboard/keyboard-shortcuts.c: Home, the arrows, Page_Up, Page_Down, End, Tab, KP_Enter, Return, Mode_switch), for ISO_Left_Tab, which is how GTK reports Shift+Tab, and for every dead key. Keyvals were checked under gjs with Gdk 4; the dead-key ranges come from enumerating Gdk.keyval_name over 0xfe50..0xfeff, plus the four Gdk.KEY_dead_* constants at 0xfe90..0xfe93 that keyval_name cannot name. Behavior change: the existing test that accepted Shift+Tab now expects it refused, and Shift+Return, which the old rule also accepted, is refused too. The docs' Keyboard section says which keys Shift alone cannot take. --- docs/index.html | 14 +++++--- modules/shortcuts.js | 77 ++++++++++++++++++++++++++++++++++++++--- tests/shortcuts.test.js | 59 +++++++++++++++++++++++++++---- 3 files changed, 134 insertions(+), 16 deletions(-) diff --git a/docs/index.html b/docs/index.html index cebdfa7..4191cf4 100644 --- a/docs/index.html +++ b/docs/index.html @@ -1181,11 +1181,15 @@

Keyboard

A bare key is not accepted: it would be taken from every application. Shift alone is accepted only - with a key that types nothing, such as - Shift+F5; - Shift+A is how a - capital A is typed. The shortcut does nothing on the lock screen - or the login screen. + with a key that types nothing and that editing text does not + need, such as Shift+F5. Shift+A is how + a capital A is typed, and Shift with an + arrow key, Home, End, Page Up, Page Down, Tab, Enter or a dead + key selects text, moves focus, ends a line or types an accented + letter: the keys GNOME Settings refuses, plus dead keys. The + shortcut does nothing on the lock screen or the login screen.

It is stored under quickts-open-menu. The prefix diff --git a/modules/shortcuts.js b/modules/shortcuts.js index 63db194..52f6f6f 100644 --- a/modules/shortcuts.js +++ b/modules/shortcuts.js @@ -23,6 +23,59 @@ export const CAPTURE_IGNORE = 'ignore'; /** Bind the combination and close. */ export const CAPTURE_ASSIGN = 'assign'; +/** + * Keys that Shift alone may not be bound to, although none of them types a + * visible character: Shift with one of them selects text, moves focus or + * ends a line in every application. + * + * GNOME Settings' own list, forbidden_keyvals in is_valid_binding() + * (gnome-control-center, panels/keyboard/keyboard-shortcuts.c), plus + * ISO_Left_Tab, which is what GTK reports for Shift+Tab. Values are + * Gdk.KEY_* under gjs with Gdk 4. + */ +const SHIFT_FORBIDDEN_KEYVALS = new Set([ + 0xff50, // Home + 0xff51, // Left + 0xff52, // Up + 0xff53, // Right + 0xff54, // Down + 0xff55, // Page_Up + 0xff56, // Page_Down + 0xff57, // End + 0xff09, // Tab + 0xfe20, // ISO_Left_Tab + 0xff8d, // KP_Enter + 0xff0d, // Return + 0xff7e, // Mode_switch +]); + +/** + * The dead keys, as inclusive keyval ranges. + * + * Derived under gjs with Gdk 4.22: Gdk.keyval_name(k) for every k in + * 0xfe50..0xfeff names dead_grave..dead_currency at 0xfe50-0xfe6f and + * dead_a..dead_hamza at 0xfe80-0xfe8d, and nothing else starting with dead_. + * The Gdk.KEY_dead_* constants add dead_lowline..dead_longsolidusoverlay at + * 0xfe90-0xfe93, which keyval_name cannot name (it returns "0xfe90"), so they + * are listed too. + */ +const DEAD_KEY_RANGES = [ + [0xfe50, 0xfe6f], + [0xfe80, 0xfe8d], + [0xfe90, 0xfe93], +]; + +/** + * Whether a key is a dead key, which types nothing itself but puts an accent + * on the next letter typed. + * + * @param {number} keyval Key value. + * @returns {boolean} True for a dead key. + */ +function isDeadKey(keyval) { + return DEAD_KEY_RANGES.some(([first, last]) => keyval >= first && keyval <= last); +} + /** * Whether a key's own code point types something visible. * @@ -39,10 +92,20 @@ function typesVisibly(codePoint) { } /** - * Whether a captured combination may be bound as a global shortcut. + * Whether a captured key combination may be bound as a global shortcut. + * + * A bare key would steal it from every application, so it never may. Shift + * alone may only with a key that types no visible character and is not one + * that editing text needs (SHIFT_FORBIDDEN_KEYVALS) or a dead key: Shift+F5 + * may, Shift+A, Shift+Left and Shift+dead_acute may not. Any other modifier + * makes a combination bindable, subject to Gtk's own accelerator check. * - * A bare key would steal it from every application. Shift alone is bindable - * only when the key types nothing on its own, close to GNOME Settings' rule. + * Built on GNOME Settings' is_valid_binding() (gnome-control-center, + * panels/keyboard/keyboard-shortcuts.c), and differs in three ways: this + * refuses every bare key, where GNOME allows one such as F5; it refuses + * Shift with a dead key, which GNOME's list leaves out; and it judges what + * Shift alone types by whether the key's code point is a visible character, + * where GNOME checks per-script keyval ranges. * * @param {number} mask Modifier mask, already reduced to the default mod mask. * @param {number} keyval Key value. @@ -57,7 +120,13 @@ export function isValidBinding( { shiftMask, acceleratorValid, codePoint }, ) { if (mask === 0) return false; - if (mask === shiftMask && typesVisibly(codePoint)) return false; + if ( + mask === shiftMask && + (typesVisibly(codePoint) || + SHIFT_FORBIDDEN_KEYVALS.has(keyval) || + isDeadKey(keyval)) + ) + return false; return acceleratorValid(keyval, mask); } diff --git a/tests/shortcuts.test.js b/tests/shortcuts.test.js index 50ae92c..230650c 100644 --- a/tests/shortcuts.test.js +++ b/tests/shortcuts.test.js @@ -12,10 +12,12 @@ import { // Stand-ins for the Gdk and Gtk values prefs.js passes in. const SHIFT = 1; const CONTROL = 4; +const SUPER = 0x4000000; const ESCAPE = 0xff1b; const BACKSPACE = 0xff08; const F5 = 0xffc2; const TAB = 0xff09; +const LEFT = 0xff51; const gtk = { escapeKey: ESCAPE, @@ -43,14 +45,57 @@ describe('isValidBinding', () => { expect(isValidBinding(mask, keyval, { ...gtk, codePoint })).toBe(false); }); - // Close to GNOME Settings' rule: Shift alone is enough for a key that types - // nothing on its own — a function key has no code point, and Tab's is a - // control character. + // Shift alone is enough for a key that types nothing on its own and that + // editing text does not need: a function key has no code point at all. + it('accepts Shift+F5', () => { + expect(isValidBinding(SHIFT, F5, { ...gtk, codePoint: 0 })).toBe(true); + }); + + // Shift with these selects text, moves focus or ends a line in every + // application, though none of them types a visible character. Keyvals and + // code points as Gdk 4 gives them (Gdk.KEY_*, Gdk.keyval_to_unicode) under + // gjs; ISO_Left_Tab is what GTK reports for Shift+Tab, and dead_acute is a + // dead key, which types the accent over the next letter. + it.each([ + ['Left', LEFT, 0], + ['Up', 0xff52, 0], + ['Right', 0xff53, 0], + ['Down', 0xff54, 0], + ['Home', 0xff50, 0], + ['End', 0xff57, 0], + ['Page_Up', 0xff55, 0], + ['Page_Down', 0xff56, 0], + ['Tab', TAB, 0x09], + ['ISO_Left_Tab', 0xfe20, 0], + ['Return', 0xff0d, 0x0d], + ['KP_Enter', 0xff8d, 0], + ['Mode_switch', 0xff7e, 0], + ['dead_acute', 0xfe51, 0], + ])( + 'rejects Shift+%s, which applications need for editing text', + (_name, keyval, codePoint) => { + expect(isValidBinding(SHIFT, keyval, { ...gtk, codePoint })).toBe(false); + }, + ); + + // The ends of each run of dead keys. + it.each([ + ['dead_grave', 0xfe50], + ['dead_currency', 0xfe6f], + ['dead_a', 0xfe80], + ['dead_hamza', 0xfe8d], + ['dead_lowline', 0xfe90], + ['dead_longsolidusoverlay', 0xfe93], + ])('rejects Shift+%s, a dead key', (_name, keyval) => { + expect(isValidBinding(SHIFT, keyval, { ...gtk, codePoint: 0 })).toBe(false); + }); + + // The same keys stay bindable with a modifier other than Shift. it.each([ - ['Shift+F5', F5, 0], - ['Shift+Tab', TAB, 0x09], - ])('accepts %s', (_reason, keyval, codePoint) => { - expect(isValidBinding(SHIFT, keyval, { ...gtk, codePoint })).toBe(true); + ['Ctrl+Left', CONTROL, LEFT, 0], + ['Super+Tab', SUPER, TAB, 0x09], + ])('accepts %s', (_reason, mask, keyval, codePoint) => { + expect(isValidBinding(mask, keyval, { ...gtk, codePoint })).toBe(true); }); it('defers to Gtk on what is a valid accelerator', () => { From 0a7f8ce85e9cace8732e37a4f2787a73790dab17 Mon Sep 17 00:00:00 2001 From: napalm255 Date: Sun, 27 Sep 2026 09:31:52 -0400 Subject: [PATCH 2/5] chore(template): give the GLib stub a VariantType Copy the canonical tests/stubs/gi-glib.js, which now carries a VariantType, byte for byte as in the other quick* extensions, and regenerate template.sha256. --- template.sha256 | 2 +- tests/stubs/gi-glib.js | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/template.sha256 b/template.sha256 index 536f7a0..8f92fd9 100644 --- a/template.sha256 +++ b/template.sha256 @@ -16,7 +16,7 @@ cbde6c3009a0b09910a20dc4bcd17bdf19c9174eee6d74c67c825efbfd9c17ca mise.toml 508b2aabe2c485cc9cdaf51ca7dd1a70e53c70c65c7dd0149dc1458a4f78a679 scripts/template-check.sh f0a3265f966c1017a80bcba09bff0c465ccec94b7ad9dd6456742f94905b3ae2 template.list f13c2fca51ef7a2db1755805d5f24a4a55c97605d40610168f1d39d12ad408af tests/docs.spec.js -450e8d7c616dfaeb3ad697cd6d457444aa9cd300fb0261da763a7f5d9499bd0f tests/stubs/gi-glib.js +8b3467177303c86b68bea7532c3bcec8585f5d66d0b7f1d604d22ef5790bea15 tests/stubs/gi-glib.js 3969a5d8b38c607f1f70be1e6e88ce3ff75749710e1591437b511a5fff300df1 tests/stubs/gi-gobject.js dbebfe1620af3038a86d9454c57778ee4a5885f07748e6acb3f83aa11353dc86 tests/stubs/gi-meta.js 757435b7d2469254f94210744c243fd0dc49983800f2f227fd90c7eb413e385a tests/stubs/gi-pango.js diff --git a/tests/stubs/gi-glib.js b/tests/stubs/gi-glib.js index 9f01469..8ce0971 100644 --- a/tests/stubs/gi-glib.js +++ b/tests/stubs/gi-glib.js @@ -68,4 +68,12 @@ export default { // pass unless the test injects its own clock. get_monotonic_time: () => 42_000_000, uuid_string_random: () => '00000000-0000-4000-8000-000000000000', + + // Only the signature is kept: a caller builds one to pass to a D-Bus call, + // and no fake reads it back. + VariantType: class VariantType { + constructor(signature) { + this.signature = signature; + } + }, }; From f3b124d4245563dc9d48b79209c3a8582131d195 Mon Sep 17 00:00:00 2001 From: napalm255 Date: Sun, 27 Sep 2026 09:34:20 -0400 Subject: [PATCH 3/5] fix: keep a saved Taildrop file when the daemon's DELETE fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit saveFile wrote the file and then asked tailscaled to delete it from its inbox. When that DELETE threw, the catch discarded the path already written and the row showed an error, so a retry saved a duplicate "name (1)". A failed DELETE after a successful save now returns { path, error: '' }, so the row says "Saved to …" as for any save. The model remembers the file, with the size it was listed at: waitingFiles leaves it out of the list, so the next menu open does not offer to save it again, and asks the daemon to delete it again. A file no longer listed at that size is forgotten, so a different file that later arrives under the same name is listed and never deleted unsaved. The fake daemon can now fail one method on a path ('DELETE /…'), which the new tests use. --- docs/index.html | 5 ++- modules/model.js | 71 ++++++++++++++++++++++++++--- tests/model.test.js | 82 ++++++++++++++++++++++++++++++++++ tests/support/daemon.js | 11 +++-- tests/taildrop-section.test.js | 36 +++++++++++++++ 5 files changed, 196 insertions(+), 9 deletions(-) diff --git a/docs/index.html b/docs/index.html index 4191cf4..3c9b15f 100644 --- a/docs/index.html +++ b/docs/index.html @@ -1793,7 +1793,10 @@

The LocalAPI

Receiving, streamed to disk. The file is written - before the daemon is told to forget it. + before the daemon is told to forget it; if the + daemon will not, the file still counts as saved, is + not offered again, and the delete is retried the + next time the menu opens. diff --git a/modules/model.js b/modules/model.js index b1473dc..efb87ba 100644 --- a/modules/model.js +++ b/modules/model.js @@ -95,6 +95,17 @@ export class TailscaleModel { #peersReadAt = 0; #menuOpen = false; + /** Each waiting file's size as last listed, by name. */ + #listedSizes = new Map(); + + /** + * Files saved here that tailscaled then would not delete, by name, with + * the size they were listed at. Hidden from waitingFiles, which asks the + * daemon to delete them again, so a saved file is never offered to save a + * second time. + */ + #savedNotDeleted = new Map(); + /** * @param {object} options Options. * @param {object} options.client Transport, from modules/io.js. @@ -306,13 +317,48 @@ export class TailscaleModel { async waitingFiles() { if (this.#disposed || !isUp(this.#state)) return []; + let files; try { - return waitingFiles(await this.#request(waitingFilesRequest())); + files = waitingFiles(await this.#request(waitingFilesRequest())); } catch (error) { if (!isCanceled(error)) console.debug(`[quickts] could not list waiting files: ${error}`); return []; } + + this.#listedSizes = new Map(files.map(({ name, size }) => [name, size])); + await this.#retryDeletes(); + + return files.filter(({ name }) => !this.#savedNotDeleted.has(name)); + } + + /** + * Ask tailscaled again to delete each file already saved here that is + * still listed. + * + * One no longer listed at the size it was saved at is dropped first: it + * is gone, and a file listed under that name now is a different one, not + * yet saved, that must be neither hidden nor deleted. + * + * @returns {Promise} Done; a refusal leaves the file for next time. + */ + async #retryDeletes() { + for (const [name, size] of this.#savedNotDeleted) { + if (this.#listedSizes.get(name) !== size) { + this.#savedNotDeleted.delete(name); + continue; + } + + try { + await this.#request(deleteFileRequest(name)); + this.#savedNotDeleted.delete(name); + } catch (error) { + if (!isCanceled(error)) + console.debug( + `[quickts] could not remove a saved file from Taildrop: ${reasonOf(error)}`, + ); + } + } } /** @@ -320,6 +366,11 @@ export class TailscaleModel { * * In that order. Deleting first loses the file if the write fails. * + * Once the file is written, a delete that fails does not undo that: the + * path is still returned, with no error, because the file is saved. The + * file stays in tailscaled's inbox, and waitingFiles hides it and asks + * again, so it is not offered, and saved as a duplicate, a second time. + * * @param {string} name The name as the daemon lists it. * @returns {Promise<{path: string, error: string}>} Where it went, or a * REASON from modules/errors.js if it did not — untranslated, so @@ -333,16 +384,26 @@ export class TailscaleModel { // left on the daemon, where `tailscale file get` can still reach it. if (!isSafeFileName(name)) return { path: '', error: REASON.PROTOCOL }; + let path; try { - const path = await this.#client.saveFile(getFileRequest(name), name); - await this.#request(deleteFileRequest(name)); - - return { path, error: '' }; + path = await this.#client.saveFile(getFileRequest(name), name); } catch (error) { if (isCanceled(error)) return { path: '', error: '' }; return { path: '', error: reasonOf(error) }; } + + try { + await this.#request(deleteFileRequest(name)); + } catch (error) { + this.#savedNotDeleted.set(name, this.#listedSizes.get(name)); + if (!isCanceled(error)) + console.debug( + `[quickts] saved a file but could not remove it from Taildrop: ${reasonOf(error)}`, + ); + } + + return { path, error: '' }; } /** diff --git a/tests/model.test.js b/tests/model.test.js index eaff084..7a69847 100644 --- a/tests/model.test.js +++ b/tests/model.test.js @@ -1082,6 +1082,88 @@ describe('waiting files', () => { expect(daemon.deleted.at(-1)).toContain('a.txt'); }); + // The file is already written by then. Reporting the failure as an error + // threw that path away, and saving again made a duplicate "a (1).txt". + it('keeps the saved path when the daemon will not forget the file', async () => { + const { model, daemon } = setup(); + daemon.responses.files = [{ Name: 'a.txt', Size: 4 }]; + await model.start(); + daemon.failures.set( + 'DELETE /localapi/v0/files/a.txt', + new TransportError(REASON.HTTP, '500'), + ); + + const result = await model.saveFile('a.txt'); + + expect(result).toEqual({ path: '/home/someone/Downloads/a.txt', error: '' }); + expect(daemon.saved).toHaveLength(1); + }); + + describe('a file saved that the daemon would not forget', () => { + const savedButKept = async () => { + const { model, daemon } = setup(); + daemon.responses.files = [{ Name: 'a.txt', Size: 4 }]; + await model.start(); + await model.waitingFiles(); + daemon.failures.set( + 'DELETE /localapi/v0/files/a.txt', + new TransportError(REASON.HTTP, '500'), + ); + await model.saveFile('a.txt'); + daemon.reset(); + + return { model, daemon }; + }; + + // Listed, it would be offered to save again, and a second save is a + // duplicate. + it('is not listed as waiting again', async () => { + const { model, daemon } = await savedButKept(); + + expect(await model.waitingFiles()).toEqual([]); + expect(daemon.saved).toHaveLength(1); + }); + + it('is not saved a second time', async () => { + const { model, daemon } = await savedButKept(); + daemon.failures.clear(); + + await model.waitingFiles(); + await model.waitingFiles(); + + expect(daemon.saved).toHaveLength(1); + }); + + it('is forgotten on the next listing once the daemon allows it', async () => { + const { model, daemon } = await savedButKept(); + daemon.failures.clear(); + + await model.waitingFiles(); + + expect(daemon.deleted).toEqual(['/localapi/v0/files/a.txt']); + }); + + it('stays hidden while the daemon still refuses', async () => { + const { model, daemon } = await savedButKept(); + + expect(await model.waitingFiles()).toEqual([]); + expect(await model.waitingFiles()).toEqual([]); + expect(daemon.deleted).toEqual([]); + }); + + // A different file under the same name, sent after the first one was + // removed some other way: it has not been saved, so it is listed and + // never deleted unsaved. + it('does not hide a new file that arrives under the same name', async () => { + const { model, daemon } = await savedButKept(); + daemon.failures.clear(); + daemon.responses.files = [{ Name: 'a.txt', Size: 99 }]; + + expect(await model.waitingFiles()).toEqual([{ name: 'a.txt', size: 99 }]); + expect(daemon.deleted).toEqual([]); + }); + }); + // tailscaled validates names on the way in; this is where one becomes a // path here, so it is checked again rather than trusted. it.each(['../escape.txt', '.bashrc', 'sub/dir.txt'])( diff --git a/tests/support/daemon.js b/tests/support/daemon.js index 7f82db1..d051dc8 100644 --- a/tests/support/daemon.js +++ b/tests/support/daemon.js @@ -87,7 +87,11 @@ export function createDaemon(seed = {}) { const paths = []; /** Bodies of every PATCH, in order. */ const patches = []; - /** Paths the daemon should reject, mapped to the error to throw. */ + /** + * Paths the daemon should reject, mapped to the error to throw. A key may + * also start with a method, as in 'DELETE /localapi/v0/files/a.txt', to + * reject only that method's requests. + */ const failures = new Map(); /** Files the daemon was told to forget, in order. */ const deleted = []; @@ -101,8 +105,9 @@ export function createDaemon(seed = {}) { paths.push(path); if (method === 'PATCH') patches.push(body); - const failure = [...failures.entries()].find(([prefix]) => - path.startsWith(prefix), + const failure = [...failures.entries()].find( + ([prefix]) => + path.startsWith(prefix) || `${method} ${path}`.startsWith(prefix), ); if (failure) throw failure[1]; diff --git a/tests/taildrop-section.test.js b/tests/taildrop-section.test.js index 95f898d..d4c85e9 100644 --- a/tests/taildrop-section.test.js +++ b/tests/taildrop-section.test.js @@ -119,6 +119,42 @@ describe('received files', () => { expect(daemon.saved).toHaveLength(1); }); + // Saved, but tailscaled would not remove it from its inbox. The file is on + // disk, so the row says so; offering it again on the next open would save + // a duplicate "report (1).pdf". + it('shows a file saved when the daemon will not forget it, and does not offer it again', async () => { + const { panel, model, daemon } = setup(); + withFiles(daemon); + panel.enable(); + await model.start(); + await settle(); + toggleOf().menu.open(); + await settle(); + + daemon.failures.set('DELETE /localapi/v0/files/report.pdf', { + name: 'TransportError', + reason: REASON.HTTP, + }); + + const row = toggleOf()._inbox.menu.items.at(0); + row.activate(); + await settle(); + + expect(row.text).toBe('Saved to /home/someone/Downloads/report.pdf'); + expect(row.sensitive).toBe(false); + expect(Main.osdMessages.at(-1).label).toBe('Saved report.pdf'); + + toggleOf().menu.close(); + toggleOf().menu.open(); + await settle(); + + const rows = toggleOf()._inbox.menu.items.map(item => item.text); + expect(rows).toHaveLength(1); + expect(rows[0]).toContain('notes.txt'); + expect(toggleOf()._inbox.label.text).toBe('1 received file'); + expect(daemon.saved).toHaveLength(1); + }); + it('does not forget a file it could not save', async () => { const { panel, model, daemon } = setup(); withFiles(daemon); From ea125cc489201a603070489452babfe67e1d22dc Mon Sep 17 00:00:00 2001 From: napalm255 Date: Sun, 27 Sep 2026 09:47:50 -0400 Subject: [PATCH 4/5] test: cover the control-character half of the Shift rule with bindable keys The case that accepted Super+Tab passed only because acceleratorValid is stubbed: Gtk.accelerator_valid refuses Tab with any modifier. Use Super+Left and Ctrl+Return instead, which GTK accepts. Nothing covered the \p{Cc} half of the visible-character check once Shift+Tab and Shift+Return became refusals: reducing it to codePoint > 0 passed every suite. Shift+Delete (keyval 0xffff, code point 0x7f, both checked under gjs) is accepted, and fails under that reduction. --- tests/shortcuts.test.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/shortcuts.test.js b/tests/shortcuts.test.js index 230650c..b94b17d 100644 --- a/tests/shortcuts.test.js +++ b/tests/shortcuts.test.js @@ -51,6 +51,12 @@ describe('isValidBinding', () => { expect(isValidBinding(SHIFT, F5, { ...gtk, codePoint: 0 })).toBe(true); }); + // Delete has a code point (0x7f), unlike F5, but a control character, so + // this exercises the \p{Cc} half of the rule rather than codePoint <= 0. + it('accepts Shift+Delete, since Delete types a control character', () => { + expect(isValidBinding(SHIFT, 0xffff, { ...gtk, codePoint: 0x7f })).toBe(true); + }); + // Shift with these selects text, moves focus or ends a line in every // application, though none of them types a visible character. Keyvals and // code points as Gdk 4 gives them (Gdk.KEY_*, Gdk.keyval_to_unicode) under @@ -90,10 +96,13 @@ describe('isValidBinding', () => { expect(isValidBinding(SHIFT, keyval, { ...gtk, codePoint: 0 })).toBe(false); }); - // The same keys stay bindable with a modifier other than Shift. + // The same keys stay bindable with a modifier other than Shift. Not Tab: + // Gtk.accelerator_valid refuses Tab with any modifier, so a Tab case here + // would pass only because acceleratorValid is stubbed. it.each([ ['Ctrl+Left', CONTROL, LEFT, 0], - ['Super+Tab', SUPER, TAB, 0x09], + ['Super+Left', SUPER, LEFT, 0], + ['Ctrl+Return', CONTROL, 0xff0d, 0x0d], ])('accepts %s', (_reason, mask, keyval, codePoint) => { expect(isValidBinding(mask, keyval, { ...gtk, codePoint })).toBe(true); }); From e803c0ab01071cb9297114cfb0ebaa036c673579 Mon Sep 17 00:00:00 2001 From: napalm255 Date: Sun, 27 Sep 2026 09:49:41 -0400 Subject: [PATCH 5/5] fix: never delete a received file QuickTS has not saved The previous commit retried a failed DELETE on each listing, for a file listed under the name and size of one already saved. A name and a size are not identity: if the saved file left the inbox another way and a new file of the same name and size arrived, that new file was deleted unsaved. A successful retry also still returned the deleted file, and a save made with no listing before it recorded an undefined size that an unlisted name matched. Nothing now deletes a file on its own. saveFile(name, size) takes the size from the row that was clicked, and on a failed DELETE after a successful save it returns the saved path, recording name and size only when the size is a number. waitingFiles sends no request of its own: it forgets any record no longer listed at exactly that size, and hides only a listed file whose name and size both match. The file stays in Tailscale's inbox, where `tailscale file get` can clear it, and after a reload it can be offered again; the docs say so. --- docs/index.html | 10 +++-- modules/model.js | 65 ++++++++++----------------- modules/taildrop-section.js | 2 +- tests/model.test.js | 81 ++++++++++++++++++++-------------- tests/taildrop-section.test.js | 5 +++ 5 files changed, 84 insertions(+), 79 deletions(-) diff --git a/docs/index.html b/docs/index.html index 3c9b15f..473e004 100644 --- a/docs/index.html +++ b/docs/index.html @@ -1793,10 +1793,12 @@

The LocalAPI

Receiving, streamed to disk. The file is written - before the daemon is told to forget it; if the - daemon will not, the file still counts as saved, is - not offered again, and the delete is retried the - next time the menu opens. + before the daemon is told to forget it. If the + daemon will not, the file still counts as saved and + is not offered again while QuickTS runs. It stays in + Tailscale’s inbox, where + tailscale file get can clear it, and + after a reload it can be offered again. diff --git a/modules/model.js b/modules/model.js index efb87ba..6fe6787 100644 --- a/modules/model.js +++ b/modules/model.js @@ -95,14 +95,12 @@ export class TailscaleModel { #peersReadAt = 0; #menuOpen = false; - /** Each waiting file's size as last listed, by name. */ - #listedSizes = new Map(); - /** * Files saved here that tailscaled then would not delete, by name, with - * the size they were listed at. Hidden from waitingFiles, which asks the - * daemon to delete them again, so a saved file is never offered to save a - * second time. + * the size they were listed at. waitingFiles hides them, so a saved file + * is not offered, and saved as a duplicate, a second time. Nothing here + * deletes them: a name and a size are not proof that the file listed + * later is the one that was saved. */ #savedNotDeleted = new Map(); @@ -326,39 +324,21 @@ export class TailscaleModel { return []; } - this.#listedSizes = new Map(files.map(({ name, size }) => [name, size])); - await this.#retryDeletes(); - - return files.filter(({ name }) => !this.#savedNotDeleted.has(name)); - } - - /** - * Ask tailscaled again to delete each file already saved here that is - * still listed. - * - * One no longer listed at the size it was saved at is dropped first: it - * is gone, and a file listed under that name now is a different one, not - * yet saved, that must be neither hidden nor deleted. - * - * @returns {Promise} Done; a refusal leaves the file for next time. - */ - async #retryDeletes() { - for (const [name, size] of this.#savedNotDeleted) { - if (this.#listedSizes.get(name) !== size) { + // Forget a saved file no longer listed at the size it was saved at: it + // has left the inbox, and a file listed under that name now is a + // different one, not yet saved. + const listed = new Map(files.map(({ name, size }) => [name, size])); + for (const [name, size] of this.#savedNotDeleted) + if (!listed.has(name) || listed.get(name) !== size) this.#savedNotDeleted.delete(name); - continue; - } - try { - await this.#request(deleteFileRequest(name)); - this.#savedNotDeleted.delete(name); - } catch (error) { - if (!isCanceled(error)) - console.debug( - `[quickts] could not remove a saved file from Taildrop: ${reasonOf(error)}`, - ); - } - } + return files.filter( + ({ name, size }) => + !( + this.#savedNotDeleted.has(name) && + this.#savedNotDeleted.get(name) === size + ), + ); } /** @@ -368,16 +348,19 @@ export class TailscaleModel { * * Once the file is written, a delete that fails does not undo that: the * path is still returned, with no error, because the file is saved. The - * file stays in tailscaled's inbox, and waitingFiles hides it and asks - * again, so it is not offered, and saved as a duplicate, a second time. + * file stays in tailscaled's inbox, where `tailscale file get` can clear + * it; while this model lives, waitingFiles hides it, so it is not offered, + * and saved as a duplicate, a second time. Only a numeric size is + * recorded: without one there is nothing but the name to match. * * @param {string} name The name as the daemon lists it. + * @param {number} size Its size as the daemon lists it. * @returns {Promise<{path: string, error: string}>} Where it went, or a * REASON from modules/errors.js if it did not — untranslated, so * modules/taildrop-section.js can turn it into a literal `_()` call * rather than being handed English composed at run time. */ - async saveFile(name) { + async saveFile(name, size) { if (this.#disposed) return { path: '', error: '' }; // The daemon listed a name that cannot be a plain file here. It is @@ -396,7 +379,7 @@ export class TailscaleModel { try { await this.#request(deleteFileRequest(name)); } catch (error) { - this.#savedNotDeleted.set(name, this.#listedSizes.get(name)); + if (Number.isFinite(size)) this.#savedNotDeleted.set(name, size); if (!isCanceled(error)) console.debug( `[quickts] saved a file but could not remove it from Taildrop: ${reasonOf(error)}`, diff --git a/modules/taildrop-section.js b/modules/taildrop-section.js index df4fc4f..c6ae06d 100644 --- a/modules/taildrop-section.js +++ b/modules/taildrop-section.js @@ -286,7 +286,7 @@ export class InboxSection { row.label.text = fill(_('Saving %s…'), file.name); row.setSensitive(false); - const { path, error } = await this._model.saveFile(file.name); + const { path, error } = await this._model.saveFile(file.name, file.size); if (generation !== this._generation) return; if (error) { diff --git a/tests/model.test.js b/tests/model.test.js index 7a69847..08c9132 100644 --- a/tests/model.test.js +++ b/tests/model.test.js @@ -1093,14 +1093,16 @@ describe('waiting files', () => { new TransportError(REASON.HTTP, '500'), ); - const result = await model.saveFile('a.txt'); + const result = await model.saveFile('a.txt', 4); expect(result).toEqual({ path: '/home/someone/Downloads/a.txt', error: '' }); expect(daemon.saved).toHaveLength(1); }); + // Nothing here ever deletes a file on its own: a name and a size are not + // proof that the file listed now is the one that was saved. describe('a file saved that the daemon would not forget', () => { - const savedButKept = async () => { + const savedButKept = async size => { const { model, daemon } = setup(); daemon.responses.files = [{ Name: 'a.txt', Size: 4 }]; await model.start(); @@ -1109,58 +1111,71 @@ describe('waiting files', () => { 'DELETE /localapi/v0/files/a.txt', new TransportError(REASON.HTTP, '500'), ); - await model.saveFile('a.txt'); + await model.saveFile('a.txt', size); + // Were a DELETE sent now, it would succeed. + daemon.failures.clear(); daemon.reset(); return { model, daemon }; }; + // Only GET /files/ may be asked for: the file itself never again. + const deleteRequests = daemon => daemon.pathsMatching('/files/a.txt'); + // Listed, it would be offered to save again, and a second save is a // duplicate. - it('is not listed as waiting again', async () => { - const { model, daemon } = await savedButKept(); + it('is not listed again, and no later listing deletes it', async () => { + const { model, daemon } = await savedButKept(4); expect(await model.waitingFiles()).toEqual([]); - expect(daemon.saved).toHaveLength(1); - }); - - it('is not saved a second time', async () => { - const { model, daemon } = await savedButKept(); - daemon.failures.clear(); - - await model.waitingFiles(); - await model.waitingFiles(); - - expect(daemon.saved).toHaveLength(1); + expect(await model.waitingFiles()).toEqual([]); + expect(deleteRequests(daemon)).toEqual([]); + expect(daemon.deleted).toEqual([]); }); - it('is forgotten on the next listing once the daemon allows it', async () => { - const { model, daemon } = await savedButKept(); - daemon.failures.clear(); - - await model.waitingFiles(); + // The original left the inbox some other way (`tailscale file get`, or + // a reply lost in a restart), and a new file of the same name and size + // arrived before the next listing. It cannot be told apart, so it is + // hidden — but it stays in Tailscale's inbox, never deleted unsaved. + it('hides, but never deletes, a same-name same-size file that replaced it', async () => { + const { model, daemon } = await savedButKept(4); + daemon.responses.files = [{ Name: 'a.txt', Size: 4 }]; - expect(daemon.deleted).toEqual(['/localapi/v0/files/a.txt']); + expect(await model.waitingFiles()).toEqual([]); + expect(deleteRequests(daemon)).toEqual([]); + expect(daemon.deleted).toEqual([]); }); - it('stays hidden while the daemon still refuses', async () => { - const { model, daemon } = await savedButKept(); + it('is forgotten once no longer listed, so a later file of that name is shown', async () => { + const { model, daemon } = await savedButKept(4); + daemon.responses.files = []; expect(await model.waitingFiles()).toEqual([]); - expect(await model.waitingFiles()).toEqual([]); - expect(daemon.deleted).toEqual([]); + + daemon.responses.files = [{ Name: 'a.txt', Size: 4 }]; + expect(await model.waitingFiles()).toEqual([{ name: 'a.txt', size: 4 }]); + expect(deleteRequests(daemon)).toEqual([]); }); - // A different file under the same name, sent after the first one was - // removed some other way: it has not been saved, so it is listed and - // never deleted unsaved. - it('does not hide a new file that arrives under the same name', async () => { - const { model, daemon } = await savedButKept(); - daemon.failures.clear(); + it('does not hide a file of that name listed at another size', async () => { + const { model, daemon } = await savedButKept(4); daemon.responses.files = [{ Name: 'a.txt', Size: 99 }]; expect(await model.waitingFiles()).toEqual([{ name: 'a.txt', size: 99 }]); - expect(daemon.deleted).toEqual([]); + expect(deleteRequests(daemon)).toEqual([]); + }); + + // With no size to match, a later listing could only be matched on the + // name, which is not enough to hide anything by. + it.each([ + ['no size', undefined], + ['a string', '4'], + ['NaN', Number.NaN], + ])('records nothing when saved with %s', async (_reason, size) => { + const { model, daemon } = await savedButKept(size); + + expect(await model.waitingFiles()).toEqual([{ name: 'a.txt', size: 4 }]); + expect(deleteRequests(daemon)).toEqual([]); }); }); diff --git a/tests/taildrop-section.test.js b/tests/taildrop-section.test.js index d4c85e9..607808a 100644 --- a/tests/taildrop-section.test.js +++ b/tests/taildrop-section.test.js @@ -144,6 +144,8 @@ describe('received files', () => { expect(row.sensitive).toBe(false); expect(Main.osdMessages.at(-1).label).toBe('Saved report.pdf'); + daemon.failures.clear(); + daemon.reset(); toggleOf().menu.close(); toggleOf().menu.open(); await settle(); @@ -153,6 +155,9 @@ describe('received files', () => { expect(rows[0]).toContain('notes.txt'); expect(toggleOf()._inbox.label.text).toBe('1 received file'); expect(daemon.saved).toHaveLength(1); + // Hidden, not deleted: it stays in Tailscale's inbox. + expect(daemon.pathsMatching('report.pdf')).toEqual([]); + expect(daemon.deleted).toEqual([]); }); it('does not forget a file it could not save', async () => {