Breadway/bos-settings
1
0
Fork 0
Code Issues 2 Pull requests 2 Projects Releases Packages Wiki Activity Actions

fix(ui): replace window.confirm with an in-app confirm sheet #76

Merged
Breadway merged 2 commits from fix/bset-69-in-app-confirm into main 2026-10-14 12:21:38 +08:00
Conversation 2 Commits 2 Files changed 17 +141 -19
Breadway commented 2026-10-13 21:00:08 +08:00
Owner
Copy link

Closes BSET-69

What

WebKitGTK in the Tauri webview shows no JS script dialogs, so confirm() returned without asking and every destructive action ran unprompted.

  • New $lib/confirm.svelte.ts with confirmAction({title, message, confirmLabel, danger}). It returns a Promise that resolves true only on the confirm button. Cancel, Escape and a backdrop click resolve false. A newer request cancels an older one.
  • New ConfirmHost.svelte is mounted once in +page.svelte and uses the existing v2 Sheet. Cancel is the first button, so it takes focus. The confirm button is btn danger unless danger: false, which Restore files uses.
  • All 14 call sites converted: Users remove, Storage (clean target, unused deps), Snapshots delete, Keybinds remove layout, Bluetooth forget, Sharing enable SSH, ServiceControl stop critical, Autostart (both), Updates without snapshot, PowerMenu, GetApps AUR, Backup restore.

