oficina / sheets Public

Close the grid library's local-only edits in Sheets

Right-click opened jspreadsheet's stock menu (Delete rows, Save as...,
About), and row/column header + Delete deleted rows only in the browser,
so every later edit posted to shifted coordinates on the server: silent
wrong-cell writes. Cmd/Ctrl+S downloaded a CSV outside the export path,
and row drag reordered rows locally. The library menu, header deletes,
export and row drag are now off; tools/drive/contextmenu.mjs proves all
14 gestures in Chromium and WebKit. Reviewed by Fable 5.1 on #33.

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

Squashed from fix/r1-grid-context-menu into main.
Paul Campbell pushed by claude agent, operated by paul@keymail.dev c55252e332fbdb60dee4d02be0e2715b7816c42b parent bf59232
3 files changed, +126 −12
  • internal/sheets/static/grid.js +9 −9
  • internal/sheets/static/sheets.css +5 −3
  • tools/drive/contextmenu.mjs +112 −0

CI cancelled — superseded by a newer push — run details

diff --git a/internal/sheets/static/grid.js b/internal/sheets/static/grid.js
index b6c2f0b..fda8e51 100644
--- a/internal/sheets/static/grid.js
+++ b/internal/sheets/static/grid.js
@@ -174,8 +174,9 @@
version = payload.version || 0;
var opts = {
- /* Our engine computes; the client paints. See the file comment. */
- parseFormulas: false,
+ parseFormulas: false, /* Our engine computes; the client paints. See the file comment. */
+ /* Library menu and Ctrl+S CSV bypass our write/export paths. false: browser menu. */
+ contextMenu: function () { return false; }, allowExport: false,
/* Every handler below is a SPREADSHEET-level option, not a
* worksheet one. Nested inside worksheets[0] they are accepted
@@ -188,10 +189,8 @@
write(parseInt(r, 10), parseInt(c, 10), value === null ? '' : String(value));
},
onselection: function (instance, x1, y1, x2, y2) {
- sel = {
- r: parseInt(y1, 10), c: parseInt(x1, 10),
- r2: parseInt(y2, 10), c2: parseInt(x2, 10)
- };
+ sel = { r: parseInt(y1, 10), c: parseInt(x1, 10),
+ r2: parseInt(y2, 10), c2: parseInt(x2, 10) };
announce();
},
/* The in-cell editor should show the formula, not its result.
@@ -217,9 +216,10 @@
minDimensions: [cols, rows],
tableOverflow: true,
columnSorting: false, /* sorting is a later slice; a half-done sort is worse than none */
- columnDrag: false,
- allowInsertRow: false,
- allowInsertColumn: false
+ /* Each of these changes the library's model only, never the server's, and then every
+ * later write lands at shifted coordinates (header + Delete, row drag). */
+ columnDrag: false, rowDrag: false, allowInsertRow: false, allowInsertColumn: false,
+ allowDeleteRow: false, allowDeleteColumn: false
}]
};
/* The resize callbacks are SPREADSHEET-level options, like the
diff --git a/internal/sheets/static/sheets.css b/internal/sheets/static/sheets.css
index c3e940a..fed0a9c 100644
--- a/internal/sheets/static/sheets.css
+++ b/internal/sheets/static/sheets.css
@@ -836,9 +836,11 @@ form[data-doc-title]:not(:focus-within) button {
/* ── The right-click menu ────────────────────────────────────────────
*
* Not Jspreadsheet's: jsuites', which it binds to `contextmenu` whether
- * or not the host app configures one. So it exists here, it is reachable
- * on every cell, and vendored it is #fff with #555 text — a white card
- * dropped on a dark grid, and the last unthemed surface on this screen.
+ * or not the host app configures one. grid.js turns it off (its stock
+ * items bypass our write paths), so today it never opens. The restatement
+ * stays because the element is still mounted, and TestVendorPaintIsRestated
+ * lists it: vendored it is #fff with #555 text — a white card on a dark
+ * grid — should anything ever open it again.
*
* It is `position: fixed` and mounted inside the grid, so it is scoped
* the same way everything else here is. */
diff --git a/tools/drive/contextmenu.mjs b/tools/drive/contextmenu.mjs
new file mode 100644
index 0000000..25b19bd
--- /dev/null
+++ b/tools/drive/contextmenu.mjs
@@ -0,0 +1,112 @@
+// The grid library's own actions must not change the grid behind the app's
+// back (B7 and its review). Each of these acts on the library's model only,
+// never the server's, and after a local row delete or row move every later
+// write lands at shifted coordinates:
+// - right-click: the stock menu ("Delete selected rows", "Add comments",
+// "Save as...", "About"). contextMenu returns false: browser menu instead.
+// - row header + Delete, column header + Delete: confirm() then deleteRow /
+// deleteColumn locally (allowDeleteRow / allowDeleteColumn: false).
+// - Ctrl/Cmd+S: the library's CSV download(), bypassing export (allowExport).
+// - dragging a row header's edge: moveRow locally (rowDrag: false).
+// Real server-rendered page and assets, mocked transport; no live data.
+// SHEETS_MENU_FIXTURE=/tmp/menus.json go test ./internal/sheets -run TestBrowserMenuFixture -count=1
+// PLAYWRIGHT=/path/to/playwright/index.mjs SHEETS_MENU_FIXTURE=/tmp/menus.json node tools/drive/contextmenu.mjs
+import assert from 'node:assert/strict';
+import fs from 'node:fs';
+const pw = await import(process.env.PLAYWRIGHT || 'playwright');
+const fixture = JSON.parse(fs.readFileSync(process.env.SHEETS_MENU_FIXTURE));
+const origin = 'https://sheets.test';
+const failures = [];
+for (const name of ['chromium', 'webkit']) {
+ const options = name === 'webkit' && process.env.WEBKIT_EXECUTABLE_PATH ? {executablePath:process.env.WEBKIT_EXECUTABLE_PATH} : {};
+ const browser = await pw[name].launch(options);
+ try {
+ const context = await browser.newContext({viewport:{width:1440,height:1000}, acceptDownloads:true});
+ const page = await context.newPage();
+ let errors = [], writes = [], dialogs = [], downloads = [];
+ page.on('pageerror', e=>errors.push(e.message));
+ // Accept every confirm(): dismissing would hide the delete it guards.
+ page.on('dialog', d=>{ dialogs.push(d.message()); d.accept(); });
+ page.on('download', d=>downloads.push(d.suggestedFilename()));
+ await page.route(origin+'/**', async route => {
+ const req = route.request(), path = new URL(req.url()).pathname;
+ if (path in fixture.assets) return route.fulfill({body:fixture.assets[path],contentType:
+ path.endsWith('.js')?'application/javascript':path.endsWith('.css')?'text/css':'image/svg+xml'});
+ if (path.endsWith('/stream')) return route.fulfill({body:'',contentType:'text/event-stream'});
+ if (req.method() !== 'GET') writes.push(path);
+ if (path.startsWith('/api/')) return route.fulfill({json:{version:1,cells:[
+ {r:0,c:0,raw:'10',display:'10'},{r:1,c:0,raw:'20',display:'20'},{r:2,c:0,raw:'30',display:'30'},
+ {r:0,c:1,raw:'b1',display:'b1'}]}});
+ return route.fulfill({body:fixture.html,contentType:'text/html'});
+ });
+ const shape = () => page.evaluate(() => {
+ const t = document.querySelector('[data-sheet-grid] .jss_worksheet');
+ const at = (x, y) => t.querySelector(`tbody td[data-x="${x}"][data-y="${y}"]`)?.textContent ?? null;
+ return {rows: t.querySelectorAll(':scope > tbody > tr').length,
+ cols: t.querySelectorAll(':scope > thead > tr:last-child > td[data-x]').length,
+ a: [at(0,0), at(0,1), at(0,2)], b1: at(1,0)};
+ });
+ async function check(label, act) {
+ await page.goto(origin+'/sheets/1');
+ await page.waitForFunction(()=>window.SheetsGrid?.worksheet());
+ await page.locator('td[data-x="0"][data-y="0"]').click();
+ const before = await shape();
+ errors = []; writes = []; dialogs = []; downloads = [];
+ try {
+ await act();
+ await page.waitForTimeout(300);
+ const after = await shape();
+ assert.deepEqual(after, before, `${name} ${label}: the grid changed locally`);
+ assert.deepEqual(dialogs, [], `${name} ${label}: the library asked to confirm`);
+ assert.deepEqual(downloads, [], `${name} ${label}: the library downloaded a file`);
+ assert.deepEqual(writes, [], `${name} ${label}: sent writes`);
+ assert.deepEqual(errors, [], `${name} ${label}: page errors`);
+ console.log(`${name}: ok ${label}`);
+ } catch (e) { failures.push(e.message); console.log(`${name}: FAIL ${label}: ${e.message.split('\n')[0]}`); }
+ }
+ const cell = page.locator('td[data-x="0"][data-y="0"]');
+ await check('right-click opens no library menu', async () => {
+ await cell.click({button:'right'});
+ const shown = await page.evaluate(()=>[...document.querySelectorAll('.jcontextmenu')]
+ .filter(m=>getComputedStyle(m).display!=='none' && m.offsetHeight>0 && m.children.length>0).map(m=>m.innerText));
+ assert.deepEqual(shown, [], `${name}: the library context menu opened: ${JSON.stringify(shown)}`);
+ for (const label of ['Delete selected rows','Add comments','Save as','About'])
+ assert.equal(await page.getByText(label).filter({visible:true}).count(), 0, `${name}: "${label}" is on screen`);
+ await page.keyboard.press('Escape');
+ // Nothing may claim the event: the browser's own menu should show.
+ const prevented = await cell.evaluate(td => {
+ const r = td.getBoundingClientRect();
+ const e = new MouseEvent('contextmenu', {bubbles:true, cancelable:true, button:2, clientX:r.x+5, clientY:r.y+5});
+ td.dispatchEvent(e); return e.defaultPrevented;
+ });
+ assert.equal(prevented, false, `${name}: something swallowed the right-click`);
+ });
+ // Playwright's WebKit treats an unclaimed Backspace as history-back and
+ // leaves the page (on main too), so Backspace is driven in Chromium only;
+ // the library's handler takes Delete and Backspace down one path.
+ for (const key of name === 'webkit' ? ['Delete'] : ['Delete', 'Backspace']) {
+ await check(`row header + ${key} deletes nothing`, async () => {
+ await page.locator('td.jss_row[data-y="1"]').click();
+ await page.keyboard.press(key);
+ });
+ await check(`column header + ${key} deletes nothing`, async () => {
+ await page.locator('thead td[data-x="0"]').click();
+ await page.keyboard.press(key);
+ });
+ }
+ for (const combo of ['Control+s', 'Meta+s']) {
+ await check(`${combo} downloads nothing`, async () => { await page.keyboard.press(combo); });
+ }
+ await check('dragging a row header moves no row', async () => {
+ const from = await page.locator('td.jss_row[data-y="0"]').boundingBox();
+ const to = await page.locator('td.jss_row[data-y="2"]').boundingBox();
+ await page.mouse.move(from.x + from.width - 3, from.y + from.height / 2);
+ await page.mouse.down();
+ await page.mouse.move(to.x + to.width - 3, to.y + to.height / 2, {steps: 8});
+ await page.mouse.up();
+ });
+ } finally {
+ await browser.close();
+ }
+}
+if (failures.length) { console.error(`${failures.length} failure(s)`); process.exit(1); }