oficina / sheets Public

A modal dialog owns the keyboard: no sheet command runs behind it

The Keyboard shortcuts dialog is a native modal, which keeps the page
behind it inert to the pointer and to focus but not to keys: Ctrl+D on
its Done button filled A1 down over A2 and persisted (Astra, #39 finding
1), and Ctrl+B, Ctrl+Z, Delete and letter keys reached commands.js,
clipboard.js and the grid library too. static/modal.js, the first script
on every page, stops key and clipboard events at the window in the
capture phase while any modal dialog is open. Defaults still run: Tab,
Enter on Done and Escape behave as before.

tools/drive/modal.mjs opens the dialog on a real sheet, presses Ctrl/Meta
D, B, Z, X, V, Delete, Backspace, a letter and Enter, closes it, commits
any hidden editor and asserts nothing was POSTed and the cells survive a
reload. Red on the pre-fix build in Chromium and WebKit (5 failures),
green after. TestTheModalGuardIsTheFirstScript pins the load order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 3 weeks ago 1b2fe6e2457057dc61cd8392325dacda9cddc3cd parent a47411d

4 files changed, +201 −0

internal/sheets/chrome_menu_test.go +24 −0
internal/sheets/static/modal.js +38 −0
internal/sheets/templates/layout.html +4 −0
tools/drive/modal.mjs +135 −0

diff --git a/internal/sheets/chrome_menu_test.go b/internal/sheets/chrome_menu_test.go

index c3e1b2c..69811b2 100644

--- a/internal/sheets/chrome_menu_test.go

+++ b/internal/sheets/chrome_menu_test.go

@@ -209,3 +209,27 @@ func TestTemplatesUseOnlyCalloutTones(t *testing.T) {

t.Errorf("a new link is not one warning callout (status %d)", rec.Code)

}

+// While a modal dialog is open no page listener may hear a key: static/

+// modal.js stops key and clipboard events at the window, in the capture

+// phase, and it has to be the first script so its listener is registered

+// before the grid library's (Astra, #39 finding 1; the browser proof is

+// tools/drive/modal.mjs, Chromium and WebKit).

