From 6ba9ccfa4cb84f605e2548bf2934d50dc591a3ee Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Fri, 24 Apr 2026 18:51:23 -0400 Subject: [PATCH] fix(ext/popup): preserve unsupported-kind fields + totp expanded state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes from the T3+T4 code review: C1 (Critical): renderSectionBlock previously rendered all fields regardless of kind. For fields with kind url/date/month_year/totp/etc. (from CLI-created items), the editor showed a blank value input; if the user typed anything, the input handler cast the kind to the wrong thing and silently overwrote the structured value with a string — destroying data. Fix: filter editor to supported kinds (text/password/concealed); key data-* attributes by field.id (not by index) so handlers look up the correct field regardless of what the render loop emitted. Unsupported-kind fields survive save untouched. A small muted note "N fields of unsupported kind (edit via CLI)" flags preserved entries. +2 tests. I1 (Important): totp.ts's kind-toggle reRender read the module- scope sectionsExpanded flag which was only updated on structural mutations — so toggling the disclosure open without adding/removing anything left the flag stale, and clicking Random/BIP39 collapsed the disclosure. Fix: read data-expanded from the live DOM before innerHTML swap. --- .../__tests__/sections-editor.test.ts | 42 ++++++++++- extension/src/popup/components/fields.ts | 72 ++++++++++++------- .../types/__tests__/sections-save.test.ts | 4 +- extension/src/popup/components/types/totp.ts | 5 ++ extension/src/popup/styles.css | 4 ++ 5 files changed, 98 insertions(+), 29 deletions(-) diff --git a/extension/src/popup/components/__tests__/sections-editor.test.ts b/extension/src/popup/components/__tests__/sections-editor.test.ts index 3dfcb4d..86d3ac7 100644 --- a/extension/src/popup/components/__tests__/sections-editor.test.ts +++ b/extension/src/popup/components/__tests__/sections-editor.test.ts @@ -105,7 +105,7 @@ describe('wireSectionsEditor', () => { ] }]; document.body.innerHTML = renderSectionsEditor(sections, true); wireSectionsEditor(document.body, sections, vi.fn()); - const deleteBtn = document.querySelector('[data-delete-field="0-0"]') as HTMLButtonElement; + const deleteBtn = document.querySelector('[data-delete-field="f0"]') as HTMLButtonElement; deleteBtn.click(); expect(sections[0].fields).toHaveLength(1); expect(sections[0].fields[0].id).toBe('f1'); @@ -142,7 +142,7 @@ describe('wireSectionsEditor', () => { document.body.innerHTML = renderSectionsEditor(sections, true); const rerender = vi.fn(); wireSectionsEditor(document.body, sections, rerender); - const labelInput = document.querySelector('[data-field-label="0-0"]') as HTMLInputElement; + const labelInput = document.querySelector('[data-field-label="f0"]') as HTMLInputElement; labelInput.value = 'new'; labelInput.dispatchEvent(new Event('input', { bubbles: true })); expect(sections[0].fields[0].label).toBe('new'); @@ -155,9 +155,45 @@ describe('wireSectionsEditor', () => { ] }]; document.body.innerHTML = renderSectionsEditor(sections, true); wireSectionsEditor(document.body, sections, vi.fn()); - const valueInput = document.querySelector('[data-field-value-input="0-0"]') as HTMLInputElement; + const valueInput = document.querySelector('[data-field-value-input="f0"]') as HTMLInputElement; valueInput.value = 'new'; valueInput.dispatchEvent(new Event('input', { bubbles: true })); expect(sections[0].fields[0].value).toEqual({ kind: 'text', value: 'new' }); }); }); + +describe('wireSectionsEditor preserves unsupported-kind fields on save', () => { + it('renders preserved note when section contains unsupported-kind fields', () => { + const sections: Section[] = [{ + name: 'mixed', + fields: [ + { id: 'f0000001', label: 'note', kind: 'text', + value: { kind: 'text', value: 'ok' }, hidden_by_default: false }, + { id: 'f0000002', label: 'when', kind: 'date' as any, + value: { kind: 'date', value: '2026-01-01' } as any, hidden_by_default: false }, + ], + }]; + document.body.innerHTML = renderSectionsEditor(sections, true); + expect(document.body.innerHTML).toContain('1 field of unsupported kind'); + expect(document.body.innerHTML).not.toContain('f0000002'); + }); + + it('add-text then save does not destroy unsupported-kind fields', () => { + const sections: Section[] = [{ + name: 'mixed', + fields: [ + { id: 'f0000002', label: 'when', kind: 'date' as any, + value: { kind: 'date', value: '2026-01-01' } as any, hidden_by_default: false }, + ], + }]; + document.body.innerHTML = renderSectionsEditor(sections, true); + wireSectionsEditor(document.body, sections, vi.fn()); + const addText = document.querySelector('[data-add-field="text"][data-section-idx="0"]') as HTMLButtonElement; + addText.click(); + expect(sections[0].fields).toHaveLength(2); + // Unsupported-kind field preserved untouched. + const dateField = sections[0].fields.find((f) => f.id === 'f0000002'); + expect(dateField).toBeDefined(); + expect(dateField!.value).toEqual({ kind: 'date', value: '2026-01-01' }); + }); +}); diff --git a/extension/src/popup/components/fields.ts b/extension/src/popup/components/fields.ts index f3b5e16..a7be540 100644 --- a/extension/src/popup/components/fields.ts +++ b/extension/src/popup/components/fields.ts @@ -206,7 +206,17 @@ function renderSectionBlock(section: Section, sIdx: number): string { ? `${escapeHtml(section.name)}` : `(anonymous)`; - const fieldsHtml = section.fields.map((f, fIdx) => renderEditorField(f, sIdx, fIdx)).join(''); + // Only render supported kinds. Other-kind fields stay in sectionsDraft + // untouched so they survive save intact. + const editable = section.fields.filter( + (f) => f.value.kind === 'text' || f.value.kind === 'password' || f.value.kind === 'concealed', + ); + const fieldsHtml = editable.map((f) => renderEditorField(f, sIdx, 0)).join(''); + + const preservedCount = section.fields.length - editable.length; + const preservedNote = preservedCount > 0 + ? `
${preservedCount} field${preservedCount === 1 ? '' : 's'} of unsupported kind (edit via CLI)
` + : ''; return `
@@ -218,6 +228,7 @@ function renderSectionBlock(section: Section, sIdx: number): string {
${fieldsHtml} + ${preservedNote}
@@ -227,21 +238,31 @@ function renderSectionBlock(section: Section, sIdx: number): string { `; } -function renderEditorField(field: Field, sIdx: number, fIdx: number): string { +function renderEditorField(field: Field, sIdx: number, _fIdx: number): string { const valueStr = (field.value.kind === 'text' || field.value.kind === 'password' || field.value.kind === 'concealed') ? field.value.value : ''; - const inputType = field.kind === 'text' ? 'text' : 'password'; - const key = `${sIdx}-${fIdx}`; + const inputType = field.value.kind === 'text' ? 'text' : 'password'; return `
- - - + + +
`; } +function findField( + sectionsDraft: Section[], + fieldId: string, +): { section: Section; fieldIdx: number } | null { + for (const section of sectionsDraft) { + const idx = section.fields.findIndex((f) => f.id === fieldId); + if (idx >= 0) return { section, fieldIdx: idx }; + } + return null; +} + /// Wire click + input handlers on a rendered sections-editor. Mutations /// happen in place on `sectionsDraft`. `rerender` is called after any /// structural change (add/remove) to regenerate the disclosure body; @@ -252,8 +273,8 @@ export function wireSectionsEditor( rerender: () => void, ): void { const toggle = scope.querySelector('.disclosure__toggle') as HTMLButtonElement | null; - const disclosure = scope.querySelector('.disclosure') as HTMLElement | null; toggle?.addEventListener('click', () => { + const disclosure = scope.querySelector('.disclosure') as HTMLElement | null; if (!disclosure) return; const expanded = disclosure.getAttribute('data-expanded') === 'true'; disclosure.setAttribute('data-expanded', expanded ? 'false' : 'true'); @@ -297,34 +318,37 @@ export function wireSectionsEditor( scope.querySelectorAll('[data-delete-field]').forEach((btn) => { btn.addEventListener('click', () => { - const [sIdxStr, fIdxStr] = (btn.dataset.deleteField ?? '0-0').split('-'); - const sIdx = Number(sIdxStr); - const fIdx = Number(fIdxStr); - sectionsDraft[sIdx].fields.splice(fIdx, 1); + const fieldId = btn.dataset.deleteField ?? ''; + const found = findField(sectionsDraft, fieldId); + if (!found) return; + found.section.fields = found.section.fields.filter((f) => f.id !== fieldId); rerender(); }); }); scope.querySelectorAll('[data-field-label]').forEach((input) => { input.addEventListener('input', () => { - const [sIdxStr, fIdxStr] = (input.dataset.fieldLabel ?? '0-0').split('-'); - const sIdx = Number(sIdxStr); - const fIdx = Number(fIdxStr); - if (sectionsDraft[sIdx]?.fields[fIdx]) { - sectionsDraft[sIdx].fields[fIdx].label = input.value; + const fieldId = input.dataset.fieldLabel ?? ''; + const found = findField(sectionsDraft, fieldId); + if (found) { + found.section.fields[found.fieldIdx].label = input.value; } }); }); scope.querySelectorAll('[data-field-value-input]').forEach((input) => { input.addEventListener('input', () => { - const [sIdxStr, fIdxStr] = (input.dataset.fieldValueInput ?? '0-0').split('-'); - const sIdx = Number(sIdxStr); - const fIdx = Number(fIdxStr); - const field = sectionsDraft[sIdx]?.fields[fIdx]; - if (!field) return; - const kind = field.value.kind as 'text' | 'password' | 'concealed'; - field.value = { kind, value: input.value }; + const fieldId = input.dataset.fieldValueInput ?? ''; + const found = findField(sectionsDraft, fieldId); + if (!found) return; + const field = found.section.fields[found.fieldIdx]; + // Only mutate supported kinds. Unsupported kinds are never rendered + // as editable (filtered by renderSectionBlock), so this path shouldn't + // fire for them — but guard defensively. + if (field.value.kind === 'text' || field.value.kind === 'password' || field.value.kind === 'concealed') { + const kind = field.value.kind; + field.value = { kind, value: input.value }; + } }); }); } diff --git a/extension/src/popup/components/types/__tests__/sections-save.test.ts b/extension/src/popup/components/types/__tests__/sections-save.test.ts index 8578ea8..0c0483a 100644 --- a/extension/src/popup/components/types/__tests__/sections-save.test.ts +++ b/extension/src/popup/components/types/__tests__/sections-save.test.ts @@ -36,10 +36,10 @@ describe('Login form packs sectionsDraft into Item.sections', () => { (document.querySelector('.add-section') as HTMLButtonElement).click(); (document.querySelector('[data-add-field="text"]') as HTMLButtonElement).click(); - const labelInput = document.querySelector('[data-field-label="0-0"]') as HTMLInputElement; + const labelInput = document.querySelector('[data-field-label]') as HTMLInputElement; labelInput.value = 'recovery email'; labelInput.dispatchEvent(new Event('input', { bubbles: true })); - const valueInput = document.querySelector('[data-field-value-input="0-0"]') as HTMLInputElement; + const valueInput = document.querySelector('[data-field-value-input]') as HTMLInputElement; valueInput.value = 'backup@example.com'; valueInput.dispatchEvent(new Event('input', { bubbles: true })); diff --git a/extension/src/popup/components/types/totp.ts b/extension/src/popup/components/types/totp.ts index befc8c5..a925cb2 100644 --- a/extension/src/popup/components/types/totp.ts +++ b/extension/src/popup/components/types/totp.ts @@ -237,6 +237,11 @@ export function renderForm(app: HTMLElement, mode: 'add' | 'edit', existing: Ite const secretVal = (document.getElementById('f-secret') as HTMLInputElement).value; const issuerVal = (document.getElementById('f-issuer') as HTMLInputElement).value; const labelVal = (document.getElementById('f-label') as HTMLInputElement).value; + // Preserve the disclosure's live expanded state across kind-toggle re-render. + const currentDisclosure = app.querySelector('.disclosure'); + if (currentDisclosure) { + sectionsExpanded = currentDisclosure.getAttribute('data-expanded') === 'true'; + } app.innerHTML = renderInner(); (document.getElementById('f-title') as HTMLInputElement).value = titleVal; (document.getElementById('f-secret') as HTMLInputElement).value = secretVal; diff --git a/extension/src/popup/styles.css b/extension/src/popup/styles.css index ff8b307..04e5a52 100644 --- a/extension/src/popup/styles.css +++ b/extension/src/popup/styles.css @@ -566,6 +566,10 @@ textarea { background: transparent; border: 0; color: #f85149; cursor: pointer; font-size: 14px; padding: 0 4px; } +.section-editor__preserved { + font-size: 10px; color: #6e7681; font-style: italic; + padding: 4px 0 4px 6px; +} .section-editor__add { display: flex; gap: 6px; margin-top: 6px;