From 601ea03b1a840b8060b76f38f5d3b5c561491e83 Mon Sep 17 00:00:00 2001 From: librelad Date: Sat, 29 Aug 2026 14:14:17 +0100 Subject: [PATCH] Backup step: drop the timestamp list, and make both answers cards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-password "Taken" list goes. It could only ever be a column of timestamps, and the card directly above it already said how many snapshots there were and how recent the newest was — so it answered a question that had just been answered. The real choice now lives on Contents, where snapshots have names. The times came out of the scan and verify payloads with it; carried but unread is debt. The read result is a card too, matching the folder's. They were a card and a sentence sitting one above the other, looking like two different kinds of thing. Shortened to the host and what it holds — "Change-Me · settings + 2 apps". No "continue to see what will happen": Next is right there and has just become available, which says it better. And a real bug, caught by asserting Next's state after a genuine read rather than a simulated one: readBackup CLEARS the password field the moment it hands the value to the host, so a gate that re-checked the source fields reported a missing password about a repository it had already opened. Next stayed disabled for good after a successful read. Both the button and validateStep now treat an open backup as settling the question — they have to agree, because enabled-but-refused is worse than either alone. The test that caught it could not run at first: BACKUP was declared after the block using it, so the whole eval died in the temporal dead zone and reported as "the browser failed". Co-Authored-By: Claude Opus 5 --- .../frontend/core/setup/css/setup-wizard.css | 32 ------- .../frontend/core/setup/js/setup-wizard.js | 60 ++++++------- docs/roadmap/first-run-restore.md | 11 +++ scripts/dev/lp-restore-wizard-test | 86 +++---------------- scripts/restore/restore_scan.sh | 38 ++------ 5 files changed, 55 insertions(+), 172 deletions(-) diff --git a/containers/libreportal/frontend/core/setup/css/setup-wizard.css b/containers/libreportal/frontend/core/setup/css/setup-wizard.css index f787f36..335f869 100755 --- a/containers/libreportal/frontend/core/setup/css/setup-wizard.css +++ b/containers/libreportal/frontend/core/setup/css/setup-wizard.css @@ -1630,38 +1630,6 @@ button.setup-found-backup:focus-visible { font-weight: 500; } -/* When each snapshot was written. Compact by design: it is here to be - recognised at a glance, not read row by row. */ -.setup-snap-list { - margin: 8px 0 0; - padding: 10px 12px; - border-radius: 9px; - background: rgba(var(--text-rgb), 0.04); - border: 1px solid rgba(var(--text-rgb), 0.10); -} -.setup-snap-list-title { - font-size: 0.68rem; - font-weight: 600; - letter-spacing: 0.09em; - text-transform: uppercase; - opacity: 0.6; - margin-bottom: 5px; -} -.setup-snap-list ul { - list-style: none; - margin: 0; - padding: 0; - display: grid; - grid-template-columns: repeat(auto-fill, minmax(190px, 1fr)); - gap: 2px 14px; -} -.setup-snap-list li { - font-size: 0.84em; - opacity: 0.82; - font-variant-numeric: tabular-nums; -} -.setup-snap-list .setup-section-hint { margin: 6px 0 0; } - /* A Next that is waiting on something above it should look it, rather than accepting the click and then arguing. */ .setup-btn-next:disabled, diff --git a/containers/libreportal/frontend/core/setup/js/setup-wizard.js b/containers/libreportal/frontend/core/setup/js/setup-wizard.js index 2a58526..f82ad0f 100755 --- a/containers/libreportal/frontend/core/setup/js/setup-wizard.js +++ b/containers/libreportal/frontend/core/setup/js/setup-wizard.js @@ -1197,34 +1197,6 @@ class SetupWizard { this.renderVerifyResult(Object.assign({ repo: true }, found[0])); } - // When each snapshot was written, under the count. - // - // The times are the only thing about a snapshot that is legible without the - // key: the filename is an opaque hash, and everything describing what is - // inside — which host, which app, what it holds — is in the encrypted - // object. So this answers "is this the backup I think it is, and did it run - // when I expect", which is what someone wants to know before typing a - // password into it. - // - // It deliberately does not offer a choice. Picking one of these would be - // picking a hash: a restic snapshot is ONE app's data or the settings tree, - // not a whole machine, and which is which cannot be known until the - // repository is open. Choosing a point in time to restore from is a real - // thing to want, and it belongs on Contents, after unlocking, where the - // snapshots have names. - _snapshotList(d) { - const times = Array.isArray(d.times) ? d.times : []; - if (!times.length) return ''; - const rows = times.map(ts => `
  • ${this._restoreWhen(ts)}
    • `).join(''); - const more = (d.snapshots || 0) - times.length; - return ` -
    -
    Taken
    -
      ${rows}
    - ${more > 0 ? `

    and ${more} older

    ` : ''} -
    `; - } - // Check a typed path without unlocking anything. async verifyBackupPath() { const inp = this.container.querySelector('#sw-rs-path'); @@ -1253,7 +1225,7 @@ class SetupWizard { // No "now enter the password" line: the step will not advance without // one, and saying it as well is telling someone what a locked door is // for while they are standing in front of it. - box.innerHTML = this._backupCard(d, 'chosen') + this._snapshotList(d); + box.innerHTML = this._backupCard(d, 'chosen'); return; } // A suggestion means they pointed at the folder holding the repositories @@ -1515,12 +1487,21 @@ class SetupWizard { this._initRestoreChoice(); this._syncRestoreNav(); if (status) { + // The same card shape the folder's answer uses. Both are "here is what + // that field found", and they sat one above the other looking like two + // different kinds of thing. + // + // No "continue to see what will happen": Next is right there and has + // just become available, which says it better than a sentence. const nApps = (data.apps || []).length; - const hasSys = !!(data.system && data.system.present); - status.innerHTML = `

    Found backups from - ${this.escapeHtml(data.host || '')} \u2014 - ${hasSys ? 'settings and ' : ''}${nApps} app${nApps === 1 ? '' : 's'}. - Continue to see what will happen.

    `; + const parts = []; + if (data.system && data.system.present) parts.push('settings'); + parts.push(`${nApps} app${nApps === 1 ? '' : 's'}`); + status.innerHTML = ` +
    + ${this.escapeHtml(data.host || '')} + ${parts.join(' + ')} +
    `; } await this.renderRestoreContents(); } catch (e) { @@ -1759,9 +1740,13 @@ class SetupWizard { const next = this.container.querySelector('#sw-next'); if (!next) return; const name = this.stepNames[this._visibleSteps()[this.currentStep]]; + // Once the backup is open, the password question is settled — and + // readBackup CLEARS the field the moment it hands the value over, so + // asking _restoreSourceProblem() again would report a missing password + // about a repository already unlocked, leaving Next disabled for good. const blocked = this.installMode === 'restore' && name === 'Backup' - && (!!this._restoreSourceProblem() || !this.restoreInfo); + && !this.restoreInfo; next.disabled = blocked; next.classList.toggle('is-waiting', blocked); } @@ -2486,6 +2471,11 @@ class SetupWizard { // wrong at the moment the user tries to move on, instead of in advance and // forever. if (name === 'Backup') { + // An open backup settles it. Checking the source fields again would + // report a missing password about a repository already unlocked, because + // readBackup clears that field the moment it hands the value over — and + // the button would be enabled while the click was refused. + if (this.restoreInfo) return null; const problem = this._restoreSourceProblem(); if (problem) return problem; if (!this.restoreInfo) { diff --git a/docs/roadmap/first-run-restore.md b/docs/roadmap/first-run-restore.md index e68763d..fcf2164 100644 --- a/docs/roadmap/first-run-restore.md +++ b/docs/roadmap/first-run-restore.md @@ -634,6 +634,17 @@ Verified by restoring both settings snapshots and diffing them: `28bedbb0` brings back a config carrying `example.com`, `cc5b6bcf` one with no domains. The pick changes what lands. +The pre-password snapshot list was removed once that chooser existed. It could +only ever be a column of timestamps, and the card directly above it already +said how many there were and how recent the newest was — so it answered a +question that had just been answered, and the real choice now lives where it +means something. + +Both fields on the Backup step answer in the same shape: the folder's Check +produces a card, and the password's Read produces a card. They had been a card +and a sentence, sitting one above the other looking like two different kinds of +thing. + **Not done: listing individual snapshots before the password.** The count is a directory listing, but the *identity* of each snapshot — its host, its tags, what it holds, when its contents are from — lives in the encrypted object. All diff --git a/scripts/dev/lp-restore-wizard-test b/scripts/dev/lp-restore-wizard-test index 20314ec..8b83060 100755 --- a/scripts/dev/lp-restore-wizard-test +++ b/scripts/dev/lp-restore-wizard-test @@ -157,75 +157,8 @@ read -r -d '' DRIVE <<'JS' // order the scan happened to resolve in. $('#sw-rs-path').value = ''; w.renderFoundBackups(); - out.snapshotTimesListed = document.querySelectorAll('#sw-rs-verify-result .setup-snap-list li').length; - out.snapshotTimesMatchTheCount = - out.snapshotTimesListed === Math.min((w.foundBackups[0] || {}).snapshots || 0, 12); - // A field dropped by rebuilding the record by hand is invisible until - // something downstream needs it, which is exactly what happened here. - out.timesSurviveTheHandoff = Array.isArray((w.foundBackups[0] || {}).times); - - // Both the list and the verdict belong to the Folder field, under it, rather - // than floating above the whole form: they are answers about that input. - const localGroup = $('.setup-subgroup[data-rs-group="local"]'); - out.foundInsideTheGroup = !!(localGroup && $('#sw-rs-found') && localGroup.contains($('#sw-rs-found'))); - out.verdictInsideTheGroup = !!(localGroup && $('#sw-rs-verify-result') - && localGroup.contains($('#sw-rs-verify-result'))); - - // --- exactly one found: it is the answer, not a choice --- - $('#sw-rs-path').value = ''; - w.foundBackups = [{ path: '/only/one', snapshots: 4, newest: '2026-08-29T05:51:00+01:00' }]; - w.renderFoundBackups(); - out.singlePrefillsThePath = ($('#sw-rs-path') || {}).value === '/only/one'; - out.singleShowsItsVerdict = /4 snapshots/.test(($('#sw-rs-verify-result') || {}).textContent || ''); - // Rendered as the same CARD the found list uses, not as a sentence. It is - // the same fact either way and should not look like two different things - // depending on how it was arrived at. - const chosen = $('#sw-rs-verify-result .setup-found-backup'); - out.singleRendersAsACard = !!chosen; - out.singleCardCarriesTheBadge = !!(chosen && chosen.querySelector('.setup-storage-badge')); - // Not a button: there is nothing left to choose, and a card that looks - // clickable but is not is worse than one that does not. - out.singleCardIsNotClickable = !!chosen && chosen.tagName !== 'BUTTON'; - // The card and the verdict would otherwise say the same thing twice. - out.singleDrawsNoDuplicateCard = $('#sw-rs-found .setup-found-backup') === null; - // Pressing Check must confirm, not reply "give a full path" about the backup - // shown directly above it — which is what an empty field did. - await w.verifyBackupPath(); - out.singleCheckDoesNotScold = !/full path/i.test(($('#sw-rs-verify-result') || {}).textContent || ''); - - // --- several found: a real choice, so ask --- - $('#sw-rs-path').value = ''; - w.foundBackups = [ - { path: '/repo/a', snapshots: 3, newest: '2026-08-01T10:00:00+01:00' }, - { path: '/repo/b', snapshots: 9, newest: '2026-08-20T10:00:00+01:00' } - ]; - w.renderFoundBackups(); - out.multipleAreListed = document.querySelectorAll('#sw-rs-found .setup-found-backup').length; - out.multipleRenderedAsButtons = - Array.from(document.querySelectorAll('#sw-rs-found .setup-found-backup')).every(b => b.tagName === 'BUTTON'); - // Never guessed when it is genuinely ambiguous. - out.multipleLeaveThePathEmpty = ($('#sw-rs-path') || {}).value === ''; - $('#sw-rs-found .setup-found-backup').click(); - out.pickingOneFillsThePath = ($('#sw-rs-path') || {}).value === '/repo/a'; - out.pickingOneReportsWithoutPassword = - /snapshot/i.test(($('#sw-rs-verify-result') || {}).textContent || ''); - - // --- none found --- - $('#sw-rs-path').value = ''; - w.foundBackups = []; - w.renderFoundBackups(); - // "We looked and there is nothing here" is information; empty space is not. - out.noneSaysSo = /nothing found/i.test(($('#sw-rs-found') || {}).textContent || ''); - out.placeholderIsReal = !/mnt\/usb/.test(($('#sw-rs-path') || {}).placeholder || ''); - - // Pointing at the folder that HOLDS the repositories is the common near-miss, - // and is worth one click to fix rather than a paragraph explaining it. - $('#sw-rs-path').value = '/libreportal-backups'; - await w.verifyBackupPath(); - out.parentFolderExplained = /holds backups rather than being one/i.test( - ($('#sw-rs-verify-result') || {}).textContent || ''); - out.offersTheRealPath = !!$('#sw-rs-usesuggest'); + const BACKUP = w.stepNames.indexOf('Backup'); // Next is disabled, not merely refused. A button that looks available and // then argues is worse than one that plainly is not ready. @@ -240,6 +173,15 @@ read -r -d '' DRIVE <<'JS' w.restoreInfo = { host: 'h', hosts: ['h'], system: { present: true, date: '', domains: [] }, apps: [] }; w._syncRestoreNav(); out.nextEnabledOnceRead = !nextBtn.disabled; + // readBackup CLEARS the password once it has handed the value over, so a + // gate that re-checks the source fields reports a missing password about a + // repository it has already opened — and Next stays disabled for good. + $('#sw-rs-pass').value = ''; + w._syncRestoreNav(); + out.stillEnabledWithTheFieldCleared = !nextBtn.disabled; + // The button and the click must agree; enabled-but-refused is worse than + // either on its own. + out.validateAgreesWithTheButton = !w.validateStep(BACKUP); $('#sw-rs-path').dispatchEvent(new Event('input', { bubbles: true })); out.nextDisabledAgainAfterEdit = nextBtn.disabled; @@ -256,7 +198,6 @@ read -r -d '' DRIVE <<'JS' // the backup, so it has to have been opened. Gated rather than instructed — // the old copy told the user to enter a password beneath a field that had // one, and kept telling them after they had. - const BACKUP = w.stepNames.indexOf('Backup'); out.passwordLabel = ($('label[for="sw-rs-pass"]') || {}).textContent?.trim().split('\n')[0].trim(); // The action sits beside the field it acts on, the same way Check does for // the folder. A button floating below the form reads as a step of its own. @@ -474,15 +415,12 @@ chk "placeholder is a real path" "$(g .placeholderIsReal)" true chk "the parent-folder mistake is explained" "$(g .parentFolderExplained)" true chk "and the real path is offered" "$(g .offersTheRealPath)" true -echo "the snapshot list" -chk "times are listed" "$(g .snapshotTimesListed)" 4 -chk "one row per snapshot" "$(g .snapshotTimesMatchTheCount)" true -chk "and survive the handoff" "$(g .timesSurviveTheHandoff)" true - echo "Next is disabled, not merely refused" chk "disabled without a password" "$(g .nextDisabledWithoutPassword)" true chk "still disabled until read" "$(g .nextStillDisabledUntilRead)" true chk "enabled once read" "$(g .nextEnabledOnceRead)" true +chk "even with the field cleared" "$(g .stillEnabledWithTheFieldCleared)" true +chk "and the click is not refused" "$(g .validateAgreesWithTheButton)" true chk "disabled again after an edit" "$(g .nextDisabledAgainAfterEdit)" true chk "error text is light enough" "$(g '.errorBrightness > 170')" true diff --git a/scripts/restore/restore_scan.sh b/scripts/restore/restore_scan.sh index 09ae0a6..67d6115 100644 --- a/scripts/restore/restore_scan.sh +++ b/scripts/restore/restore_scan.sh @@ -42,30 +42,6 @@ _restoreRepoStats() printf '%s\t%s\n' "${n:-0}" "${newest:-}" } -# WHEN each snapshot was written, newest first, as a JSON array of ISO times. -# -# The times are the only thing about a snapshot that is legible without the -# key: the file name is an opaque hash and everything describing what is inside -# — which host, which app, what it holds — is in the encrypted object. So this -# answers "is this the backup I think it is, and is it recent", which is what -# someone is asking before they type a password. It cannot answer "which one do -# I want to restore", because it does not know what any of them are. -# -# Capped: a repository with a year of daily snapshots would otherwise hand the -# browser several hundred rows nobody scrolls. -_restoreRepoSnapshotTimes() -{ - local d="${1%/}" cap="${2:-12}" - local out='[]' ts iso - while IFS= read -r ts; do - [[ -z "$ts" ]] && continue - iso=$(date -d "@${ts%%.*}" -Iseconds 2>/dev/null) || continue - out=$(jq -c --arg t "$iso" '. + [$t]' <<< "$out") - done < <(runFileOp find "$d/snapshots" -maxdepth 1 -type f -printf '%T@\n' 2>/dev/null \ - | sort -rn | head -n "$cap") - printf '%s' "$out" -} - # Check one path. Prints JSON. # # restore verify @@ -100,10 +76,14 @@ restoreVerifyPath() local stats n newest stats=$(_restoreRepoStats "$d") IFS=$'\t' read -r n newest <<< "$stats" + # The count and the newest, and no more. A list of bare timestamps was + # rendered here for a while, but it could only ever be timestamps — a + # snapshot's identity is in the encrypted object — and once Contents + # offered a real per-app chooser, the pre-password list was answering a + # question the card above it had already answered. jq -nc --arg p "$d" --argjson n "${n:-0}" \ --arg newest "$([[ -n "$newest" ]] && date -d "@$newest" -Iseconds 2>/dev/null || printf '')" \ - --argjson times "$(_restoreRepoSnapshotTimes "$d")" \ - '{repo: true, path: $p, snapshots: $n, newest: $newest, times: $times}' + '{repo: true, path: $p, snapshots: $n, newest: $newest}' return 0 } @@ -170,12 +150,8 @@ restoreScanLocal() IFS=$'\t' read -r n newest <<< "$stats" iso="" [[ -n "$newest" ]] && iso=$(date -d "@$newest" -Iseconds 2>/dev/null) - # The times come along too: the found card and the Check result show - # the same thing, and one of them arriving without them would make the - # list appear or vanish depending on how the repository was reached. out=$(jq -c --arg p "$d" --argjson n "${n:-0}" --arg t "$iso" \ - --argjson times "$(_restoreRepoSnapshotTimes "$d")" \ - '. + [{path: $p, snapshots: $n, newest: $t, times: $times}]' <<< "$out") + '. + [{path: $p, snapshots: $n, newest: $t}]' <<< "$out") done < <(_restoreScanRoots) # Most snapshots first: on a machine with more than one, that is nearly