diff --git a/docs/PAGES.md b/docs/PAGES.md index 4d58bcd..bcebe09 100644 --- a/docs/PAGES.md +++ b/docs/PAGES.md @@ -37,6 +37,8 @@ flowchart TD "Create a new database" is the other way into the same iframe: with no file to sniff, the connector embeds `0x67.html` straight away and, once it announces readiness, tells it to start a fresh database instead of opening one (`kw-create`, the create-side counterpart to `kw-open` in `packages/embed-protocol`). Naming and creating the database — `Kdbx.create` and everything it depends on — happens exactly where opening one does, inside `0x67.html`; the connector never gains its own copy of that logic, it only decides which of the two messages to send. +A connector owns two things beyond the file itself, both because the browser gives them to whichever document owns the tab rather than to the one inside the iframe. The first is the tab: the app knows which database is open and whether it is locked, but only the connector can name the tab and mark it with that state, so the app reports and the connector applies — which is why that logic lives in `pages/shared/` rather than in either page. The second is the keyboard: a keystroke goes to whichever document has focus, so a find pressed while the visitor is on the connector's own chrome would search the connector's page instead of the database, and the connector forwards it to the app rather than let that happen. + On save, since there is nowhere to write back to, the local connector downloads the updated bytes the same way `0x67.html` would if opened standalone — the only piece of this connector that's genuinely local-specific. This applies equally to a freshly created database: its first save is just a download, named for whatever the create screen's own form was given. ## How the Google Drive connector works diff --git a/e2e/auto-lock.test.ts b/e2e/auto-lock.test.ts index d2e10c0..2e49897 100644 --- a/e2e/auto-lock.test.ts +++ b/e2e/auto-lock.test.ts @@ -81,7 +81,7 @@ test('a tab left hidden locks the embedded database on its own', async () => { await otherTab.close(); assert.equal( await page.title(), - `🔒 ${basename(fixture.path)} - KeePass Web - Local file`, + `🔒 ${basename(fixture.path)} - Locked - KeePass Web - Local file`, 'and the tab bar shows it locked, without being opened', ); }); diff --git a/e2e/entry-copy.test.ts b/e2e/entry-copy.test.ts index bcde9c1..c16c074 100644 --- a/e2e/entry-copy.test.ts +++ b/e2e/entry-copy.test.ts @@ -1,7 +1,9 @@ -/** Real-browser coverage for the entry table's controls (issue #67). jsdom - * dispatches events straight at an element; only a real browser hit-tests a - * coordinate, so this is what proves the cells and the open control are - * actually clickable where they render. +/** Real-browser coverage for the entry table's controls (issues #67, #76, + * #77). jsdom dispatches events straight at an element and has no layout + * engine at all; only a real browser hit-tests a coordinate and gives a cell + * or a column a width, so this is what proves the controls are clickable where + * they render, that the copy control holds the cell's right edge, and that a + * dragged column actually changes size. * * What gets copied is asserted in the jsdom tests instead: headless Chrome * refuses `navigator.clipboard.writeText` outright ("Write permission denied"), @@ -49,7 +51,7 @@ async function usernameCellCentre(): Promise<{ x: number; y: number }> { const box = await app.$$eval( '.entry-table tbody td', (cells, name) => { - const cell = cells.find((c) => c.firstChild?.textContent === name); + const cell = cells.find((c) => c.querySelector('.entry-cell-text')?.textContent === name); if (!cell) return null; const rect = cell.getBoundingClientRect(); return { x: rect.x + rect.width / 2, y: rect.y + rect.height / 2 }; @@ -69,6 +71,62 @@ test('clicking a value never opens the entry', async () => { assert.equal(await app.$('#detail-title'), null, 'the card stayed shut'); }); +test('the copy control holds the right edge of its cell', async () => { + const geometry = await app.$$eval( + '.entry-table tbody td', + (cells, name) => { + const cell = cells.find((c) => c.querySelector('.entry-cell-text')?.textContent === name); + const hint = cell?.querySelector('.copy-hint'); + const text = cell?.querySelector('.entry-cell-text'); + if (!cell || !hint || !text) return null; + const inner = cell.querySelector('.entry-cell') as HTMLElement; + return { + gapToTheRightEdge: inner.getBoundingClientRect().right - hint.getBoundingClientRect().right, + gapAfterTheText: hint.getBoundingClientRect().left - text.getBoundingClientRect().right, + }; + }, + 'octocat', + ); + assert.ok(geometry, 'the username cell carries both a text box and a copy control'); + + assert.ok( + geometry.gapToTheRightEdge < 1, + `the control sits at the cell's right edge, ${geometry.gapToTheRightEdge}px short of it`, + ); + // A short value leaves room, and the control does not follow the text into it. + assert.ok( + geometry.gapAfterTheText > 8, + `it is pinned there rather than trailing the text, ${geometry.gapAfterTheText}px behind it`, + ); +}); + +test('dragging a column header changes that column width', async () => { + const handle = await app.$('th[data-column="username"] .col-resize'); + assert.ok(handle, 'every resizable column carries a handle'); + + const widthOf = (): Promise => + app.$eval('th[data-column="username"]', (th) => th.getBoundingClientRect().width); + const before = await widthOf(); + + const box = await handle.boundingBox(); + assert.ok(box, 'the handle is laid out'); + const y = box.y + box.height / 2; + await page.mouse.move(box.x + box.width / 2, y); + await page.mouse.down(); + await page.mouse.move(box.x + box.width / 2 + 90, y); + await page.mouse.up(); + + const after = await widthOf(); + assert.ok(after > before + 60, `the column widened, from ${before}px to ${after}px`); + assert.equal( + await app.$eval('th[data-column="username"] .col-resize', (el) => + el.getAttribute('aria-valuenow'), + ), + String(Math.round(after)), + 'and says so to anyone reading it out', + ); +}); + test('the row control is the way into the card', async () => { const openButton = await app.$('.entry-table-open button'); assert.ok(openButton, 'every row carries one'); diff --git a/e2e/local-to-app-embed.test.ts b/e2e/local-to-app-embed.test.ts index c3b9dd8..c1de623 100644 --- a/e2e/local-to-app-embed.test.ts +++ b/e2e/local-to-app-embed.test.ts @@ -69,13 +69,58 @@ test('dropping a file on local.html embeds a working 0x67 app that unlocks the s await iframeFrame.click('#unlock-btn'); await iframeFrame.waitForSelector('.entry-table'); - const titleText = await iframeFrame.$eval('.entry-table-title', (el) => el.textContent); + const titleText = await iframeFrame.$eval( + '.entry-table-title .entry-cell-text', + (el) => el.textContent, + ); assert.ok( titleText?.includes(fixture.entryTitle), `unlocked vault shows the fixture entry, got "${titleText}"`, ); }); +test('the find keystroke reaches the app even when focus is on the host page', async () => { + const fixture = await writeKdbxFixture(); + + await page.goto(`${server.origin}/local.html`, { waitUntil: 'networkidle0' }); + const fileInput = (await page.waitForSelector('#file-input')) as ElementHandle; + assert.ok(fileInput, 'the file input exists'); + await fileInput.uploadFile(fixture.path); + + const iframeElement = await page.waitForSelector('#app-frame'); + assert.ok(iframeElement, 'the app is embedded'); + const app = await iframeElement.contentFrame(); + assert.ok(app, 'the iframe has a content frame'); + + const passwordInput = await app.waitForSelector('#master-password'); + assert.ok(passwordInput, 'the app shows its unlock screen'); + await passwordInput.type(fixture.password); + await app.click('#unlock-btn'); + await app.waitForSelector('.entry-table'); + + // Click the host's own chrome, so this document — not the iframe — is the + // one holding focus and the one the keystroke will be delivered to. Without + // the host forwarding it, the app would never see it at all. + await page.click('#host-filename'); + await app.$eval('#search-input', (el) => (el as HTMLElement).blur()); + assert.notEqual( + await app.evaluate(() => document.activeElement?.id), + 'search-input', + 'focus really is off the search field to begin with', + ); + + await page.keyboard.down('Control'); + await page.keyboard.press('f'); + await page.keyboard.up('Control'); + + await app.waitForFunction(() => document.activeElement?.id === 'search-input'); + assert.equal( + await app.evaluate(() => document.activeElement?.id), + 'search-input', + 'the find crossed into the app and landed in its database-wide search', + ); +}); + test('clicking "Create a new database" on local.html embeds the app straight on its create-database screen', async () => { await page.goto(`${server.origin}/local.html`, { waitUntil: 'networkidle0' }); diff --git a/e2e/reveal-focus.test.ts b/e2e/reveal-focus.test.ts new file mode 100644 index 0000000..b6b498d --- /dev/null +++ b/e2e/reveal-focus.test.ts @@ -0,0 +1,65 @@ +/** The caret has to survive a click on a control that acts on the field it + * sits beside (issue #72). jsdom never moves focus on a press at all, so the + * pages suite can only assert that the press is cancelled; a real browser is + * the only place the focus move it prevents actually happens. The unlock + * screen's reveal toggle stands in for the entry-edit row's controls too — + * all of them go through the same helper. */ +import assert from 'node:assert/strict'; +import { after, before, test } from 'node:test'; +import { fileURLToPath } from 'node:url'; +import puppeteer, { type Browser, type ElementHandle, type Frame, type Page } from 'puppeteer-core'; +import { resolveChromePath } from './support/chrome.ts'; +import { type DistServer, startDistServer } from './support/dist-server.ts'; +import { writeKdbxFixture } from './support/fixture.ts'; +import { resolveLaunchOptions } from './support/launch-options.ts'; + +const distDir = fileURLToPath(new URL('../dist', import.meta.url)); + +let server: DistServer; +let browser: Browser; +let page: Page; + +before(async () => { + server = await startDistServer(distDir); + browser = await puppeteer.launch({ + executablePath: resolveChromePath(), + ...resolveLaunchOptions(), + args: ['--no-sandbox'], + }); + page = await browser.newPage(); +}); + +after(async () => { + await browser.close(); + await server.close(); +}); + +test('revealing the master password leaves the caret in the field', async () => { + const fixture = await writeKdbxFixture(); + + await page.goto(`${server.origin}/local.html`, { waitUntil: 'networkidle0' }); + const fileInput = (await page.waitForSelector('#file-input')) as ElementHandle; + assert.ok(fileInput, 'the chooser offers a file input'); + await fileInput.uploadFile(fixture.path); + + const frameElement = await page.waitForSelector('#app-frame'); + assert.ok(frameElement, 'the app is embedded in an iframe'); + const app = (await frameElement.contentFrame()) as Frame; + + // Stop at the unlock screen: this is about typing a password, not reading a database. + const passwordInput = await app.waitForSelector('#master-password'); + assert.ok(passwordInput, 'the embedded app shows its unlock screen'); + await passwordInput.type('half-typed'); + + await app.click('[data-action="toggle-password"]'); + + const state = await app.evaluate(() => ({ + focused: document.activeElement?.id ?? '', + type: (document.getElementById('master-password') as HTMLInputElement).type, + value: (document.getElementById('master-password') as HTMLInputElement).value, + })); + + assert.equal(state.focused, 'master-password', 'the field kept focus, so typing carries on'); + assert.equal(state.type, 'text', 'and the toggle still revealed the password'); + assert.equal(state.value, 'half-typed', 'without disturbing what was already typed'); +}); diff --git a/e2e/tab-title.test.ts b/e2e/tab-title.test.ts index acdbaca..f97ab6c 100644 --- a/e2e/tab-title.test.ts +++ b/e2e/tab-title.test.ts @@ -1,9 +1,9 @@ -/** The tab title belongs to local.html, but only the embedded 0x67 app knows - * which database is open and whether it is locked — so the title is right - * only if a real cross-document postMessage is delivered and handled. The - * jsdom suites test each page in its own isolated window and cannot show - * that; this drives the built distributables in Chrome, where the two - * documents really are separate. */ +/** The tab belongs to local.html, but only the embedded 0x67 app knows which + * database is open and whether it is locked — so the tab's name and its icon + * are right only if a real cross-document postMessage is delivered and + * handled. The jsdom suites test each page in its own isolated window and + * cannot show that; this drives the built distributables in Chrome, where the + * two documents really are separate. */ import assert from 'node:assert/strict'; import { basename } from 'node:path'; import { after, before, test } from 'node:test'; @@ -51,12 +51,16 @@ async function waitForTitle(expected: string): Promise { } } +const tabIcon = (): Promise => + page.$eval('link[rel="icon"]', (link) => link.getAttribute('href') ?? ''); + test('the tab names the open database and tracks its lock state', async () => { const fixture = await writeKdbxFixture(); const filename = basename(fixture.path); await page.goto(`${server.origin}/local.html`, { waitUntil: 'networkidle0' }); assert.equal(await page.title(), BASE_TITLE, 'nothing open, so the tab is just this page'); + const pageIcon = await tabIcon(); // waitForSelector can't infer the element type from an id selector. const fileInput = (await page.waitForSelector('#file-input')) as ElementHandle; @@ -68,7 +72,9 @@ test('the tab names the open database and tracks its lock state', async () => { const iframeFrame = await iframeElement.contentFrame(); assert.ok(iframeFrame, 'the iframe has a content frame'); - await waitForTitle(`🔒 ${filename} - ${BASE_TITLE}`); + await waitForTitle(`🔒 ${filename} - Locked - ${BASE_TITLE}`); + const lockedIcon = await tabIcon(); + assert.notEqual(lockedIcon, pageIcon, 'a held database is not the page at rest'); const passwordInput = await iframeFrame.waitForSelector('#master-password'); assert.ok(passwordInput, 'the embedded app went straight to its unlock screen'); @@ -76,7 +82,9 @@ test('the tab names the open database and tracks its lock state', async () => { await iframeFrame.click('#unlock-btn'); await iframeFrame.waitForSelector('.entry-table'); - await waitForTitle(`🔓 ${filename} - ${BASE_TITLE}`); + await waitForTitle(`🔓 ${filename} - Unlocked - ${BASE_TITLE}`); + const unlockedIcon = await tabIcon(); + assert.notEqual(unlockedIcon, lockedIcon, 'and the two states do not share an icon'); // Nothing is unsaved, but the app still asks before giving the database up. await page.click('[data-action="back-to-chooser"]'); @@ -86,4 +94,5 @@ test('the tab names the open database and tracks its lock state', async () => { await iframeFrame.click('#dlg-confirm-discard [data-action="confirm-discard"]'); await page.waitForSelector('#drop-zone'); await waitForTitle(BASE_TITLE); + assert.equal(await tabIcon(), pageIcon, 'closing hands the page its own icon back'); }); diff --git a/packages/embed-protocol/src/index.ts b/packages/embed-protocol/src/index.ts index 14792f6..d8f35ca 100644 --- a/packages/embed-protocol/src/index.ts +++ b/packages/embed-protocol/src/index.ts @@ -2,7 +2,8 @@ keepass-web implementation and whatever host embeds it in an iframe. Centralizes shapes/guards/builders (previously duplicated per side) so both ends provably agree on the wire format: kw-ready, kw-open, kw-create, -kw-save, kw-saved, kw-title, kw-close-request, kw-close-ack, kw-close. */ +kw-save, kw-saved, kw-title, kw-find, kw-close-request, kw-close-ack, +kw-close. */ export interface ReadyMessage { type: 'kw-ready'; @@ -38,6 +39,13 @@ export interface TitleMessage { locked: boolean; } +/* Whichever document has focus receives the keystroke, and outside the iframe +that is the host; it forwards the find rather than letting the browser's own +search the one page it can see (#78). */ +export interface FindMessage { + type: 'kw-find'; +} + export interface CloseRequestMessage { type: 'kw-close-request'; } @@ -96,6 +104,10 @@ export function isTitleMessage(data: unknown): data is TitleMessage { return typeof rec.filename === 'string' && typeof rec.locked === 'boolean'; } +export function isFindMessage(data: unknown): data is FindMessage { + return hasType(data, 'kw-find'); +} + export function isCloseRequestMessage(data: unknown): data is CloseRequestMessage { return hasType(data, 'kw-close-request'); } @@ -134,6 +146,10 @@ export function titleMessage(filename: string, locked: boolean): TitleMessage { return { type: 'kw-title', filename, locked }; } +export function findMessage(): FindMessage { + return { type: 'kw-find' }; +} + export function closeRequestMessage(): CloseRequestMessage { return { type: 'kw-close-request' }; } diff --git a/packages/embed-protocol/tests/index.test.ts b/packages/embed-protocol/tests/index.test.ts index 6267d75..e414d79 100644 --- a/packages/embed-protocol/tests/index.test.ts +++ b/packages/embed-protocol/tests/index.test.ts @@ -5,10 +5,12 @@ import { closeMessage, closeRequestMessage, createMessage, + findMessage, isCloseAckMessage, isCloseMessage, isCloseRequestMessage, isCreateMessage, + isFindMessage, isOpenMessage, isReadyMessage, isSavedMessage, @@ -84,6 +86,14 @@ test('titleMessage / isTitleMessage round-trip', () => { assert.equal(isTitleMessage({ type: 'kw-title', filename: 42, locked: true }), false); }); +test('findMessage / isFindMessage round-trip', () => { + assert.deepEqual(findMessage(), { type: 'kw-find' }); + assert.equal(isFindMessage(findMessage()), true); + assert.equal(isFindMessage(null), false); + assert.equal(isFindMessage(42), false); + assert.equal(isFindMessage({ type: 'nope' }), false); +}); + test('closeRequestMessage / isCloseRequestMessage round-trip', () => { assert.deepEqual(closeRequestMessage(), { type: 'kw-close-request' }); assert.equal(isCloseRequestMessage(closeRequestMessage()), true); diff --git a/pages/0x67/bundle-iife.json b/pages/0x67/bundle-iife.json index b692584..a6367b8 100644 --- a/pages/0x67/bundle-iife.json +++ b/pages/0x67/bundle-iife.json @@ -3,13 +3,10 @@ "output": "../../build/pages/0x67/bundle.js", "files": [ "embed-protocol/src/index.js", - "chacha20/src/index.js", - "argon2/src/blake2b.js", "argon2/src/argon2.js", "argon2/src/index.js", - "kdbx/src/bytes.js", "kdbx/src/xml.js", "kdbx/src/constants.js", @@ -26,7 +23,7 @@ "kdbx/src/model.js", "kdbx/src/meta-binaries.js", "kdbx/src/kdbx.js", - + "../pages/shared/logic.js", "../pages/0x67/logic.js", "../pages/0x67/page.js" ], @@ -59,7 +56,6 @@ "pushHistorySnapshot", "restoreHistoryEntry", "deleteHistoryEntry", - "entryField", "entryTitle", "groupName", @@ -83,7 +79,6 @@ "isoToLocalInputValue", "localInputValueToIso", "defaultExpiryLocalInputValue", - "isOpenMessage", "isCreateMessage", "isSavedMessage", @@ -92,6 +87,8 @@ "saveMessage", "titleMessage", "closeAckMessage", - "closeMessage" + "closeMessage", + "applyTabState", + "isFindMessage" ] } diff --git a/pages/0x67/globals.d.ts b/pages/0x67/globals.d.ts index 923c7b1..8f05b01 100644 --- a/pages/0x67/globals.d.ts +++ b/pages/0x67/globals.d.ts @@ -41,6 +41,9 @@ interface TitleMessage { interface CloseRequestMessage { type: 'kw-close-request'; } +interface FindMessage { + type: 'kw-find'; +} interface ReadyMessage { type: 'kw-ready'; } @@ -55,9 +58,17 @@ declare function isOpenMessage(data: unknown): data is OpenMessage; declare function isCreateMessage(data: unknown): data is CreateMessage; declare function isSavedMessage(data: unknown): data is SavedMessage; declare function isCloseRequestMessage(data: unknown): data is CloseRequestMessage; +declare function isFindMessage(data: unknown): data is FindMessage; declare function readyMessage(): ReadyMessage; declare function saveMessage(filename: string, bytes: ArrayBuffer): SaveMessage; declare function titleMessage(filename: string, locked: boolean): TitleMessage; + +declare function applyTabState( + doc: Document, + baseTitle: string, + filename: string, + locked: boolean, +): void; declare function closeAckMessage(): CloseAckMessage; declare function closeMessage(): CloseMessage; diff --git a/pages/0x67/page.css b/pages/0x67/page.css index 57cd3e3..489c4f1 100644 --- a/pages/0x67/page.css +++ b/pages/0x67/page.css @@ -774,8 +774,12 @@ so an absolutely positioned menu would be cut off. */ width: 100%; border-collapse: collapse; font-size: 0.85rem; + /* Only under fixed layout does a column's width mean anything (#77). */ + table-layout: fixed; } +/* Starting proportions, not caps: spare width is shared out among them, and a +drag replaces the value outright (#77). */ .entry-table th { position: sticky; top: 0; @@ -789,6 +793,45 @@ so an absolutely positioned menu would be cut off. */ text-transform: uppercase; color: var(--muted); white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + width: 10rem; +} + +.entry-table th[data-column="title"], +.entry-table th[data-column="url"] { + width: 15rem; +} + +.entry-table th[data-column="password"] { + width: 8rem; +} + +/* Sits inside the sticky header, which is its containing block (#77). */ +.col-resize { + position: absolute; + top: 0; + right: 0; + width: 12px; + height: 100%; + cursor: col-resize; + touch-action: none; +} + +.col-resize::before { + content: ""; + position: absolute; + top: 25%; + bottom: 25%; + right: 5px; + width: 1px; + background: var(--border); +} + +.col-resize:hover::before, +.col-resize:focus-visible::before { + width: 2px; + background: var(--accent); } /* A tap on a cell copies (#67), so the cell must not start a text selection or @@ -799,17 +842,30 @@ wait on a double-tap-to-zoom before that tap registers. */ -webkit-touch-callout: none; padding: 0.5rem 0.75rem; border-bottom: 1px solid var(--border); - max-width: 220px; + overflow: hidden; + cursor: pointer; +} + +/* min-width:0 is what lets the text box shrink below its content, so it is the +text that ellipsizes and the copy control that keeps its place (#76). */ +.entry-cell { + display: flex; + align-items: center; + gap: 0.35rem; + min-width: 0; +} + +.entry-cell-text { overflow: hidden; text-overflow: ellipsis; white-space: nowrap; - cursor: pointer; } /* Present at rest, brighter on hover or focus (#67): hover may raise emphasis, never be what makes the affordance discoverable, since touch has none. */ .copy-hint { - margin-left: 0.35rem; + margin-left: auto; + flex: 0 0 auto; padding: 0; border: none; background: none; @@ -824,8 +880,11 @@ never be what makes the affordance discoverable, since touch has none. */ opacity: 1; } +.entry-table th.entry-table-open { + width: 2.5rem; +} + .entry-table-open { - width: 1px; text-align: right; white-space: nowrap; } diff --git a/pages/0x67/page.ts b/pages/0x67/page.ts index 7292ca6..9fc2017 100644 --- a/pages/0x67/page.ts +++ b/pages/0x67/page.ts @@ -112,6 +112,14 @@ function makeIconButton( return btn; } +/* A pointer press moves focus to the button before the click lands, so a +control that acts on a field takes the caret out of that field; suppressing +the press's default leaves focus and caret exactly where they were, while a +keyboard user still reaches the button by tabbing to it (#72). */ +function keepFieldFocus(button: HTMLElement): void { + button.addEventListener('mousedown', (event) => event.preventDefault()); +} + // Wire a dialog's ✕ button to close it, replacing a repeated line in every openXDialog. function wireClose(dlg: HTMLDialogElement): void { must(dlg.querySelector('[data-action="close"]')).onclick = () => dlg.close(); @@ -230,6 +238,7 @@ function showUnlock(preserveDirty = false): void { }); const togglePasswordBtn = qs('[data-action="toggle-password"]'); + keepFieldFocus(togglePasswordBtn); togglePasswordBtn.addEventListener('click', () => { passwordInput.type = passwordInput.type === 'password' ? 'text' : 'password'; togglePasswordBtn.textContent = passwordInput.type === 'password' ? '👁' : '🙈'; @@ -661,7 +670,9 @@ function renderEntryPanel(): void { rows = sortEntries(rows, app.sortField, app.sortDir); if (app.entryView === 'table') { - listEl.appendChild(buildEntryTable(rows)); + const table = buildEntryTable(rows); + listEl.appendChild(table); + publishColumnWidths(table); } else { for (const { entry, group } of rows) { listEl.appendChild(buildEntryRow(entry, group)); @@ -739,7 +750,8 @@ browser already decides what counts as a click, so a scroll that starts on a cell copies nothing and a secondary button never reaches here at all. */ function wireCellCopy(cell: HTMLTableCellElement, value: string, label: string): void { cell.addEventListener('click', (event) => { - if (event.target !== event.currentTarget) return; // the copy button runs its own handler + // Anywhere but the copy button, which runs its own handler (#76). + if ((event.target as Element).closest('.copy-hint')) return; if (value) copyToClipboard(value, label); }); } @@ -752,14 +764,105 @@ function copyHint(value: string, label: string): HTMLButtonElement { }); } +/* The text ellipsizes inside its own box so the copy control keeps its place at +the cell's right edge (#76); appended straight after the text it rode past the +edge and out of sight the moment a value was longer than its column. */ function buildEntryCell(display: string, value: string, label: string): HTMLTableCellElement { const td = document.createElement('td'); - td.appendChild(document.createTextNode(display)); - if (value) td.appendChild(copyHint(value, label)); + const inner = document.createElement('div'); + inner.className = 'entry-cell'; + const text = document.createElement('span'); + text.className = 'entry-cell-text'; + text.textContent = display; + inner.appendChild(text); + if (value) inner.appendChild(copyHint(value, label)); + td.appendChild(inner); wireCellCopy(td, value, label); return td; } +const COLUMN_WIDTH_MIN = 80; +const COLUMN_WIDTH_MAX = 640; +const COLUMN_WIDTH_STEP = 16; + +/* Widths stay in memory for the same reason the group rail's does (#63, #77): +one silently restored forever would be state nobody asked to keep, yet it has +to outlive the table, which is rebuilt from scratch on every render. */ +const columnWidths = new Map(); + +// The column as rendered until the user picks a width; their choice after that (#77). +function currentColumnWidth(th: HTMLTableCellElement): number { + return columnWidths.get(must(th.dataset.column)) ?? th.getBoundingClientRect().width; +} + +function setColumnWidth(th: HTMLTableCellElement, px: number): void { + const width = Math.min(COLUMN_WIDTH_MAX, Math.max(COLUMN_WIDTH_MIN, Math.round(px))); + columnWidths.set(must(th.dataset.column), width); + th.style.width = `${width}px`; + must(th.querySelector('.col-resize')).setAttribute('aria-valuenow', String(width)); +} + +/* Pointer events rather than mouse events (#77), as the group rail does: one +path covers mouse, touch and pen, so a column resizes wherever it is visible. */ +function addColumnHandle(th: HTMLTableCellElement, label: string): void { + const handle = document.createElement('span'); + handle.className = 'col-resize'; + handle.tabIndex = 0; + handle.setAttribute('role', 'separator'); + handle.setAttribute('aria-orientation', 'vertical'); + handle.setAttribute('aria-label', `Resize ${label} column`); + handle.setAttribute('aria-valuemin', String(COLUMN_WIDTH_MIN)); + handle.setAttribute('aria-valuemax', String(COLUMN_WIDTH_MAX)); + + handle.addEventListener('pointerdown', (down) => { + down.preventDefault(); + handle.setPointerCapture(down.pointerId); + const startX = down.clientX; + const startWidth = currentColumnWidth(th); + + const onMove = (move: PointerEvent) => setColumnWidth(th, startWidth + move.clientX - startX); + const onDone = () => { + handle.releasePointerCapture(down.pointerId); + handle.removeEventListener('pointermove', onMove); + handle.removeEventListener('pointerup', onDone); + handle.removeEventListener('pointercancel', onDone); + }; + + handle.addEventListener('pointermove', onMove); + handle.addEventListener('pointerup', onDone); + handle.addEventListener('pointercancel', onDone); + }); + + handle.addEventListener('keydown', (key) => { + if (key.key !== 'ArrowLeft' && key.key !== 'ArrowRight') return; + key.preventDefault(); + const step = key.key === 'ArrowLeft' ? -COLUMN_WIDTH_STEP : COLUMN_WIDTH_STEP; + setColumnWidth(th, currentColumnWidth(th) + step); + }); + + th.appendChild(handle); +} + +function buildColumnHeader(id: string, label: string): HTMLTableCellElement { + const th = document.createElement('th'); + th.textContent = label; + th.dataset.column = id; + const chosen = columnWidths.get(id); + if (chosen !== undefined) th.style.width = `${chosen}px`; + addColumnHandle(th, label); + return th; +} + +/* A column only has a measurable width once the table is in the document, so +the starting value a screen reader reads out is published after it is attached +rather than while it is being built (#77). */ +function publishColumnWidths(table: HTMLTableElement): void { + for (const handle of table.querySelectorAll('.col-resize')) { + const th = must(handle.parentElement) as HTMLTableCellElement; + handle.setAttribute('aria-valuenow', String(Math.round(currentColumnWidth(th)))); + } +} + function buildEntryTable(rows: EntryWithGroup[]): HTMLTableElement { const table = document.createElement('table'); table.className = 'entry-table'; @@ -768,15 +871,13 @@ function buildEntryTable(rows: EntryWithGroup[]): HTMLTableElement { const thead = document.createElement('thead'); const headRow = document.createElement('tr'); - const titleTh = document.createElement('th'); - titleTh.textContent = 'Title'; - headRow.appendChild(titleTh); + headRow.appendChild(buildColumnHeader('title', 'Title')); for (const column of visibleColumns) { - const th = document.createElement('th'); - th.textContent = column.label; - headRow.appendChild(th); + headRow.appendChild(buildColumnHeader(column.key, column.label)); } + // Only wide enough for the row control, and not the user's to resize. const openTh = document.createElement('th'); + openTh.className = 'entry-table-open'; const openLabel = document.createElement('span'); openLabel.className = 'visually-hidden'; openLabel.textContent = 'Open'; @@ -995,6 +1096,25 @@ function wireEntryListEvents(): void { updateViewToggleUI(); } +/* What someone pressing find wants is an entry, not a word on the screen in +front of them, and the search field looks through the whole database rather +than the page (#78). The edit screen keeps the browser's own find: it holds +typing nobody has committed, and leaving would throw it away. */ +function startFind(): boolean { + if (app.db === null) return false; + // A dialog holds the keyboard while it is open, so the list behind it is not the subject. + if (document.querySelector('dialog[open]') !== null) return false; + if (document.querySelector('#root .screen-edit') !== null) return false; + if (document.querySelector('#root .screen-detail') !== null) { + app.currentEntry = null; + showEntryList(); + } + const search = qs('#search-input'); + search.focus(); + search.select(); + return true; +} + // ============================================================ // Screen: Entry Detail // ============================================================ @@ -1375,6 +1495,7 @@ function buildEditField( valueInput.type = valueInput.type === 'password' ? 'text' : 'password'; toggle.textContent = valueInput.type === 'password' ? '👁' : '🙈'; }); + keepFieldFocus(toggle); row.appendChild(toggle); } @@ -1383,6 +1504,7 @@ function buildEditField( // with — the user may have already edited it. copyToClipboard(valueInput.value, fieldLabel(keyInput.value) || 'Value'); }); + keepFieldFocus(copyBtn); row.appendChild(copyBtn); if (key === 'Password') { @@ -1391,6 +1513,7 @@ function buildEditField( valueInput.value = password; }); }); + keepFieldFocus(generateBtn); row.appendChild(generateBtn); } @@ -1948,6 +2071,7 @@ function openMoveToDialog( // app → host : kw-ready app booted, send a vault or a create instruction // host → app : kw-open open this vault (filename, bytes: ArrayBuffer) // host → app : kw-create start a brand-new, empty vault +// host → app : kw-find host saw the find keystroke; take it // app → host : kw-save user saved; please persist (filename, bytes: ArrayBuffer) // host → app : kw-saved result of that persist (ok, error?) // host → app : kw-close-request host wants to remove this iframe; may I? @@ -1975,17 +2099,16 @@ function postToHost(message: object): void { window.parent.postMessage(message, HOST_ORIGIN); } -/** The tab title belongs to whichever document owns the tab: the host when -embedded, this page when standalone. Every screen announces itself, so the -tab bar names the open database and its lock state without being opened (#65). */ +/** The tab belongs to whichever document owns it: the host when embedded, this +page when standalone. Every screen announces itself, so the tab bar names the +open database and shows its lock state without being opened (#65, #73). */ function publishTitle(): void { const locked = app.db === null; if (isEmbedded()) { postToHost(titleMessage(app.filename, locked)); return; } - const icon = locked ? '🔒' : '🔓'; - document.title = app.filename ? `${icon} ${app.filename} - ${BASE_TITLE}` : BASE_TITLE; + applyTabState(document, BASE_TITLE, app.filename, locked); } function handleHostMessage(event: MessageEvent): void { @@ -2004,6 +2127,9 @@ function handleHostMessage(event: MessageEvent): void { pendingSave = null; const { ok, error } = event.data; resolve?.(error === undefined ? { ok } : { ok, error }); + } else if (isFindMessage(event.data)) { + // The host already swallowed the keystroke, so its outcome is nothing to report. + startFind(); } else if (isCloseRequestMessage(event.data)) { confirmUnsavedChanges(DISCARD_PROMPT, () => postToHost(closeAckMessage()), CLOSE_PROMPT); } @@ -2043,3 +2169,10 @@ window.addEventListener('beforeunload', (e) => { }); document.addEventListener('visibilitychange', handleVisibilityChange); + +/* Only taken when there is something to search; on the upload, unlock and edit +screens the browser's own find is still the right answer (#78). */ +document.addEventListener('keydown', (e) => { + if (e.key !== 'f' || e.altKey || e.shiftKey || !(e.metaKey || e.ctrlKey)) return; + if (startFind()) e.preventDefault(); +}); diff --git a/pages/cloud-google-drive/bundle-iife.json b/pages/cloud-google-drive/bundle-iife.json index 05fe91d..ab10c96 100644 --- a/pages/cloud-google-drive/bundle-iife.json +++ b/pages/cloud-google-drive/bundle-iife.json @@ -4,7 +4,7 @@ "files": [ "router/src/index.js", "embed-protocol/src/index.js", - + "../pages/shared/logic.js", "../pages/cloud-google-drive/logic.js", "../pages/cloud-google-drive/page.js" ], @@ -23,6 +23,8 @@ "openMessage", "createMessage", "savedMessage", - "closeRequestMessage" + "closeRequestMessage", + "applyTabState", + "findMessage" ] } diff --git a/pages/cloud-google-drive/globals.d.ts b/pages/cloud-google-drive/globals.d.ts index 7977955..5d82748 100644 --- a/pages/cloud-google-drive/globals.d.ts +++ b/pages/cloud-google-drive/globals.d.ts @@ -141,8 +141,20 @@ interface GooglePicker { PickerBuilder: new () => PickerBuilderInstance; } +declare function applyTabState( + doc: Document, + baseTitle: string, + filename: string, + locked: boolean, +): void; + declare const gapi: GapiLoadable; declare const google: { picker: GooglePicker; accounts: { oauth2: GoogleOAuth2 }; }; + +interface FindMessage { + type: 'kw-find'; +} +declare function findMessage(): FindMessage; diff --git a/pages/cloud-google-drive/page.ts b/pages/cloud-google-drive/page.ts index c6da7ff..27bfffb 100644 --- a/pages/cloud-google-drive/page.ts +++ b/pages/cloud-google-drive/page.ts @@ -257,14 +257,33 @@ function embedApp(headerLabel: string, implementation: string): void { requestCloseIframe(tearDownIframe); }); window.addEventListener('message', handleFrameMessage); + window.addEventListener('keydown', handleFindKey); // Setting src last means the iframe's script (and its kw-ready handshake) // can't fire before the listener above is attached. qs('#app-frame').src = implementation; } +/* The app reports its lock state for the tab (#65); a find is only worth +forwarding once it is unlocked and has entries to look through (#78). */ +let appUnlocked = false; + +/* Focus outside the iframe means this document gets the keystroke, so the find +is forwarded rather than left to the browser's own, which can only see this +page's chrome (#78). */ +function handleFindKey(event: KeyboardEvent): void { + if (event.key !== 'f' || event.altKey || event.shiftKey || !(event.metaKey || event.ctrlKey)) { + return; + } + if (!appUnlocked) return; + event.preventDefault(); + must(qs('#app-frame').contentWindow).postMessage(findMessage(), APP_ORIGIN); +} + function tearDownIframe(): void { window.removeEventListener('message', handleFrameMessage); - document.title = BASE_TITLE; + window.removeEventListener('keydown', handleFindKey); + appUnlocked = false; + applyTabState(document, BASE_TITLE, '', true); currentFile = null; pendingAction = null; showChooser(); @@ -304,9 +323,8 @@ function handleFrameMessage(event: MessageEvent): void { void createFileOnDrive(event.data.filename, event.data.bytes, source); } } else if (isTitleMessage(event.data)) { - const { filename, locked } = event.data; - const icon = locked ? '🔒' : '🔓'; - document.title = filename ? `${icon} ${filename} - ${BASE_TITLE}` : BASE_TITLE; + appUnlocked = !event.data.locked; + applyTabState(document, BASE_TITLE, event.data.filename, event.data.locked); } else if (isCloseAckMessage(event.data)) { const afterClose = pendingClose; pendingClose = null; diff --git a/pages/local/bundle-iife.json b/pages/local/bundle-iife.json index 7bbec10..59338a7 100644 --- a/pages/local/bundle-iife.json +++ b/pages/local/bundle-iife.json @@ -4,7 +4,7 @@ "files": [ "router/src/index.js", "embed-protocol/src/index.js", - + "../pages/shared/logic.js", "../pages/local/logic.js", "../pages/local/page.js" ], @@ -19,6 +19,8 @@ "createMessage", "savedMessage", "closeRequestMessage", - "must" + "must", + "applyTabState", + "findMessage" ] } diff --git a/pages/local/globals.d.ts b/pages/local/globals.d.ts index 12b1546..6c9b173 100644 --- a/pages/local/globals.d.ts +++ b/pages/local/globals.d.ts @@ -66,3 +66,15 @@ declare function savedMessage(ok: boolean, error?: string): SavedMessage; declare function closeRequestMessage(): CloseRequestMessage; declare function must(value: T | null | undefined): T; + +declare function applyTabState( + doc: Document, + baseTitle: string, + filename: string, + locked: boolean, +): void; + +interface FindMessage { + type: 'kw-find'; +} +declare function findMessage(): FindMessage; diff --git a/pages/local/page.ts b/pages/local/page.ts index 542e520..8fb202d 100644 --- a/pages/local/page.ts +++ b/pages/local/page.ts @@ -124,13 +124,32 @@ function embedApp(headerLabel: string, implementation: string): void { requestCloseIframe(tearDownIframe); }); window.addEventListener('message', handleFrameMessage); + window.addEventListener('keydown', handleFindKey); // src is set last so the iframe's kw-ready can't fire before the listener attaches. qs('#app-frame').src = implementation; } +/* The app reports its lock state for the tab (#65); a find is only worth +forwarding once it is unlocked and has entries to look through (#78). */ +let appUnlocked = false; + +/* Focus outside the iframe means this document gets the keystroke, so the find +is forwarded rather than left to the browser's own, which can only see this +page's chrome (#78). */ +function handleFindKey(event: KeyboardEvent): void { + if (event.key !== 'f' || event.altKey || event.shiftKey || !(event.metaKey || event.ctrlKey)) { + return; + } + if (!appUnlocked) return; + event.preventDefault(); + must(qs('#app-frame').contentWindow).postMessage(findMessage(), APP_ORIGIN); +} + function tearDownIframe(): void { window.removeEventListener('message', handleFrameMessage); - document.title = BASE_TITLE; + window.removeEventListener('keydown', handleFindKey); + appUnlocked = false; + applyTabState(document, BASE_TITLE, '', true); pendingAction = null; showChooser(); } @@ -160,9 +179,8 @@ function handleFrameMessage(event: MessageEvent): void { qs('#host-filename').textContent = event.data.filename; downloadAndAck(event.data.filename, event.data.bytes, source); } else if (isTitleMessage(event.data)) { - const { filename, locked } = event.data; - const icon = locked ? '🔒' : '🔓'; - document.title = filename ? `${icon} ${filename} - ${BASE_TITLE}` : BASE_TITLE; + appUnlocked = !event.data.locked; + applyTabState(document, BASE_TITLE, event.data.filename, event.data.locked); } else if (isCloseAckMessage(event.data)) { const afterClose = pendingClose; pendingClose = null; diff --git a/pages/package.json b/pages/package.json index f24ac6f..8d3d5af 100644 --- a/pages/package.json +++ b/pages/package.json @@ -6,7 +6,7 @@ "description": "KeePass Web — the browser app itself, built into self-contained HTML distributables.", "scripts": { "typecheck": "tsc --noEmit", - "test": "node --experimental-strip-types --experimental-test-coverage --test-coverage-lines=100 --test-coverage-branches=100 --test-coverage-functions=100 --test-coverage-include='0x67/**/*.ts' --test-coverage-include='index/**/*.ts' --test-coverage-include='local/**/*.ts' --test-coverage-include='cloud-google-drive/**/*.ts' --test-coverage-exclude='**/*.d.ts' --test 'tests/**/*.test.ts'", + "test": "node --experimental-strip-types --experimental-test-coverage --test-coverage-lines=100 --test-coverage-branches=100 --test-coverage-functions=100 --test-coverage-include='0x67/**/*.ts' --test-coverage-include='shared/**/*.ts' --test-coverage-include='index/**/*.ts' --test-coverage-include='local/**/*.ts' --test-coverage-include='cloud-google-drive/**/*.ts' --test-coverage-exclude='**/*.d.ts' --test 'tests/**/*.test.ts'", "build": "tsc --project tsconfig.build.json && node --experimental-strip-types ../tools/build/bundle-iife/src/index.ts 0x67/bundle-iife.json && node --experimental-strip-types ../tools/build/bundle-iife/src/index.ts local/bundle-iife.json && node --experimental-strip-types ../tools/build/bundle-iife/src/index.ts cloud-google-drive/bundle-iife.json && node --experimental-strip-types ../tools/build/bundle-iife/src/index.ts index/bundle-iife.json && node --experimental-strip-types ../tools/build/inliner/src/index.ts 0x67/build.json && node --experimental-strip-types ../tools/build/inliner/src/index.ts local/build.json && node --experimental-strip-types ../tools/build/inliner/src/index.ts index/build.json && node --experimental-strip-types ../tools/build/inliner/src/index.ts cloud-google-drive/build.json" }, "devDependencies": { diff --git a/pages/shared/logic.ts b/pages/shared/logic.ts new file mode 100644 index 0000000..4ca216c --- /dev/null +++ b/pages/shared/logic.ts @@ -0,0 +1,44 @@ +/** Tab chrome for every page that can hold a database: the name a tab carries +and the icon it shows. The document that owns the tab is not always the one +that knows the state — an embedded app reports it back over embed-protocol — +so the connector pages and the app itself both end up here, and the icons are +defined once instead of once per page. */ + +/* Hue and silhouette rather than an open versus closed shackle (#73): a tab +gives an icon 16 pixels, where a shackle's gap is about two of them and the two +states are indistinguishable. Unlocked is a different color and shows rows, +because rows are what clicking that tab is about to put on screen. */ +const LOCKED_ICON = + "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16'%3E%3Crect width='16' height='16' rx='3.4' fill='%230e7c5a'/%3E%3Cpath d='M6 8.4V7a2 2 0 0 1 4 0v1.4' fill='none' stroke='%23fff' stroke-width='1.35' stroke-linecap='round'/%3E%3Crect x='4.5' y='8.4' width='7' height='4.5' rx='1.1' fill='%23fff'/%3E%3C/svg%3E"; +const UNLOCKED_ICON = + "data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16'%3E%3Crect width='16' height='16' rx='3.4' fill='%238a5a1e'/%3E%3Crect x='3.7' y='4.4' width='8.6' height='1.9' rx='.95' fill='%23fff'/%3E%3Crect x='3.7' y='7.05' width='8.6' height='1.9' rx='.95' fill='%23fff'/%3E%3Crect x='3.7' y='9.7' width='8.6' height='1.9' rx='.95' fill='%23fff'/%3E%3C/svg%3E"; + +/** The tab's name. It spells the state out as well as showing it, because 🔒 +and 🔓 are as hard to tell apart in a title as they are in a tab (#73). */ +export function tabTitle(baseTitle: string, filename: string, locked: boolean): string { + if (!filename) return baseTitle; + return `${locked ? '🔒' : '🔓'} ${filename} - ${locked ? 'Locked' : 'Unlocked'} - ${baseTitle}`; +} + +/** Name the tab and mark it with the database's state; no filename means no +database, which leaves the page's own icon alone. */ +export function applyTabState( + doc: Document, + baseTitle: string, + filename: string, + locked: boolean, +): void { + doc.title = tabTitle(baseTitle, filename, locked); + + const link = doc.querySelector('link[rel="icon"]'); + if (!link) return; + + /* Kept on the element rather than in this module, so it is captured once per + document and before anything overwrites it: giving a database up has to hand + the tab back the icon its own page shipped with (#73). */ + if (link.dataset.pageIcon === undefined) link.dataset.pageIcon = link.getAttribute('href') ?? ''; + + let icon = link.dataset.pageIcon; + if (filename) icon = locked ? LOCKED_ICON : UNLOCKED_ICON; + link.setAttribute('href', icon); +} diff --git a/pages/tests/0x67-host.test.ts b/pages/tests/0x67-host.test.ts index 9213ba1..1a93104 100644 --- a/pages/tests/0x67-host.test.ts +++ b/pages/tests/0x67-host.test.ts @@ -50,6 +50,7 @@ import { touchLastModified, } from '../../packages/kdbx/src/index.ts'; import * as logic from '../0x67/logic.ts'; +import { applyTabState } from '../shared/logic.ts'; // ============================================================ // jsdom environment with a mocked parent frame @@ -96,6 +97,7 @@ dom.window.HTMLDialogElement.prototype.close = function (this: HTMLDialogElement }; Object.assign(globalThis, { + applyTabState, Kdbx, Credentials, getChildren, @@ -539,3 +541,27 @@ test('0x67 embedded in a host frame: choosing Save from the unsaved-changes prom }, ); }); + +test('0x67 embedded in a host frame: the host forwards the find keystroke', async (t) => { + await t.test('kw-find puts the caret in the search field', async () => { + // Focus outside the iframe means the host is the one that receives the + // keystroke, so what crosses to the app is the message, not the key (#78). + sendFromHost({ + type: 'kw-open', + filename: 'find-me.kdbx', + bytes: new Uint8Array(dbBytes).buffer, + }); + await waitFor(() => q('#master-password') !== null); + q('#master-password').value = PASSWORD; + q('#unlock-form').dispatchEvent( + new dom.window.Event('submit', { bubbles: true, cancelable: true }), + ); + await waitFor(() => q('#search-input') !== null); + + q('#search-input').blur(); + assert.notEqual(dom.window.document.activeElement, q('#search-input')); + + sendFromHost({ type: 'kw-find' }); + assert.equal(dom.window.document.activeElement, q('#search-input')); + }); +}); diff --git a/pages/tests/0x67-mobile.test.ts b/pages/tests/0x67-mobile.test.ts index ad393f6..24af468 100644 --- a/pages/tests/0x67-mobile.test.ts +++ b/pages/tests/0x67-mobile.test.ts @@ -37,6 +37,7 @@ import { setText, } from '../../packages/kdbx/src/index.ts'; import * as logic from '../0x67/logic.ts'; +import { applyTabState } from '../shared/logic.ts'; // ============================================================ // jsdom environment at a phone-width viewport @@ -75,6 +76,7 @@ dom.window.HTMLDialogElement.prototype.close = function (this: HTMLDialogElement }; Object.assign(globalThis, { + applyTabState, Kdbx, Credentials, getChildren, diff --git a/pages/tests/0x67-page.test.ts b/pages/tests/0x67-page.test.ts index ca5581a..11fd0f5 100644 --- a/pages/tests/0x67-page.test.ts +++ b/pages/tests/0x67-page.test.ts @@ -59,6 +59,7 @@ import { } from '../../packages/kdbx/src/index.ts'; import type { generatePassword } from '../0x67/logic.ts'; import * as logic from '../0x67/logic.ts'; +import { applyTabState } from '../shared/logic.ts'; // ============================================================ // jsdom environment, built from the real page.html @@ -142,6 +143,7 @@ dom.window.HTMLElement.prototype.releasePointerCapture = () => {}; // --- exactly like bundle.js does in the real browser build (see // --- bundle-iife.json's "exports" list, which this mirrors exactly). --- Object.assign(globalThis, { + applyTabState, Kdbx, Credentials, getChildren, @@ -313,7 +315,7 @@ test('0x67 app', async (t) => { await waitFor(() => q('#master-password') !== null); assert.equal(q('#db-filename').textContent, 'dropped.kdbx'); - assert.equal(dom.window.document.title, '🔒 dropped.kdbx - KeePass Web'); + assert.equal(dom.window.document.title, '🔒 dropped.kdbx - Locked - KeePass Web'); }); await t.test('unlock screen "back" returns to upload and clears the file', () => { @@ -422,13 +424,22 @@ test('0x67 app', async (t) => { assert.equal(passwordInput.type, 'password'); }); + // jsdom never moves focus on a press, so the observable here is that the + // handler cancels it; e2e/reveal-focus.test.ts proves the real outcome. + await t.test('revealing the master password does not steal the caret', () => { + assert.ok( + dispatch(q('[data-action="toggle-password"]'), 'mousedown').defaultPrevented, + 'the press that would move focus to the button is suppressed', + ); + }); + await t.test('the correct password and key file unlock into the entry list', async () => { q('#master-password').value = PASSWORD; dispatch(q('#unlock-form'), 'submit'); await waitFor(() => dom.window.document.body.classList.contains('app-mode')); assert.ok(q('#group-tree').querySelector('.group-btn')); - assert.equal(dom.window.document.title, '🔓 real.kdbx - KeePass Web'); + assert.equal(dom.window.document.title, '🔓 real.kdbx - Unlocked - KeePass Web'); // Table view is the default. assert.equal(root().querySelectorAll('.entry-table').length, 1); // Switch to tile view, which the rest of this suite's entry-list @@ -1188,6 +1199,21 @@ test('0x67 app', async (t) => { }, ); + await t.test('no control in the edit row takes the caret out of the value field', () => { + q('[data-action="edit"]').dispatchEvent(new dom.window.Event('click', { bubbles: true })); + const passwordRow = Array.from(root().querySelectorAll('.edit-field')).find( + (row) => row.querySelector('.edit-key')?.value === 'Password', + ) as HTMLElement; + + for (const title of ['Show / hide', 'Copy', 'Generate password']) { + const control = passwordRow.querySelector(`[title="${title}"]`); + assert.ok(control, `the password row carries a "${title}" control`); + assert.ok(dispatch(control, 'mousedown').defaultPrevented, `"${title}" keeps the caret put`); + } + + q('[data-action="cancel"]').dispatchEvent(new dom.window.Event('click', { bubbles: true })); + }); + await t.test('editing an existing entry (not new) cancels back to the detail screen', () => { q('[data-action="edit"]').dispatchEvent(new dom.window.Event('click', { bubbles: true })); assert.equal(q('#edit-title').textContent, 'Edit Entry'); @@ -1610,7 +1636,7 @@ test('0x67 app', async (t) => { assert.equal(lockDlg.open, false); await waitFor(() => q('#master-password') !== null); assert.equal(q('#db-filename').textContent, 'real.kdbx'); - assert.equal(dom.window.document.title, '🔒 real.kdbx - KeePass Web'); + assert.equal(dom.window.document.title, '🔒 real.kdbx - Locked - KeePass Web'); // A wrong password on the relocked (freshly re-encrypted) state is // still rejected — locking doesn't weaken the credential check. @@ -1863,7 +1889,7 @@ test('a tab left hidden locks itself, and coming back in time calls it off', asy assert.equal(q('#db-filename').textContent, 'auto-lock.kdbx'); assert.equal( dom.window.document.title, - '🔒 auto-lock.kdbx - KeePass Web', + '🔒 auto-lock.kdbx - Locked - KeePass Web', 'the tab bar says so without being opened', ); @@ -2270,6 +2296,10 @@ test('entry list table view: columns, masked password, click to copy, button to appendChild(rootGroup, createEntry({ title: 'No Password', username: 'bare' })); const bytes = await kdbx.save(); + const findKey = (init: Record = { key: 'f', metaKey: true }): Event => + dispatch(dom.window.document, 'keydown', init); + assert.equal(findKey().defaultPrevented, false, 'no database, so no in-app find (#78)'); + const fileInput = q('#file-input'); setFiles(fileInput, [makeFile('views.kdbx', bytes)]); dispatch(fileInput, 'change'); @@ -2295,13 +2325,63 @@ test('entry list table view: columns, masked password, click to copy, button to "default visible columns, plus the open column's screen-reader-only name", ); - // The cell's own text, without the copy hint appended beside it. + // Columns resize the way the group rail does. jsdom has no layout engine, so + // every drag starts from a zero-width column; that still proves the + // arithmetic, the clamps, and that a width survives the table being rebuilt. + const urlTh = (): HTMLElement => q('th[data-column="url"]'); + const urlHandle = (): HTMLElement => q('th[data-column="url"] .col-resize'); + assert.equal(urlHandle().getAttribute('aria-label'), 'Resize URL column'); + + dispatch(urlHandle(), 'pointerdown', { clientX: 100, pointerId: 1 }); + dispatch(urlHandle(), 'pointermove', { clientX: 420, pointerId: 1 }); + assert.equal(urlTh().style.width, '320px'); + assert.equal(urlHandle().getAttribute('aria-valuenow'), '320'); + + dispatch(urlHandle(), 'pointermove', { clientX: 0, pointerId: 1 }); + assert.equal(urlTh().style.width, '80px', 'clamped at the narrow end'); + dispatch(urlHandle(), 'pointermove', { clientX: 9000, pointerId: 1 }); + assert.equal(urlTh().style.width, '640px', 'clamped at the wide end'); + + dispatch(urlHandle(), 'pointerup', { clientX: 9000, pointerId: 1 }); + dispatch(urlHandle(), 'pointermove', { clientX: 250, pointerId: 1 }); + assert.equal(urlTh().style.width, '640px', 'releasing stops tracking the pointer'); + + dispatch(urlHandle(), 'keydown', { key: 'ArrowLeft' }); + assert.equal(urlTh().style.width, '624px', 'one step narrower than the 640 just dragged to'); + dispatch(urlHandle(), 'keydown', { key: 'a' }); + assert.equal(urlTh().style.width, '624px', 'unchanged by a non-arrow key'); + dispatch(urlHandle(), 'keydown', { key: 'ArrowRight' }); + assert.equal(urlTh().style.width, '640px', 'one step wider, back at the ceiling'); + + dispatch(q('[data-action="view-tile"]'), 'click'); + dispatch(q('[data-action="view-table"]'), 'click'); + assert.equal(urlTh().style.width, '640px', 'the rebuilt column kept its width'); + assert.equal(urlHandle().getAttribute('aria-valuenow'), '640'); + + // A column nobody has touched carries no width of its own and reports what it + // was laid out at, which under jsdom is zero. + assert.equal(q('th[data-column="title"]').style.width, ''); + assert.equal( + q('th[data-column="title"] .col-resize').getAttribute('aria-valuenow'), + '0', + ); + + // The cell's own text, without the copy control beside it (#76). const bodyCells = (): string[] => - Array.from(root().querySelectorAll('.entry-table tbody td')).map( - (td) => td.firstChild?.textContent ?? '', + Array.from(root().querySelectorAll('.entry-table tbody .entry-cell-text')).map( + (text) => text.textContent ?? '', ); const [titleCell, usernameCell, passwordCell, urlCell] = bodyCells(); assert.ok(titleCell?.includes('GitHub')); + + // The copy control is the cell's own trailing element rather than something + // glued to the end of the text, which is what keeps it in view (#76). + const firstCell = q('.entry-table tbody td'); + assert.equal( + firstCell.querySelector('.entry-cell')?.lastElementChild?.className, + 'copy-hint', + 'it comes after the text box, as a sibling of it', + ); assert.equal(usernameCell, 'octocat'); assert.equal(passwordCell, '••••••••', 'password is masked on screen'); assert.equal(urlCell, 'https://github.com'); @@ -2330,7 +2410,7 @@ test('entry list table view: columns, masked password, click to copy, button to clipboardWritesShouldFail = false; const maskedCell = (): HTMLElement => Array.from(root().querySelectorAll('.entry-table tbody td')).find( - (td) => td.firstChild?.textContent === '••••••••', + (td) => td.querySelector('.entry-cell-text')?.textContent === '••••••••', ) as HTMLElement; // A click copies the real value, names it, and leaves the card shut. @@ -2353,12 +2433,46 @@ test('entry list table view: columns, masked password, click to copy, button to await Promise.resolve(); assert.equal(clipboardText, 'hunter2', 'the copy button copies exactly once'); + // Find looks through the database rather than the page, so it takes the + // keystroke and puts the caret in the search field with whatever was already + // typed selected, ready to be replaced (#78). + q('#search-input').value = 'stale'; + assert.equal(findKey().defaultPrevented, true); + assert.equal(dom.window.document.activeElement, q('#search-input'), 'the caret moved there'); + assert.equal(q('#search-input').selectionStart, 0, 'over the whole value'); + assert.equal(q('#search-input').selectionEnd, 'stale'.length); + + assert.equal(findKey({ key: 'f', ctrlKey: true }).defaultPrevented, true, 'ctrl works too'); + assert.equal(findKey({ key: 'g', metaKey: true }).defaultPrevented, false); + assert.equal(findKey({ key: 'f' }).defaultPrevented, false, 'unmodified f is just typing'); + assert.equal(findKey({ key: 'f', metaKey: true, shiftKey: true }).defaultPrevented, false); + assert.equal(findKey({ key: 'f', ctrlKey: true, altKey: true }).defaultPrevented, false); + q('#search-input').value = ''; + + // A modal has the keyboard; the list behind it is not what find is about. + dispatch(q('[data-action="settings"]'), 'click'); + assert.equal(findKey().defaultPrevented, false, 'the dialog keeps the keystroke'); + dispatch(dq('#dlg-settings [data-action="close"]'), 'click'); + assert.equal(findKey().defaultPrevented, true, 'and find works again once it is gone'); + // Opening the card is its own button, never a gesture over the values. const openBtn = (q('.entry-table tbody tr') as HTMLElement).querySelector( '.entry-table-open button', ) as HTMLButtonElement; dispatch(openBtn, 'click'); assert.ok(q('#detail-title')?.textContent?.includes('GitHub'), 'the › opens it'); + + // The card is read-only, so find can leave it and go back to the list (#78). + assert.equal(findKey().defaultPrevented, true); + assert.equal(q('#detail-title'), null, 'the card gave way to the list'); + assert.equal(dom.window.document.activeElement, q('#search-input')); + + // The edit screen holds typing nobody has committed, so find stays out of it. + dispatch(q('.entry-table tbody .entry-table-open button'), 'click'); + dispatch(q('[data-action="edit"]'), 'click'); + assert.equal(findKey().defaultPrevented, false, 'the browser keeps its own find here'); + assert.ok(q('#edit-title'), 'and the edit screen is still the one showing'); + dispatch(q('[data-action="cancel"]'), 'click'); dispatch(q('[data-action="back"]'), 'click'); dispatch(q('[data-action="view-table"]'), 'click'); @@ -2384,7 +2498,11 @@ test('entry list table view: columns, masked password, click to copy, button to const attachmentsTd = (q('.entry-table tbody tr') as HTMLElement).querySelectorAll('td')[ columnIndex ] as HTMLElement; - assert.equal(attachmentsTd.firstChild?.textContent, '', 'no attachments on this entry'); + assert.equal( + attachmentsTd.querySelector('.entry-cell-text')?.textContent, + '', + 'no attachments on this entry', + ); assert.equal(attachmentsTd.querySelector('.copy-hint'), null, 'and so no copy hint'); clipboardText = ''; dispatch(attachmentsTd, 'click'); diff --git a/pages/tests/cloud-google-drive-page.test.ts b/pages/tests/cloud-google-drive-page.test.ts index 89346b9..539140e 100644 --- a/pages/tests/cloud-google-drive-page.test.ts +++ b/pages/tests/cloud-google-drive-page.test.ts @@ -17,6 +17,7 @@ import { JSDOM } from 'jsdom'; import * as embedProtocol from '../../packages/embed-protocol/src/index.ts'; import { identifyFormat } from '../../packages/router/src/index.ts'; import * as logic from '../cloud-google-drive/logic.ts'; +import * as shared from '../shared/logic.ts'; // ============================================================ // Environment @@ -141,7 +142,7 @@ Object.defineProperty(globalThis, 'google', { }); Object.defineProperty(globalThis, 'gapi', { value: gapiMock, configurable: true, writable: true }); -Object.assign(globalThis, { identifyFormat, ...embedProtocol, ...logic }); +Object.assign(globalThis, { identifyFormat, ...embedProtocol, ...logic, ...shared }); await import('../cloud-google-drive/page.ts'); @@ -391,16 +392,47 @@ test('Google Drive connector', async (t) => { await t.test('kw-title names the open database in the tab', () => { sendMessage({ type: 'kw-title', filename: 'vault.kdbx', locked: true }, { source: frameWin }); - assert.equal(doc.title, '🔒 vault.kdbx - KeePass Web - Google Drive'); + assert.equal(doc.title, '🔒 vault.kdbx - Locked - KeePass Web - Google Drive'); sendMessage({ type: 'kw-title', filename: 'vault.kdbx', locked: false }, { source: frameWin }); - assert.equal(doc.title, '🔓 vault.kdbx - KeePass Web - Google Drive'); + assert.equal(doc.title, '🔓 vault.kdbx - Unlocked - KeePass Web - Google Drive'); // An app with nothing open reports no filename, leaving this page's own title. sendMessage({ type: 'kw-title', filename: '', locked: true }, { source: frameWin }); assert.equal(doc.title, 'KeePass Web - Google Drive'); }); + await t.test('the find keystroke reaches the app, but only once it is unlocked', () => { + const findKey = (init: Record = { key: 'f', metaKey: true }): Event => { + const evt = new dom.window.Event('keydown', { bubbles: true, cancelable: true }); + Object.assign(evt, init); + dom.window.dispatchEvent(evt); + return evt; + }; + + // Locked, so there is nothing to search and the browser keeps its own find. + sendMessage({ type: 'kw-title', filename: 'vault.kdbx', locked: true }, { source: frameWin }); + const whileLocked = frameInbox.length; + assert.equal(findKey().defaultPrevented, false); + assert.equal(frameInbox.length, whileLocked, 'nothing forwarded'); + + sendMessage({ type: 'kw-title', filename: 'vault.kdbx', locked: false }, { source: frameWin }); + assert.equal(findKey().defaultPrevented, true); + assert.deepEqual(frameInbox.at(-1)?.message, { type: 'kw-find' }); + + // Ctrl for everyone who is not on a Mac. + assert.equal(findKey({ key: 'f', ctrlKey: true }).defaultPrevented, true); + assert.deepEqual(frameInbox.at(-1)?.message, { type: 'kw-find' }); + + // Nothing else is that chord, so nothing else is taken. + const taken = frameInbox.length; + assert.equal(findKey({ key: 'g', metaKey: true }).defaultPrevented, false); + assert.equal(findKey({ key: 'f' }).defaultPrevented, false); + assert.equal(findKey({ key: 'f', metaKey: true, shiftKey: true }).defaultPrevented, false); + assert.equal(findKey({ key: 'f', ctrlKey: true, altKey: true }).defaultPrevented, false); + assert.equal(frameInbox.length, taken, 'and nothing more was forwarded'); + }); + await t.test('a stray kw-close-ack with nothing pending is a harmless no-op', () => { const before = frameInbox.length; sendMessage({ type: 'kw-close-ack' }, { source: frameWin }); diff --git a/pages/tests/local-page.test.ts b/pages/tests/local-page.test.ts index 8eb3c57..7c2766a 100644 --- a/pages/tests/local-page.test.ts +++ b/pages/tests/local-page.test.ts @@ -16,6 +16,7 @@ import { JSDOM } from 'jsdom'; import * as embedProtocol from '../../packages/embed-protocol/src/index.ts'; import { identifyFormat } from '../../packages/router/src/index.ts'; import { must } from '../local/logic.ts'; +import { applyTabState } from '../shared/logic.ts'; const htmlPath = fileURLToPath(new URL('../local/page.html', import.meta.url)); const html = readFileSync(htmlPath, 'utf8'); @@ -41,7 +42,7 @@ dom.window.HTMLAnchorElement.prototype.click = function (this: HTMLAnchorElement downloadNames.push(this.download); }; -Object.assign(globalThis, { identifyFormat, ...embedProtocol, must }); +Object.assign(globalThis, { identifyFormat, ...embedProtocol, must, applyTabState }); await import('../local/page.ts'); @@ -217,16 +218,43 @@ test('local file connector', async (t) => { await t.test('kw-title names the open database in the tab', () => { sendMessage({ type: 'kw-title', filename: 'vault.kdbx', locked: true }, { source: frameWin }); - assert.equal(doc.title, '🔒 vault.kdbx - KeePass Web - Local file'); + assert.equal(doc.title, '🔒 vault.kdbx - Locked - KeePass Web - Local file'); sendMessage({ type: 'kw-title', filename: 'vault.kdbx', locked: false }, { source: frameWin }); - assert.equal(doc.title, '🔓 vault.kdbx - KeePass Web - Local file'); + assert.equal(doc.title, '🔓 vault.kdbx - Unlocked - KeePass Web - Local file'); // An app with nothing open reports no filename, leaving this page's own title. sendMessage({ type: 'kw-title', filename: '', locked: true }, { source: frameWin }); assert.equal(doc.title, 'KeePass Web - Local file'); }); + await t.test('the find keystroke reaches the app, but only once it is unlocked', () => { + const findKey = (init: Record = { key: 'f', metaKey: true }): Event => + dispatch(dom.window, 'keydown', init); + + // Locked, so there is nothing to search and the browser keeps its own find. + sendMessage({ type: 'kw-title', filename: 'vault.kdbx', locked: true }, { source: frameWin }); + const whileLocked = frameInbox.length; + assert.equal(findKey().defaultPrevented, false); + assert.equal(frameInbox.length, whileLocked, 'nothing forwarded'); + + sendMessage({ type: 'kw-title', filename: 'vault.kdbx', locked: false }, { source: frameWin }); + assert.equal(findKey().defaultPrevented, true); + assert.deepEqual(frameInbox.at(-1)?.message, { type: 'kw-find' }); + + // Ctrl for everyone who is not on a Mac. + assert.equal(findKey({ key: 'f', ctrlKey: true }).defaultPrevented, true); + assert.deepEqual(frameInbox.at(-1)?.message, { type: 'kw-find' }); + + // Nothing else is that chord, so nothing else is taken. + const taken = frameInbox.length; + assert.equal(findKey({ key: 'g', metaKey: true }).defaultPrevented, false); + assert.equal(findKey({ key: 'f' }).defaultPrevented, false); + assert.equal(findKey({ key: 'f', metaKey: true, shiftKey: true }).defaultPrevented, false); + assert.equal(findKey({ key: 'f', ctrlKey: true, altKey: true }).defaultPrevented, false); + assert.equal(frameInbox.length, taken, 'and nothing more was forwarded'); + }); + await t.test('a stray kw-close-ack with nothing pending is a harmless no-op', () => { const before = frameInbox.length; sendMessage({ type: 'kw-close-ack' }, { source: frameWin }); diff --git a/pages/tests/shared-logic.test.ts b/pages/tests/shared-logic.test.ts new file mode 100644 index 0000000..56ffa8f --- /dev/null +++ b/pages/tests/shared-logic.test.ts @@ -0,0 +1,77 @@ +/** + * Tests for shared/logic.ts, the tab chrome every page that can hold a + * database goes through (issue #73). It touches a Document but never reaches + * for a global one, so a plain jsdom document passed in is enough — no page + * markup, no bundle, no boot sequence. + */ + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { JSDOM } from 'jsdom'; +import { applyTabState, tabTitle } from '../shared/logic.ts'; + +const PAGE_ICON = 'data:image/svg+xml,%3Csvg/%3E'; + +function pageDocument(withIcon = true): Document { + const icon = withIcon ? `` : ''; + const dom = new JSDOM( + `${icon}Base`, + ); + return dom.window.document as unknown as Document; +} + +const iconHref = (doc: Document): string => + doc.querySelector('link[rel="icon"]')?.getAttribute('href') ?? ''; + +test('the title names the database and spells its state out', () => { + assert.equal(tabTitle('KeePass Web', 'vault.kdbx', true), '🔒 vault.kdbx - Locked - KeePass Web'); + assert.equal( + tabTitle('KeePass Web', 'vault.kdbx', false), + '🔓 vault.kdbx - Unlocked - KeePass Web', + ); +}); + +test('no database means the page keeps its own name', () => { + assert.equal(tabTitle('KeePass Web - Local file', '', true), 'KeePass Web - Local file'); + assert.equal(tabTitle('KeePass Web - Local file', '', false), 'KeePass Web - Local file'); +}); + +test('the icon tracks the lock state and hands the page its own back', () => { + const doc = pageDocument(); + + applyTabState(doc, 'Base', 'vault.kdbx', true); + const locked = iconHref(doc); + assert.equal(doc.title, '🔒 vault.kdbx - Locked - Base'); + assert.notEqual(locked, PAGE_ICON, 'a held database is not the page at rest'); + + applyTabState(doc, 'Base', 'vault.kdbx', false); + const unlocked = iconHref(doc); + assert.equal(doc.title, '🔓 vault.kdbx - Unlocked - Base'); + assert.notEqual(unlocked, locked, 'and the two states are not the same icon'); + + // Hue and glyph are what carry at 16px, so the two must differ in both. + assert.ok(locked.includes('%230e7c5a') && locked.includes('M6 8.4V7'), 'accent, with a padlock'); + assert.ok(unlocked.includes('%238a5a1e') && !unlocked.includes('M6 8.4V7'), 'warning, with rows'); + + applyTabState(doc, 'Base', '', true); + assert.equal(doc.title, 'Base'); + assert.equal(iconHref(doc), PAGE_ICON, 'closing gives the page its own icon back'); +}); + +test('a page whose icon link carries no href still gets one back', () => { + const dom = new JSDOM(''); + const doc = dom.window.document as unknown as Document; + + applyTabState(doc, 'Base', 'vault.kdbx', true); + assert.notEqual(iconHref(doc), '', 'the locked icon still goes on'); + + applyTabState(doc, 'Base', '', true); + assert.equal(iconHref(doc), '', 'and it comes back off, leaving nothing behind'); +}); + +test('a page with no icon link is titled anyway, not crashed', () => { + const doc = pageDocument(false); + applyTabState(doc, 'Base', 'vault.kdbx', true); + assert.equal(doc.title, '🔒 vault.kdbx - Locked - Base'); + assert.equal(doc.querySelector('link[rel="icon"]'), null); +});