Tests

  • confirm.test.mjs (added to npm test) covers the resolution rules, plus a source scan that fails on any bare confirm(, alert( or prompt(. The scan fails on the old Users.svelte.
  • svelte-check 0/0, npm test, build, cargo fmt, clippy -D warnings, cargo test (144).
  • Live BOS VM: Remove on a throwaway account showed the sheet with the account still present. Cancel kept it. Confirming deleted the account and /home/victim.

Capability changes

None removed. The prompts now actually appear. Some wording is split into a title and a message, and the confirm buttons say what they do (Remove, Delete, Forget, Enable...) instead of OK.

Closes BSET-69 ## What WebKitGTK in the Tauri webview shows no JS script dialogs, so `confirm()` returned without asking and every destructive action ran unprompted. - New `$lib/confirm.svelte.ts` with `confirmAction({title, message, confirmLabel, danger})`. It returns a Promise<boolean> that resolves true only on the confirm button. Cancel, Escape and a backdrop click resolve false. A newer request cancels an older one. - New `ConfirmHost.svelte` is mounted once in `+page.svelte` and uses the existing v2 `Sheet`. Cancel is the first button, so it takes focus. The confirm button is `btn danger` unless `danger: false`, which Restore files uses. - All 14 call sites converted: Users remove, Storage (clean target, unused deps), Snapshots delete, Keybinds remove layout, Bluetooth forget, Sharing enable SSH, ServiceControl stop critical, Autostart (both), Updates without snapshot, PowerMenu, GetApps AUR, Backup restore. ## Tests - `confirm.test.mjs` (added to npm test) covers the resolution rules, plus a source scan that fails on any bare `confirm(`, `alert(` or `prompt(`. The scan fails on the old Users.svelte. - svelte-check 0/0, npm test, build, cargo fmt, clippy -D warnings, cargo test (144). - Live BOS VM: Remove on a throwaway account showed the sheet with the account still present. Cancel kept it. Confirming deleted the account and /home/victim. ## Capability changes None removed. The prompts now actually appear. Some wording is split into a title and a message, and the confirm buttons say what they do (Remove, Delete, Forget, Enable...) instead of OK.
fix(ui): replace window.confirm with an in-app confirm sheet
All checks were successful
check / check (push) Successful in 8m8s
Details
edfa3b8df1 
WebKitGTK in Tauri shows no script dialogs, so confirm() returned without
asking and every destructive action ran unprompted: removing a user, emptying
the trash, deleting snapshots, power off and 10 more (found in a BOS VM).

confirmAction() returns a promise that resolves true only on the confirm
button; Cancel, Escape and the backdrop resolve false, and Cancel takes focus.
ConfirmHost renders it once at the app root. confirm.test.mjs covers the
resolution rules and fails the build if confirm/alert/prompt come back.

Verified in a BOS VM: Remove shows the sheet with the account intact, Cancel
keeps it, confirming removes the account and its home.
Merge remote-tracking branch 'origin/main' into fix/bset-69-in-app-confirm
All checks were successful
check / check (push) Successful in 8m0s
Details
bfa068d9d8 
# Conflicts:
#	frontend/package.json
Breadway commented 2026-10-14 12:21:25 +08:00
Author
Owner
Copy link

Re-review after base merge. Reviewed exact head bfa068d9d8 (base a7b3e8b, merge of main incl. #67 #75 #77 #78). Verdict: PASS.

  • Merge resolution: frontend/package.json test script equals main's script (keycapture, nightlight, liveGate, snapshotRestore, snapshotLabel, uniqueBy, displayLayout, v2, drafts) plus confirm.test.mjs appended. All 10 test files in frontend/src/lib are run; nothing dropped.
  • Bare dialogs: scan of frontend/src (svelte, ts, js, html) finds no confirm(/alert(/prompt( calls, including Memory.svelte, Display.svelte, Bluetooth.svelte. Bluetooth forget still awaits confirmAction (Forget label) after #77.
  • 14 destructive call sites all await confirmAction; resolve true only via the confirm button; Cancel, Escape, backdrop (Sheet onClose) call answer(false); superseded request resolves false. Ban test runs in npm test.
  • Memory/Display panels add no destructive actions needing a confirm (Display uses keep/revert guard).
  • Local on exact head: npm ci, npm run check (0 errors 0 warnings), npm test (all pass incl. confirm), npm run build: ok. cargo fmt --check ok, clippy --locked --all-targets -D warnings ok, cargo test --locked 179 passed.
  • Forgejo CI check / check on bfa068d: success (8m0s).
    Note (non-blocking): snapshotRestore.ts confirmAndRestoreSnapshot with a sync confirm callback is only used by its unit test; the app restores via a Sheet.
Re-review after base merge. Reviewed exact head bfa068d9d8563ab166199350fd15b7cd78bd90ab (base a7b3e8b, merge of main incl. #67 #75 #77 #78). Verdict: PASS. - Merge resolution: frontend/package.json test script equals main's script (keycapture, nightlight, liveGate, snapshotRestore, snapshotLabel, uniqueBy, displayLayout, v2, drafts) plus confirm.test.mjs appended. All 10 test files in frontend/src/lib are run; nothing dropped. - Bare dialogs: scan of frontend/src (svelte, ts, js, html) finds no confirm(/alert(/prompt( calls, including Memory.svelte, Display.svelte, Bluetooth.svelte. Bluetooth forget still awaits confirmAction (Forget label) after #77. - 14 destructive call sites all await confirmAction; resolve true only via the confirm button; Cancel, Escape, backdrop (Sheet onClose) call answer(false); superseded request resolves false. Ban test runs in npm test. - Memory/Display panels add no destructive actions needing a confirm (Display uses keep/revert guard). - Local on exact head: npm ci, npm run check (0 errors 0 warnings), npm test (all pass incl. confirm), npm run build: ok. cargo fmt --check ok, clippy --locked --all-targets -D warnings ok, cargo test --locked 179 passed. - Forgejo CI check / check on bfa068d: success (8m0s). Note (non-blocking): snapshotRestore.ts confirmAndRestoreSnapshot with a sync confirm callback is only used by its unit test; the app restores via a Sheet.
Breadway commented 2026-10-14 12:27:47 +08:00
Author
Owner
Copy link

Merged with a merge commit at d432319ac5 (head bfa068d, base a7b3e8b). Awaiting coordinator verification of integrated behavior on main.

Merged with a merge commit at d432319ac559fcc7b29bb7c2df96bd4c98794bfe (head bfa068d, base a7b3e8b). Awaiting coordinator verification of integrated behavior on main.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
breadbug

   
bug

   
displays

   
enhancement

   
keybinds

   
network

   
phase-1

   
power

   
privacy

   
recovery

   
search

   
snapshots

   
sound

   
ui

   
ux
No labels
breadbug
bug
displays
enhancement
keybinds
network
phase-1
power
privacy
recovery
search
snapshots
sound
ui
ux
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Breadway/bos-settings!76
No description provided.