+func TestTheModalGuardIsTheFirstScript(t *testing.T) {

+ app, cookies := signIn(t)

+ s, _ := newSheet(t, app, cookies, "Budget")

+ body := page(t, app, cookies, "/sheets/"+strconv.FormatUint(s.ID, 10)).Body.String()

+ first := regexp.MustCompile(`<script[^>]*src="([^"]+)"`).FindStringSubmatch(body)

+ if first == nil || !strings.HasPrefix(first[1], "/static/modal.") {

+ t.Fatalf("the first script is %v, want static/modal.js", first)

+ }

+ js, err := staticFiles().(interface{ ReadFile(string) ([]byte, error) }).ReadFile("static/modal.js")

+ if err != nil {

+ t.Fatal(err)

+ }

+ for _, want := range []string{"window.addEventListener(type", "stopImmediatePropagation()", "}, true);", "'keydown'", "'paste'", "dialog:modal"} {

+ if !strings.Contains(string(js), want) {

+ t.Errorf("modal.js lost %q", want)

+ }

diff --git a/internal/sheets/static/modal.js b/internal/sheets/static/modal.js

new file mode 100644

index 0000000..4750db1

--- /dev/null

+++ b/internal/sheets/static/modal.js

@@ -0,0 +1,38 @@

+/* A modal dialog owns the keyboard.

+ *

+ * A native modal <dialog> makes the page behind it inert to the pointer

+ * and to focus, but not to key events: a keydown on the dialog's own

+ * button still travels to document and window, where commands.js,

+ * clipboard.js and the vendored grid all listen. So with the Keyboard

+ * shortcuts dialog open, Ctrl+D filled the selection down behind it and

+ * a letter key started an edit in a cell nobody could see — persisted

+ * writes made from a dialog (Astra, #39 finding 1).

+ *

+ * So while any modal dialog is open, key and clipboard events stop here,

+ * at the window in the capture phase, before any page listener hears

+ * them. Stopping propagation is not preventing the default: Tab still

+ * moves inside the dialog, Enter and Space still press its buttons,

+ * typing still reaches its fields and Escape still closes it. A dialog

+ * that needs key handling of its own adds it on the dialog element in

+ * the capture phase.

+ *

+ * Loaded first of the page's scripts, so its listener is registered

+ * before any other window listener. General on purpose: it is about

+ * every modal on every page, not about one dialog. */

+(function () {

+ 'use strict';

+ function modalOpen() {

+ try {

+ return !!document.querySelector('dialog:modal');

+ } catch (e) {

+ return !!document.querySelector('dialog[open]');

+ }

+ ['keydown', 'keypress', 'keyup', 'beforeinput', 'paste', 'cut', 'copy'].forEach(function (type) {

+ window.addEventListener(type, function (e) {

+ if (modalOpen()) e.stopImmediatePropagation();

+ }, true);

+ });

+}());

diff --git a/internal/sheets/templates/layout.html b/internal/sheets/templates/layout.html

index 9229294..ffe3073 100644

--- a/internal/sheets/templates/layout.html

+++ b/internal/sheets/templates/layout.html

@@ -38,6 +38,10 @@

+{{/* First of every script: while a modal dialog is open, key and

+ clipboard events stop at the window before any page listener hears

+ them (static/modal.js). */}}

+<script defer src="{{asset "static/modal.js"}}"></script>

diff --git a/tools/drive/modal.mjs b/tools/drive/modal.mjs

new file mode 100644

index 0000000..e3f258e

--- /dev/null

+++ b/tools/drive/modal.mjs

@@ -0,0 +1,135 @@

+/* A modal dialog owns the keyboard: no sheet command runs behind it.

+ *

+ * The Keyboard shortcuts dialog (account menu → Help) is a native modal

+ * <dialog>. Native modality makes the page behind it inert to the

+ * pointer and to focus, but not to key events: a keydown on the dialog's

+ * Done button still bubbles to document, where commands.js, clipboard.js

+ * and the vendored grid all listen. Astra found Ctrl+D inside the open

+ * dialog filling A1 down over A2 and persisting (#39, finding 1).

+ *

+ * This opens the dialog on a real sheet, presses the editing keys, closes

+ * it, commits anything a hidden editor might be holding, and asserts that

+ * nothing was POSTed and the persisted cells are unchanged after a

+ * reload. Chromium and WebKit, each with its own session.

+ *

+ * go build -o /tmp/sheets ./cmd/sheets

+ * SHEETS_ORIGIN=http://127.0.0.1:8799 SHEETS_INSTANCE_KEY=dev SHEETS_OWNERS=paul@example.com \

+ * SHEETS_DEK="$(head -c 32 /dev/urandom | base64)" /tmp/sheets --db /tmp/s.db --socket /tmp/s.sock > /tmp/s.log 2>&1 &

+ * SHEETS_SOCK=/tmp/s.sock SHEETS_LOG=/tmp/s.log PLAYWRIGHT=/path/to/playwright/index.mjs node tools/drive/modal.mjs

+ */

+import net from 'node:net';

+import fs from 'node:fs';

+const pw = await import(process.env.PLAYWRIGHT || 'playwright');

+const SOCK = process.env.SHEETS_SOCK || '/tmp/s.sock';

+const LOG = process.env.SHEETS_LOG || '/tmp/s.log';

+const PORT = 8799;

+const base = `http://127.0.0.1:${PORT}`;

+const proxy = net.createServer((c) => {

+ const u = net.connect(SOCK);

+ c.pipe(u); u.pipe(c);

+ c.on('error', () => u.destroy()); u.on('error', () => c.destroy());

+});

+await new Promise((r) => proxy.listen(PORT, '127.0.0.1', r));

+let failed = 0;

+const check = (name, ok, detail = '') => {

+ if (!ok) failed++;

+ console.log(` ${ok ? 'ok ' : 'FAIL'} ${name}${ok || !detail ? '' : ' — ' + detail}`);

+};

+async function signIn(page) {

+ const before = fs.readFileSync(LOG, 'utf8').length;

+ await page.goto(base + '/signin');

+ await page.fill('input[name=address]', 'paul@example.com');

+ await Promise.all([page.waitForURL(/sent/), page.click('form[action="/signin"] button[type=submit]')]);

+ let m = null;

+ for (let i = 0; i < 50 && !m; i++) {

+ m = [...fs.readFileSync(LOG, 'utf8').slice(before).matchAll(/http:\/\/127\.0\.0\.1:8799\/auth\/verify\?[^\s\\"]+/g)].pop();

+ if (!m) await page.waitForTimeout(100);

+ }

+ await page.goto(m[0]);

+const cell = (page, c, r) => page.evaluate(([c, r]) => {

+ const ws = window.SheetsGrid.worksheet();

+ return { value: ws.getValueFromCoords(c, r), bold: ws.records[r][c].element.hasAttribute('data-b') };

+}, [c, r]);

+for (const name of ['chromium', 'webkit']) {

+ console.log(`\n— ${name}`);

+ const browser = await pw[name].launch();

+ const page = await browser.newPage({ viewport: { width: 1440, height: 900 } });

+ const errors = [];

+ page.on('pageerror', (e) => errors.push(e.message));

+ await signIn(page);

+ await Promise.all([page.waitForURL(/\/sheets\/\d+$/), page.click('[estilo-bar-actions] form[action="/sheets"] button')]);

+ const sheet = page.url();

+ await page.waitForFunction(() => window.SheetsGrid && window.SheetsGrid.worksheet(), null, { timeout: 15000 });

+ await page.evaluate(() => { window.SheetsGrid.write(0, 0, 'Invoice'); window.SheetsGrid.write(1, 0, '920.5'); });

+ await page.waitForTimeout(600);

+ await page.reload();

+ await page.waitForFunction(() => window.SheetsGrid && window.SheetsGrid.worksheet(), null, { timeout: 15000 });

+ await page.locator('td[data-x="0"][data-y="0"]').click();

+ await page.evaluate(() => window.SheetsGrid.worksheet().updateSelectionFromCoords(0, 0, 0, 1));

+ const posts = [];

+ page.on('request', (req) => { if (req.method() !== 'GET') posts.push(`${req.method()} ${new URL(req.url()).pathname}`); });

+ await page.click('[estilo-account] > summary');

+ await page.click('[estilo-account-panel] button[commandfor="sheets-shortcuts"]');

+ await page.waitForTimeout(200);

+ check('the dialog is open and owns focus', await page.evaluate(() => {

+ const d = document.getElementById('sheets-shortcuts');

+ return d.open && d.contains(document.activeElement);

+ }));

+ // Backspace only in Chromium: Playwright's WebKit build treats it as

+ // history Back on a non-editable focus, a browser default no page can

+ // or should suppress (Safari does not). Delete covers the same command.

+ const keys = ['Control+d', 'Meta+d', 'Control+b', 'Meta+b', 'Control+z', 'Meta+z', 'Control+x', 'Delete', 'x', 'Enter', 'Control+v'];

+ if (name === 'chromium') keys.push('Backspace');

+ for (const key of keys) {

+ await page.keyboard.press(key);

+ if (!(await page.evaluate(() => document.getElementById('sheets-shortcuts').open))) {

+ // Enter on Done closes the dialog, which is its job; reopen it for

+ // the keys that follow.

+ await page.click('[estilo-account] > summary');

+ await page.click('[estilo-account-panel] button[commandfor="sheets-shortcuts"]');

+ await page.waitForTimeout(100);

+ }

+ await page.waitForTimeout(400);

+ await page.keyboard.press('Escape');

+ await page.waitForTimeout(200);

+ check('Escape closes the dialog', !(await page.evaluate(() => document.getElementById('sheets-shortcuts').open)));

+ // Commit anything a hidden in-cell editor could be holding, by clicking

+ // another cell (D6).

+ await page.waitForFunction(() => window.SheetsGrid && window.SheetsGrid.worksheet(), null, { timeout: 15000 });

+ const d6 = await page.evaluate(() => {

+ const r = window.SheetsGrid.worksheet().records[5][3].element.getBoundingClientRect();

+ return { x: r.x + r.width / 2, y: r.y + r.height / 2 };

+ });

+ await page.mouse.click(d6.x, d6.y);

+ await page.waitForTimeout(800);

+ check('nothing was posted while the dialog owned the keyboard', posts.length === 0, posts.join(', '));

+ await page.goto(sheet);

+ await page.waitForFunction(() => window.SheetsGrid && window.SheetsGrid.worksheet(), null, { timeout: 15000 });

+ const a1 = await cell(page, 0, 0), a2 = await cell(page, 0, 1);

+ check('A1 is unchanged after a reload', a1.value === 'Invoice' && !a1.bold, JSON.stringify(a1));

+ check('A2 is unchanged after a reload', a2.value === '920.5' && !a2.bold, JSON.stringify(a2));

+ // The keys still work when no dialog is open: the suppression is the

+ // modal's, not a switch left off.

+ const wasBold = (await cell(page, 0, 0)).bold;

+ await page.locator('td[data-x="0"][data-y="0"]').click();

+ await page.keyboard.press('Control+b');

+ await page.waitForTimeout(600);

+ check('...and Ctrl+B still toggles bold once the dialog is closed', (await cell(page, 0, 0)).bold !== wasBold);

+ check('no page errors', errors.length === 0, errors.join(' | '));

+ await browser.close();

+proxy.close();

+console.log(failed ? `\n${failed} check(s) failed` : '\nall checks passed');

+process.exit(failed ? 1 : 0);