fix(ext/popup): preserve unsupported-kind fields + totp expanded state

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.
This commit is contained in:
adlee-was-taken
2026-04-24 18:51:23 -04:00
parent e1d32b0379
commit 6ba9ccfa4c
5 changed files with 98 additions and 29 deletions

View File

@@ -206,7 +206,17 @@ function renderSectionBlock(section: Section, sIdx: number): string {
? `<span class="name">${escapeHtml(section.name)}</span>`
: `<span class="name anon">(anonymous)</span>`;
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
? `<div class="section-editor__preserved">${preservedCount} field${preservedCount === 1 ? '' : 's'} of unsupported kind (edit via CLI)</div>`
: '';
return `
<div class="section-editor" data-section-idx="${sIdx}">
@@ -218,6 +228,7 @@ function renderSectionBlock(section: Section, sIdx: number): string {
</span>
</div>
${fieldsHtml}
${preservedNote}
<div class="section-editor__add">
<button type="button" data-add-field="text" data-section-idx="${sIdx}">+ text</button>
<button type="button" data-add-field="password" data-section-idx="${sIdx}">+ password</button>
@@ -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 `
<div class="section-editor__field">
<input type="text" data-field-label="${key}" value="${escapeHtml(field.label)}" placeholder="label">
<input type="${inputType}" data-field-value-input="${key}" value="${escapeHtml(valueStr)}" placeholder="value">
<button type="button" class="delete-field" data-delete-field="${key}">×</button>
<input type="text" data-field-label="${escapeHtml(field.id)}" value="${escapeHtml(field.label)}" placeholder="label">
<input type="${inputType}" data-field-value-input="${escapeHtml(field.id)}" value="${escapeHtml(valueStr)}" placeholder="value">
<button type="button" class="delete-field" data-delete-field="${escapeHtml(field.id)}" data-section-idx="${sIdx}">×</button>
</div>
`;
}
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<HTMLButtonElement>('[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<HTMLInputElement>('[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<HTMLInputElement>('[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 };
}
});
});
}