fix(ui): review findings: shortcuts during capture, armed bind focus, duplicate app list, legacy model (#495)

ui/src/Settings.svelte

@@ -15,6 +15,7 @@
   import { fill, changedKeys } from './session.js';
   import { themes, normalizePalette } from './design/themes.js';
   import { droppedBinds } from './lib/keybindRules.js';
+  import { capturing } from './lib/capture.js';
   import TitleBar from './shell/TitleBar.svelte';
   import Sidebar from './shell/Sidebar.svelte';
   import ScrollChip from './shell/ScrollChip.svelte';
@@ -235,6 +236,7 @@
     if (c) c.focus();
   }
   function onKeydown(e) {
+    if (capturing()) return;   // the keys belong to the binding being recorded
     const mod = e.ctrlKey && !e.altKey && !e.shiftKey;
     if (mod && e.key.toLowerCase() === 'f') { e.preventDefault(); const i = searchInput(); if (i) { i.focus(); i.select(); } return; }
     if (e.key === 'Escape' && (searching || e.target === searchInput())) {

ui/src/controls/AppList.svelte

@@ -6,13 +6,22 @@
   let { value = '', title = '', onChange = () => {}, disabled = false, labelledby, describedby, valueId,
         pick = pickExe } = $props();
   let open = $state(false);
-  const items = $derived(String(value ?? '').split(',').map((s) => s.trim()).filter(Boolean));
+  // The core matches exe names case-insensitively, so two spellings are one entry; a hand-edited ini can
+  // repeat one, and the keyed list below throws on a duplicate key, so the first spelling wins.
+  const items = $derived.by(() => {
+    const seen = new Set();
+    return String(value ?? '').split(',').map((s) => s.trim()).filter((s) => {
+      const k = s.toLowerCase();
+      if (!s || seen.has(k)) return false;
+      seen.add(k);
+      return true;
+    });
+  });
   const summary = $derived(items.length === 0 ? 'None' : items.length === 1 ? items[0] : `${items.length} apps`);
   const uid = 'al-' + Math.random().toString(36).slice(2, 8);
   async function add() {
     const name = await pick();
     if (!name) return;
-    // The core matches exe names case-insensitively, so two spellings are one entry.
     if (items.some((i) => i.toLowerCase() === name.toLowerCase())) return;
     onChange([...items, name].join(','));
   }

ui/src/controls/Bindings.svelte

@@ -10,6 +10,8 @@
   import { checkKeyBind, checkClickBind, refusalText } from '../lib/keybindRules.js';
   import { vkName, BUTTON_NAMES, modList, popcount, WHEEL_UP, WHEEL_DOWN } from '../lib/bindNames.js';
   import { unitsOf, clearUnit, placeBinding, maxOf } from '../lib/bindings.js';
+  import { setCapturing } from '../lib/capture.js';
+  import { onDestroy } from 'svelte';
   let { row, values = {}, live = () => {}, disabled = false, labelledby, describedby, valueId } = $props();
 
   const units = $derived(unitsOf(row, values));
@@ -27,6 +29,9 @@
   const list = $derived(armed && armed.list ? armed.list : units);
   const showGhost = $derived(armed ? !!armed.add : units.length < max);
   const uid = 'bx-' + Math.random().toString(36).slice(2, 8);
+  $effect(() => { setCapturing(uid, !!armed); return () => setCapturing(uid, false); });
+  // Leaving the page while armed (the sidebar re-keys it) must put the old binding back.
+  onDestroy(() => cancel());
 
   const mods = (e) => (e.ctrlKey ? 1 : 0) | (e.altKey ? 2 : 0) | (e.shiftKey ? 4 : 0) | (e.metaKey ? 8 : 0);
   const text = (m, name) => [...modList(m), name].join('+');
@@ -82,11 +87,17 @@
     if (!btn) return;
     const m = mods(e);
     if (btn === 4 && m === 0) return;   // a plain right-click removes the binding (contextmenu)
+    const name = text(m, BUTTON_NAMES[btn]);
+    const v = popcount(m) > 2 ? 'twomods' : checkClickBind(btn, m);
+    // A refused press on the box being recorded stays armed and says why. Anywhere else it is the
+    // user leaving (Close, another row, the sidebar): cancel and let it through. preventDefault
+    // there would also keep the focus, so the blur that cancels would never come.
+    if (v !== 'ok' && !(e.target instanceof Element && e.target.closest('button') === document.activeElement)) {
+      cancel();
+      return;
+    }
     e.preventDefault();
     if (btn === 4) skipContext = true;
-    const name = text(m, BUTTON_NAMES[btn]);
-    if (popcount(m) > 2) { refuse('twomods', name); return; }
-    const v = checkClickBind(btn, m);
     if (v !== 'ok') { refuse(v, name); return; }
     capturedAt = performance.now();
     commit({ kind: 'btn', code: btn, mods: m }, name);

ui/src/controls/PanBinding.svelte

@@ -7,6 +7,7 @@
   import { checkKeyBind, refusalText } from '../lib/keybindRules.js';
   import { modList, popcount } from '../lib/bindNames.js';
   import { PAN_ARROWS, panState, panPatch } from '../lib/bindings.js';
+  import { setCapturing } from '../lib/capture.js';
   let { row, values = {}, live = () => {}, disabled = false, labelledby, describedby, valueId } = $props();
 
   const now = $derived(panState(values));
@@ -18,6 +19,7 @@
   let refusal = $state('');
   let liveMsg = $state('');
   const uid = 'pan-' + Math.random().toString(36).slice(2, 8);
+  $effect(() => { setCapturing(uid, armed); return () => setCapturing(uid, false); });
   const mods = (e) => (e.ctrlKey ? 1 : 0) | (e.altKey ? 2 : 0) | (e.shiftKey ? 4 : 0) | (e.metaKey ? 8 : 0);
 
   function arm() {

ui/src/lib/KeybindCapture.svelte

@@ -13,6 +13,7 @@
   // listening and says why, on screen and to screen readers.
   import { checkKeyBind, checkClickBind, refusalText } from './keybindRules.js';
   import { MOD_BITS, vkName, BUTTON_NAMES, shortButtonName } from './bindNames.js';
+  import { onDestroy } from 'svelte';
   // row.modsOnly = the arrow-key pan row: only the modifiers are captured (modsKey is a virtual key the
   // owner translates into the four real binds); the arrow keys are fixed and drawn by the owner.
   // `onClear` (optional) replaces the right-click clear patch.
@@ -117,6 +118,8 @@
     onChange(clearPatch);
     armed = true;
   }
+  // Leaving onboarding while armed must put the old binding back.
+  onDestroy(() => { if (armed) cancel(); });
   function cancel() {
     if (preCapture) onChange(preCapture);
     armed = false; preCapture = null; refusal = '';
@@ -176,10 +179,17 @@
     if (!btn) return;
     const mods = eventMods(e);
     if (btn === 4 && mods === 0) return;        // a plain right-click clears the row (contextmenu)
-    e.preventDefault();
-    if (btn === 4) skipContextMenu = true;
     const what = [modsName(mods), BUTTON_NAMES[btn]].filter(Boolean).join('+');
     const verdict = checkClickBind(btn, mods);
+    // A refused press on the armed keycap stays armed and says why. Anywhere else (Next, the other
+    // row) it is the user leaving: cancel and let it through, since preventDefault would also keep
+    // the focus and the blur that cancels would never come.
+    if (verdict !== 'ok' && !(e.target instanceof Element && e.target.closest('button') === document.activeElement)) {
+      cancel();
+      return;
+    }
+    e.preventDefault();
+    if (btn === 4) skipContextMenu = true;
     if (verdict !== 'ok') { refuse(verdict, what); return; }
     const patch = { [row.buttonKey]: String(btn), [row.vkKey]: '0' };
     if (row.modsKey) patch[row.modsKey] = '0';

ui/src/lib/capture.js

@@ -0,0 +1,6 @@
+// Keybind captures that are listening right now (Bindings, PanBinding). Settings' page shortcuts
+// (Ctrl+F, Ctrl+Q, Escape) stand down while one is: its window listener runs before theirs, so the
+// combination being recorded would also run the shortcut (#495).
+const listening = new Set();
+export function setCapturing(id, on) { if (on) listening.add(id); else listening.delete(id); }
+export const capturing = () => listening.size > 0;

ui/src/session.js

@@ -35,9 +35,14 @@ function schemaDefaults() {
 
 // Issue #456: renderScaling replaced bilinear. An ini (or profile) without it reads the way the core
 // migrates it (ParseConfig): bilinear=0 keeps Nearest, anything else is the xBR default.
+// A model the core does not know (the retired model=magnify) runs Auto (ClampConfig), so it reads
+// as hybrid here too, or the Engine select shows blank and the Auto-only rows hide.
+const MODELS = new Set(['render', 'transform', 'hybrid']);
 function migrate(m) {
-  if (m.renderScaling === undefined && String(m.bilinear) === '0') return { ...m, renderScaling: '0' };
-  return m;
+  let out = m;
+  if (m.renderScaling === undefined && String(m.bilinear) === '0') out = { ...out, renderScaling: '0' };
+  if (m.model !== undefined && !MODELS.has(String(m.model).trim())) out = { ...out, model: 'hybrid' };
+  return out;
 }
 
 // session: { values, saved } as the host reports them. Returns both maps with defaults filled in.

ui/tests/bindings.spec.js

@@ -82,6 +82,6 @@ test('pan: none, arrows with one shared mask, custom for anything else', () => {
 
 test('hotkeys files contain no em-dash', () => {
   for (const f of ['../src/lib/bindings.js', '../src/lib/bindNames.js', '../src/controls/Bindings.svelte',
-    '../src/controls/PanBinding.svelte', '../src/controls/bindings.css'])
+    '../src/controls/PanBinding.svelte', '../src/controls/bindings.css', '../src/lib/capture.js'])
     expect(readFileSync(new URL(f, import.meta.url), 'utf8')).not.toContain(String.fromCharCode(0x2014));
 });

ui/tests/session.spec.js

@@ -59,6 +59,15 @@ test('session helpers: global keys never count, defaults fill both sides', () =>
   expect(changedKeys(f.values, f.saved)).toEqual(['maxLevel']);
 });
 
+test('session helpers: an unknown model (the retired magnify) reads as hybrid, like the core', () => {
+  const f = fill({ values: { model: 'magnify' }, saved: { model: 'magnify' } });
+  expect(f.values.model).toBe('hybrid');
+  expect(f.saved.model).toBe('hybrid');
+  expect(changedKeys(f.values, f.saved)).toEqual([]);
+  expect(fill({ values: { model: 'transform' } }).values.model).toBe('transform');
+  expect(fill({ values: { model: 'render' } }).values.model).toBe('render');
+});
+
 test('session files contain no em-dash', () => {
   for (const f of ['../src/session.js', '../src/Settings.svelte', '../src/prompts/Prompt.svelte', '../src/bridge.js'])
     expect(readFileSync(new URL(f, import.meta.url), 'utf8')).not.toContain(String.fromCharCode(0x2014));

ui/tests/settings.spec.js

@@ -175,6 +175,29 @@ test('Zoom engine: the engine row, per-window rows follow Auto, the share-keys a
   expect((await sent(page, 'setConfig')).some((m) => m.key === 'noSwallowApps' && m.value.includes('RDR2.exe'))).toBe(true);
 });
 
+test('an app list with a hand-edited duplicate still opens, listing it once (#495)', async ({ page }) => {
+  await page.addInitScript(() => { window.__cfgExtra = { showAdvanced: '1', noSwallowApps: 'doom.exe, Doom.exe,doom.exe,b.exe' }; });
+  const errors = [];
+  page.on('pageerror', (e) => errors.push(e.message));
+  await page.goto('/');
+  await go(page, 'hotkeys');
+  const apps = key(page, 'noSwallowApps');
+  await expect(apps.locator('.sum')).toHaveText('2 apps');
+  await apps.getByRole('button', { name: /Manage/ }).click();
+  await expect(page.getByRole('dialog').getByRole('button', { name: /^Remove / })).toHaveCount(2);
+  await page.getByRole('button', { name: 'Remove b.exe' }).click();
+  expect((await sent(page, 'setConfig')).filter((m) => m.key === 'noSwallowApps').at(-1).value).toBe('doom.exe');
+  expect(errors).toEqual([]);
+});
+
+test('a legacy model=magnify shows Auto with its per-window rows (#495)', async ({ page }) => {
+  await page.addInitScript(() => { window.__cfgExtra = { showAdvanced: '1', model: 'magnify' }; });
+  await page.goto('/');
+  await go(page, 'zoom');
+  await expect(key(page, 'model').locator('select')).toHaveValue('hybrid');
+  await expect(key(page, 'engineGame')).toBeVisible();
+});
+
 // ---- other pages -----------------------------------------------------------------------------------
 
 test('View: the edge margin shows only when the pointer is kept within the edges', async ({ page }) => {
@@ -299,6 +322,48 @@ test('Hotkeys: click a binding to record it again; Escape puts the old one back'
   await expect(boxes(page, '__zoomIn')).toHaveText(['Mouse 5', 'PageUp']);
 });
 
+test('Hotkeys: Ctrl+Q and Ctrl+F while recording bind the key, not the page shortcut (#495)', async ({ page }) => {
+  await page.goto('/');
+  await key(page, '__cursorLock').locator('.kchg').click();
+  await page.keyboard.press('Control+q');
+  expect(await persisted(page, 'cursorLockVk')).toBe('81');
+  expect(await persisted(page, 'cursorLockMods')).toBe('1');
+  expect((await sent(page, 'window')).filter((m) => m.action === 'quitWind')).toHaveLength(0);
+  await key(page, '__cursorLock').locator('.kchg').click();
+  await page.keyboard.press('Control+f');
+  expect(await persisted(page, 'cursorLockVk')).toBe('70');
+  await expect(page.locator('.side .search input')).not.toBeFocused();
+  await page.keyboard.press('Control+f');   // not recording any more: the shortcut is back
+  await expect(page.locator('.side .search input')).toBeFocused();
+});
+
+test('Hotkeys: clicking away from a recording zoom box puts the old binding back (#495)', async ({ page }) => {
+  await page.goto('/');
+  const zin = key(page, '__zoomIn').locator('.kchg').first();   // Mouse 5
+  await zin.click();
+  expect(await persisted(page, 'zoomInButton')).toBe('0');
+  await key(page, '__zoomOut').locator('.kchg').first().click();   // a plain click elsewhere is leaving
+  expect(await persisted(page, 'zoomInButton')).toBe('2');
+  await expect(key(page, '__zoomIn').locator('.kc.live')).toHaveCount(0);
+  await expect(page.locator('.kc.live')).toHaveCount(1);   // only Zoom out records
+  await page.keyboard.press('Escape');
+  await zin.click();
+  await go(page, 'zoom');   // the page unmounts while recording
+  expect(await persisted(page, 'zoomInButton')).toBe('2');
+  await go(page, 'hotkeys');
+  await expect(boxes(page, '__zoomIn')).toHaveText(['Mouse 5', 'PageUp']);
+});
+
+test('Hotkeys: a refused click on the recording box itself stays armed and says why', async ({ page }) => {
+  await page.goto('/');
+  const zin = key(page, '__zoomIn').locator('.kchg').first();
+  await zin.click();
+  await zin.click();   // a plain left click needs a modifier
+  await expect(zin).toHaveAccessibleName(/Recording/);
+  await expect(key(page, '__zoomIn').locator('.refusal')).toBeVisible();
+  await page.keyboard.press('Escape');
+});
+
 test('Hotkeys: recording a key over a binding replaces it', async ({ page }) => {
   await page.goto('/');
   await key(page, '__cursorLock').locator('.kchg').click();