diff --git a/docs/index.html b/docs/index.html
index cebdfa7..473e004 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
@@ -1789,7 +1793,12 @@
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 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 b1473dc..6fe6787 100644
--- a/modules/model.js
+++ b/modules/model.js
@@ -95,6 +95,15 @@ export class TailscaleModel {
#peersReadAt = 0;
#menuOpen = false;
+ /**
+ * Files saved here that tailscaled then would not delete, by name, with
+ * 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();
+
/**
* @param {object} options Options.
* @param {object} options.client Transport, from modules/io.js.
@@ -306,13 +315,30 @@ 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 [];
}
+
+ // 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);
+
+ return files.filter(
+ ({ name, size }) =>
+ !(
+ this.#savedNotDeleted.has(name) &&
+ this.#savedNotDeleted.get(name) === size
+ ),
+ );
}
/**
@@ -320,29 +346,47 @@ 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, 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
// 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) {
+ 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)}`,
+ );
+ }
+
+ return { path, error: '' };
}
/**
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/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/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/model.test.js b/tests/model.test.js
index eaff084..08c9132 100644
--- a/tests/model.test.js
+++ b/tests/model.test.js
@@ -1082,6 +1082,103 @@ 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', 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 size => {
+ 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', 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 again, and no later listing deletes it', async () => {
+ const { model, daemon } = await savedButKept(4);
+
+ expect(await model.waitingFiles()).toEqual([]);
+ expect(await model.waitingFiles()).toEqual([]);
+ expect(deleteRequests(daemon)).toEqual([]);
+ expect(daemon.deleted).toEqual([]);
+ });
+
+ // 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(await model.waitingFiles()).toEqual([]);
+ expect(deleteRequests(daemon)).toEqual([]);
+ expect(daemon.deleted).toEqual([]);
+ });
+
+ 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([]);
+
+ daemon.responses.files = [{ Name: 'a.txt', Size: 4 }];
+ expect(await model.waitingFiles()).toEqual([{ name: 'a.txt', size: 4 }]);
+ expect(deleteRequests(daemon)).toEqual([]);
+ });
+
+ 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(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([]);
+ });
+ });
+
// 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/shortcuts.test.js b/tests/shortcuts.test.js
index 50ae92c..b94b17d 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,66 @@ 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);
+ });
+
+ // 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
+ // 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. Not Tab:
+ // Gtk.accelerator_valid refuses Tab with any modifier, so a Tab case here
+ // would pass only because acceleratorValid is stubbed.
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+Left', SUPER, LEFT, 0],
+ ['Ctrl+Return', CONTROL, 0xff0d, 0x0d],
+ ])('accepts %s', (_reason, mask, keyval, codePoint) => {
+ expect(isValidBinding(mask, keyval, { ...gtk, codePoint })).toBe(true);
});
it('defers to Gtk on what is a valid accelerator', () => {
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;
+ }
+ },
};
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..607808a 100644
--- a/tests/taildrop-section.test.js
+++ b/tests/taildrop-section.test.js
@@ -119,6 +119,47 @@ 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');
+
+ daemon.failures.clear();
+ daemon.reset();
+ 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);
+ // 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 () => {
const { panel, model, daemon } = setup();
withFiles(daemon);