amadan
Turn off the grid library's right-click menu
Right-clicking a cell opened jsuites' stock context menu: "Delete selected rows", "Add comments", "Copy...", "Paste...", "Save as..." and an "About" advert. Those items act on the library's own model and bypass the app's write, comment and export paths. grid.js never configured contextMenu, so the library's defaults applied. contextMenu now returns false, which the library treats as "no menu" and leaves the event alone, so the browser's own menu shows. An app menu built from the app's own commands is a possible later step; the Edit menu already carries all of them. grid.js stays at the 300-line cap: three worksheet flags share one line to make room. Test: tools/drive/contextmenu.mjs right-clicks a cell in Chromium and WebKit against the real rendered page and assets, and asserts no library menu is shown, the event is not swallowed, no write is sent and there are no page errors. It fails on main (the stock menu opens) and passes here.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+65
−6
internal/sheets/static/grid.js+3 −3internal/sheets/static/sheets.css+5 −3tools/drive/contextmenu.mjs+57 −0
CI passed — run details
CI log
amadan: .amadan/timeout: this job's budget is 15m0s (this runner allows up to 30m0s) === vet === CGO_ENABLED=0 go vet ./... === fmt === === build === CGO_ENABLED=0 go build ./... === migrations === rastrillo migration check rastrillo migration check: models and migrations agree === go-test === CGO_ENABLED=0 go test -count=1 ./... ? amadan.net/oficina/sheets/cmd/sheets [no test files] ok amadan.net/oficina/sheets/internal/guardstest 0.089s ok amadan.net/oficina/sheets/internal/sheets 10.293s ok amadan.net/oficina/sheets/internal/sheets/calc 0.008s ok amadan.net/oficina/sheets/internal/sheets/format 0.003s ok amadan.net/oficina/sheets/internal/sheets/live 0.053s ok amadan.net/oficina/sheets/internal/sheets/seal 0.002s ok amadan.net/oficina/sheets/internal/sheets/sealtest 0.003s ok amadan.net/oficina/sheets/internal/sheets/store 2.764s ok amadan.net/oficina/sheets/internal/sheets/xlsx 0.282s ? amadan.net/oficina/sheets/tools/gencorpus [no test files] === race === go test -race -count=1 ./internal/sheets/store/... ./internal/sheets/live/... ok amadan.net/oficina/sheets/internal/sheets/store 49.545s ok amadan.net/oficina/sheets/internal/sheets/live 1.059s
diff --git a/internal/sheets/static/grid.js b/internal/sheets/static/grid.js| index b6c2f0b..47aa92c 100644 |
| --- a/internal/sheets/static/grid.js |
| +++ b/internal/sheets/static/grid.js |
| @@ -176,6 +176,8 @@ |
| var opts = { |
| /* Our engine computes; the client paints. See the file comment. */ |
| parseFormulas: false, |
| + /* The library's right-click items bypass our write paths. false: browser menu. */ |
| + contextMenu: function () { return false; }, |
| /* Every handler below is a SPREADSHEET-level option, not a |
| * worksheet one. Nested inside worksheets[0] they are accepted |
| @@ -217,9 +219,7 @@ |
| 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 |
| + columnDrag: false, allowInsertRow: false, allowInsertColumn: 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 294690c..2529259 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..38eaee9 |
| --- /dev/null |
| +++ b/tools/drive/contextmenu.mjs |
| @@ -0,0 +1,57 @@ |
| +// Right-clicking a cell must not open the grid library's stock context menu |
| +// (B7). Its items ("Delete selected rows", "Add comments", "Copy...", |
| +// "Paste...", "Save as...", "About") act on the library's own model and |
| +// bypass the app's write, comment and export paths. The app configures |
| +// contextMenu to return false, so the browser's own menu shows instead. |
| +// 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'; |
| +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 page = await browser.newPage({viewport:{width:1440,height:1000}}); |
| + const errors = []; let writes = []; |
| + page.on('pageerror', e=>errors.push(e.message)); |
| + 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'}]}}); |
| + return route.fulfill({body:fixture.html,contentType:'text/html'}); |
| + }); |
| + await page.goto(origin+'/sheets/1'); |
| + await page.waitForFunction(()=>window.SheetsGrid?.worksheet()); |
| + const cell = page.locator('td[data-x="0"][data-y="0"]'); |
| + await cell.click(); |
| + writes = []; |
| + 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`); |
| + } |
| + // Dispatched as a real contextmenu event; defaultPrevented tells us whether |
| + // anything claimed it (the library's menu does, the browser's does not). |
| + const prevented = await cell.evaluate(td => { |
| + const e = new MouseEvent('contextmenu', {bubbles:true, cancelable:true, button:2, clientX:td.getBoundingClientRect().x+5, clientY:td.getBoundingClientRect().y+5}); |
| + td.dispatchEvent(e); |
| + return e.defaultPrevented; |
| + }); |
| + assert.equal(prevented, false, `${name}: something swallowed the right-click instead of leaving the browser menu`); |
| + assert.deepEqual(writes, [], `${name}: right-click sent writes`); |
| + assert.deepEqual(errors, [], `${name}: page errors`); |
| + console.log(`${name}: no library context menu`); |
| + } finally { |
| + await browser.close(); |
| + } |
| +} |