Files
miniSCP/docs/gotchas.md
T
jgamboaandClaude c4364c1051 Fix the dead win7 context menu, flatten the selection, add a tab menu
Right-click did nothing under the win7 skin, and then swallowed the next
click. 7.css declares `position: relative` on ul[role=menu] and the `skin`
layer outranks `utilities`, so Tailwind's `.fixed` lost: [style.left/top.px]
became flow offsets from the menu's static position — the last child of the
h-screen frame — and every menu rendered below the viewport, unreachable
behind overflow:hidden. Its backdrop is a plain <div>, unmatched by 7.css, so
that stayed genuinely fixed and ate the following click. Regressed in 0ae2a62,
when the menu adopted 7.css's real markup.

Restated `position: fixed` in skinfix, scoped to a new `w7-ctx` marker so
7.css's own menus keep theirs. scripts/menu-probe.ts is the regression test:
it renders the menu at declared coordinates, pixel-scans whether it landed
there, and carries a marker-less control menu (the bug verbatim) as its own
discriminator. A resting pixel diff cannot see this class of bug.

Selection band goes flat: --w7-sel-bg/-bd (#cbe8f6/#26a0da) is what Explorer
actually painted, where 7.css reused --w7-el-grad-h, its Aero element-hover
ramp. That var is left alone — buttons and tabs still want the gloss. The
unfocused pane gets the grey idle band; the band still never disappears with
focus, it only weakens. Harness diff confirms the change touches selection
rows and nothing else, in both themes.

New workspace-tab right-click menu in both skins: New Tab, Close Tab, Close
Other Tabs, Close Tabs to the Right, Disconnect. The target id travels inside
the action string because the right-clicked tab is not necessarily the active
one. Illegal entries are disabled rather than omitted, the way Win7 greyed
out — and since 7.css's [aria-disabled] handling is CSS-only, the keyboard
paths check the flag themselves.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-16 20:50:42 +02:00

24 KiB
Raw Blame History

Gotchas — landmines this codebase has already stepped on

Every one of these caused a real bug or debugging spiral. Read before touching the adjacent code.

Angular (v22, zoneless, signals, OnPush)

  • Never consume push streams through signal+effect. An effect that reads a signal and (directly or transitively) writes signals re-triggers itself — this froze the entire app on the first transfer. Push messages (transferUpdate etc.) go through plain callback registries in RpcService. Signal coalescing would also silently drop intermediate progress snapshots.
  • No signal writes during template render (NG0600). Anything that loads panels from code paths reachable during render defers with queueMicrotask (see PanelsService.newWorkspace).
  • [value] on <select> is applied before @for options render and silently selects nothing. Use per-option [selected] bindings (connection manager stack picker).
  • Template bindings with special characters: [class.bg-primary/15] works; [class.shadow-[...]] (brackets) does NOT parse — use [style.box-shadow]. Static class attributes may use arbitrary values freely.
  • Functions called from templates ARE reactive: signals read inside them register on the view's consumer (call-stack-based tracking). That's why iconClass(entry) reacting to settings.iconColors() works.
  • Dedicated tools > shell: IDE diagnostics between paired template/class edits are often stale mid-sequence — trust bun run build:ui (full AOT type-check), not the first red squiggle.
  • Tailwind v4 canonical class names: wrap-break-word not break-words, accent-primary not accent-[var(--primary)], z-60 not z-[60]. Every color used in a template must resolve through @theme inline.
  • The win7 skin layer outranks Tailwind utilities. 7.css is imported layer(skin), ordered above utilities, so any utility you add to a button/input/select/textarea/table is silently dead under that skin. If it matters, neutralize in the @layer skin block in app/skins/7like/7like.css — and note revert-layer only reaches utilities from inside skin; used from skinfix it falls back onto 7.css and is a no-op. This bit once already.
  • @import … layer(name) works through @tailwindcss/postcss (it wraps the imported AST), but the deep path is mandatory: 7.css ships no exports map and Tailwind's resolver reads only style/mainFields, so a bare @import "7.css" resolves to nothing. Use 7.css/dist/7.scoped.css.
  • Stylesheet url() must resolve at BUILD time: assets referenced from styles.css live under ui/src/ (esbuild emits them hashed into media/ and rewrites the url), NOT ui/public/ — a public path fails the build with "Could not resolve".
  • Two class attributes on one element: the second is silently dropped. No warning, no build error — the layout classes simply never apply. This collapsed the tab strip and two window headers at once (fe028a9), because a scripted edit appended class="…" to elements that already had one. When removing a conditional binding, check what the branch contained: the bindings there were genuinely dead, but the layout classes inside them were not, and nothing replaced them.
  • A component template: is a TS template literal, so no backticks inside it — not even in an HTML comment. A backtick in a comment terminates the literal and the errors point at the prose, not at the quoting.

The interface scale (root font-size lever)

  • electrobun's native page zoom is a silent no-op on Linux — do not reach for it. webviewSetPageZoom in the shipped libNativeWrapper.so (v1.18.1, and the _cef.so, and the copy in our own build output) is a compiled empty-body stub: the disassembly has no call instruction, and webkit_web_view_set_zoom_level is not even in the library's imports. webviewGetPageZoom returns a hardcoded 1.0 from .rodata, so the failure is unobservable through the API — set 2.0, read back "1.0 = success". The JSDoc's "(WebKit only)" means macOS WKWebView. This is why the scale is CSS.
  • Everything the UI sizes with must be rem. The scale works by setting the root font-size, so a px value freezes while everything around it grows. The three px reservoirs were: the panel-chrome tokens (--row-h etc.), 93 text-[11px] arbitraries, and 7.css itself — 370 px values, zero rem, with --w7-font reaching only 15 rules' typography. 7like.css's skinfix carries a scale bridge re-expressing 7.css's vars and hardcoded box metrics in rem. 1px hairlines and focus rects stay px deliberately (crisp at any scale; the tr border exists to stop selection shifting rows).
  • Restating a 7.css rule in skinfix also outranks 7.css's VARIANTS of that rule — by layer, regardless of specificity. The title-bar-controls restatement silently shrank the close button from 48px to 29px (7.css widens it via button[aria-label=Close]), and the tablist padding flattened the active tab's 4px raise. Caught only by the harness pixel diff. When bridging a rule, grep 7.scoped.css for every variant that overrides the same properties and restate those too.
  • px constants in TS that mirror rem-sized CSS are latent scale bugs. The shadlike context menu's MENU_W = 224 was a hardcoded mirror of w-56 (14rem): box grew, math didn't, flip fired at the wrong edge. Convert through @core/metrics remPx() at use time — it reads the LIVE root font-size, and per call, since the scale changes mid-session. The editor's canvas-based charWidth is cached keyed on the computed font string for the same reason.

Hover states and the frame edge

  • A child's box-shadow is NOT clipped by its parent's border or radius. The close button's hover glow (7.css: 0 0 7px 3px #e68e75 on a ::before) painted across the window's top border and onto the desktop beyond it — the caption buttons sit flush against the top edge (border-top: 0), and the frame's rounded transparent corners clip nothing. Fixed with clip-path: inset(0 0 -1rem 0) on .w7-frame > .title-bar: glow stops at the frame like the real DWM, bottom bleed (authentic) stays open. Note clip-path also clips hit-testing — safe here because the drag region is the title-bar's own box, entirely inside the clip.
  • Hover states CAN be tested headlessly: scripts/hover-probe.ts copies the built stylesheet with :hover rewritten to .simhover, renders the frame markup with the state forced, and pixel-scans artifact regions, exiting non-zero on hover-introduced red outside the frame. Firefox --screenshot has no pointer and import -window captures the real pointer's state, so the class rewrite is the only programmatic hover on this machine. Re-run it after touching title-bar or hover styling.

The two skin trees (ui/src/app/skins/)

  • .win7 is not a name we may change. 7.scoped.css is precompiled with every selector prefixed by that literal class, so the skin id in AppSettings stays "win7" even though its folder is 7like/. The folder names are cosmetic; the class and the persisted setting value are not.
  • Reach core/ through the @core/* alias, never a relative path. Skin components sit at varying depths and get moved; the alias is what let 67 imports survive two relocations untouched. A ../../core/... path is a future merge conflict.
  • @font-face url() resolves relative to the file that declares it. When the win7 rules moved three directories deeper, ./fonts/... would silently have 404'd — build still green, app just renders a fallback font. The fonts therefore live inside the skin that uses them.
  • The @layer statement must stay the first statement in ui/src/styles.css. It is one of only two rule types allowed above @import, and it is what sorts skin above utilities. Every @import (Tailwind and both skin sheets) must also precede all other rules.
  • Duplicate selectors across skins are safe. app-file-panel exists in both trees; standalone components resolve through their importer's imports: array, never a global registry, and the trees are never mounted together.
  • The build DOES type-check components nothing imports. ngtsc compiles the whole program, so a skin that is currently unreachable still fails bun run build:ui on a type or template error. Verified deliberately by injecting an error into an unreferenced file — worth knowing before assuming a green build proves nothing about the inactive skin.

The win7 skin (7.css)

  • Unlayered declarations beat every @layer, regardless of specificity. html, body { background: … } in styles.css is unlayered, so no skinfix rule can override it — the win7 transparent-corner reset has to be unlayered too. The frame's rounded corners only read as rounded because nothing paints an opaque full-bleed ground behind them; anything that does fills the corners and the window becomes a rectangle with a curve drawn on it.
  • 7.css restates border-color at (0,3,1) via .win7 button:not(:disabled):not(:hover) and its :hover twin. A two-class selector like .win7 .w7-flat (0,2,0) loses to it, so paint suppression for flat buttons lives in skinfix where layer order settles it. The .w7-toolbar arm only ever won by tying on specificity and landing later in the same layer — an accident w7-flat did not share, which is why it rendered a bordered slab while its neighbours went flat.
  • overflow-x: auto also clips vertically. CSS refuses to pair visible with a non-visible axis, so the computed overflow-y becomes auto too. 7.css raises the active tab with margin-top: -2px; inside the horizontally scrolling tab strip that put its top border outside the scroll box and deleted it silently. The strip reserves padding-top: 2px to compensate — mirroring the padding-left: 3px 7.css already reserves for the same tab's -3px left margin.
  • 7.css hardcodes light-mode literals outside its own var system — color: #222 on tablist buttons, black/white bevels in the ListView and menu. Each one has to be routed through a --w7-* token declared in :root.win7 and overridden in both dark blocks (the [data-theme] attribute one and the prefers-color-scheme one; system dark sets no attribute).
  • The skin layer can also override position, and that breaks LAYOUT, not just paint. ul[role=menu] carries position: relative in 7.css (for the gutter stripe's ::before and submenu anchoring), which outranked Tailwind's .fixed on the app's own context menu. [style.left/top.px] then became flow offsets from the menu's static position — the last child of the h-screen frame — so every right-click menu rendered below the viewport, unreachable behind overflow: hidden. The backdrop is a plain <div>, which 7.css does not match, so it stayed genuinely fixed and ate the next click: the symptom was "right-click does nothing, then the app ignores me once". Fixed with a position: fixed restatement in skinfix, scoped to a w7-ctx marker class so 7.css's own menus keep theirs. Regressed in 0ae2a62, when the menu root became a real ul[role=menu] — i.e. adopting the framework's markup is what exposed it. A resting pixel diff cannot catch this class of bug: nothing about the idle window changes. scripts/menu-probe.ts renders the menu at declared coordinates and scans whether it landed there, with a marker-less control menu as its own discriminator.
  • Its selectors are exact, and a near-miss looks exactly like "7.css doesn't support this". menu[role=tablist] button is a DESCENDANT selector, so every button inside the strip becomes a tab — the + and the right-hand tools must live outside it. .window.glass > .title-bar is direct-child, so an inner wrapper breaks the glass coupling. A wrong guess here already produced one hand-written lookalike that was deleted.

Electrobun

  • Window close is not cancellable (response type {}; only the app-level before-quit supports event.response = {allow:false}). All unsaved-state guards must either intercept app-controlled close paths (⌘W, buttons) or survive the close (editor draft mirroring).
  • One rpc instance per window — BrowserView binding steals the rpc's transport; a shared instance silently sends to the wrong webview.
  • RPC before socket open is silently lost. Everything gates on the socket-open promise (RpcService ready).
  • The emitter has no off(). Window event closures must not capture records you want garbage collected.
  • electrobun build defaults to the dev channel; release builds need --env=stable. The stable app name differs from the dev one (miniSCP vs miniSCP-dev), including the app-data dir implications when testing.
  • Tray: never call setImage after construction on macOS — the template treatment is lost and the icon renders black on dark menu bars.
  • ApplicationMenu is macOS-only in our usage; accelerators on Windows are single-character only.
  • ContextMenu.showContextMenu is a silent no-op on Linux (electrobun's own dist/api/bun/proc/linux.md says so; the app also prints "Application menus are not supported on Linux" at boot). Because the UI suppresses the webview's own right-click menu, that left EVERY panel context menu dead on Linux — no error, nothing happens. The menu is plain HTML now (ui/src/app/skins/*/context-menu.ts) on every platform; don't reintroduce the native one for "nativeness" on mac/win without keeping a Linux path.

Linux

  • ERR_DLOPEN_FAILED on bun run start means a missing system library, not broken app code. libNativeWrapper.so links libayatana-appindicator3.so.1 hard (for the tray), so the app cannot start at all without it — even though the tray is a minor feature. Diagnose with ldd build/dev-linux-x64/*/bin/libNativeWrapper.so | grep 'not found'; the libasar.so => not found line in that output is a red herring (it ships beside the wrapper and resolves at load). Install list per distro is in platform-notes.md.
  • Black window on NVIDIA + Failed to create GBM buffer of size WxH: Invalid argument (usually with X11 Error: GLXBadWindow (code 170)). The app boots, logs "miniSCP started", reports no error — and paints nothing. WebKitGTK's DMABUF renderer can't allocate buffers on the NVIDIA proprietary driver; WEBKIT_DISABLE_DMABUF_RENDERER=1 is the upstream workaround.
    • The app cannot apply this to itself. WebKit reads the variable when it forks its web process, and Bun's process.env writes do not propagate to child processes (verified: a spawned child sees it unset). Setting it in src/bun/index.ts — even in a module imported before electrobun/bun — does nothing. It must be in the environment before the app process starts.
    • Hence every launch path injects it: scripts/linux-gpu.ts (launchEnv()) for bun run dev / bun run start, and the miniSCP.sh wrapper that release.ts writes beside bin/launcher in the tarball. A new launch path must call launchEnv() or it will render black on NVIDIA.
    • Gated on /proc/driver/nvidia so Intel/AMD keep hardware compositing, and an explicit WEBKIT_DISABLE_DMABUF_RENDERER from the user always wins.
    • A "does it boot?" log check does NOT catch this — grep for the GBM line, or look at the window.
  • bun run start leaves the old instance running, and import -window miniSCP grabs the OLDEST match. Instances pile up across rebuilds, so every screenshot silently comes from a stale build and the fix you just made looks like it did nothing. Cost three wrong readings in one session, including a "the CSS is in the bundle but does not apply" dead end. Kill first (pkill -f dev-linux-x64), confirm with pgrep -cf 'dev-linux-x64/miniSCP-dev/bin/launcher', then launch and capture. Note pkill -f 'electrobun dev' also matches the shell running it and kills your own command (exit 144) — match on the build path instead. That is only half the trap: pkill -f dev-linux-x64 does the same thing, because the pattern appears in the killing command's OWN argv. Break the literal with a character class — pkill -f 'dev-linux-x[6]4' — the way ps | grep [x]yz has always worked.
  • Prefer the headless probes over launching the app. bun run start opens a real window that takes focus, which interrupts whatever the user is doing; there is no Xvfb/xvfb-run on this machine and Wayland offers no launch-without-focus. scripts/shot.ts (+ --dark), hover-probe.ts and menu-probe.ts all render through headless Firefox and open nothing. Launch the real app only for what Gecko genuinely cannot answer — WebKitGTK behaviour, the native frame, transparency, live RPC — ask first, and batch every live check into a single launch.
  • X11 capture cannot answer "is this pixel transparent?" for the frameless window. import -window grabs the window's own pixels with no compositing, so a transparent corner and an opaque black one both come out black; png32: just fills alpha with 1 and proves nothing. XWayland's root window is not the composited desktop, so import -window root fails too. Frame transparency and the maximized corners need a human looking at the screen — say so rather than reporting a screenshot as confirmation.
  • Benign startup noise on Linux — non-fatal, the app runs normally: libayatana-appindicator is deprecated. Please use libayatana-appindicator-glib (upstream electrobun's choice, not ours).
  • Window-open flash prevention is three-layer — remove any one and the white/inverted blink returns: (1) windows are created hidden: true and revealed by showWhenReady on the webview's first dom-ready (800 ms fallback), (2) the bun preload bakes the persisted data-theme/tone/accent attributes before first paint (buildPreload), (3) ui/src/index.html carries an inline <style> with the html background per theme — its oklch values MUST stay in sync with the --background tokens in styles.css. The win7 skin adds a fourth value to keep in sync: buildPreload also bakes the .win7 class, and index.html carries a matching html.win7 arm for all four theme states. Miss it and every win7 window flashes white.
  • The skin is a LAUNCH-TIME choice. Never apply one to a running window. This shipped broken once: SettingsService.applySkin toggled the .win7 class the moment the setting changed, which was harmless when there was one component tree and became a broken window as soon as there were two — 7.css painted onto shadlike markup that has no .window.glass, no .title-bar, no menu[role=tablist] and no <table> ListView. Each skin's CSS is written against its OWN tree's markup; the class must only ever be written by buildPreload, for the tree that window actually booted.
    • A reload does not rescue it. The preload is a persistent WebKitUserScript injected at DOCUMENT_START, so its string is frozen at webview construction — webview.loadURL() re-runs it with the OLD __APP_SKIN. (updatePreloadScriptToWebView exists in the FFI but is reachable only from electrobun's internal <electrobun-webview> handler.)
    • titleBarStyle/transparent are construction-time only — no setters exist, so a live window can never be re-framed. win7 → modern is the dangerous direction: the modern tree draws no title bar, so it would land in a frameless transparent window with no way to move, minimize or close.
    • Don't "fix" this by recreating the main window either: exitOnLastWindowClosed is true, the main rpc is captured by six push closures, the tray captures win, listeners cannot be removed (no off()) — and it would drop every session and tab.
    • Bun pins it: initSessionSkin freezes the skin before any window exists, and buildPreload/chromeOptions read that, never the live settings. Skip the pin and a window opened after a pending change boots the other skin.
  • Dev vs prod URL: getMainViewUrl() resolves the ng-serve dev server only when DEV_SERVER_URL is set AND the updater channel is dev AND the server answers a HEAD request; secondary windows must reuse the resolved URL, never hardcode views://.
  • Upstream Bun panic (FFI_Callback_threadsafe_call → web_worker.spin): rare segfault under long sessions; upstream electrobun×Bun issue, restart recovers, nothing app-side to fix.

Domain logic

  • sessionId ≠ filesystem identity. Two panels on the same server are two SSH sessions. Every same-file/nested-path guard compares fsKey (resolvedIP:port / "local"); comparing sessionId caused a truncate-to-zero data-loss bug. When fsKeys differ but paths collide, the probe-file check unmasks aliased hosts. Never "simplify" this back.
  • createWriteStream truncates BEFORE the source is read — that's why self-copy is destructive, and why guards must run before any stream opens.
  • The synthetic '..' row: type 'dir', must be excluded from selection, targets(), filtering match-count, kind/rights/owner display, and drag sources. Any new row-level feature must decide what '..' does.
  • Semaphore.wake() must reserve slots synchronously; waking waiters that increment asynchronously loses budget under concurrent release.
  • Settings: the backend spread-merges ({...DEFAULT_SETTINGS, ...data}) so new keys auto-default, but the UI SettingsService reads field-by-field — a new setting silently stays at default until wired in all five spots.
  • The editor draft mirror sends full content per tick (2 MB cap upstream); don't shrink the 400 ms debounce.
  • panel.touched gates localDir auto-navigation; connectToken guards against a slow connect resolving into a panel the user already reused.

i18n

  • Translations live in src/shared/i18n/*.ts (TS, not JSON) — deliberate: it sidesteps resolveJsonModule differences between the Angular esbuild and Bun builds and gives full typing. en.ts is the source of truth; other locales are partial and fall back to English then the raw key.
  • i18n.t() is reactive because it reads the locale signal — that's what makes live language switching work with no pipe. If you cache a translated string in a plain field it WON'T update on switch; call t() in the template (or in a signal-reading getter) instead. Same for i18n.date()/size().
  • Dictionary values are TS string literals: French/other apostrophes break single-quoted entries ('…c'est…'). Use double quotes for any value containing an apostrophe. Plurals are { one, other } maps (not ICU strings); zh/ja supply only other.
  • Native menus can't use the Angular service — menu.ts goes through backend i18n.ts, and the app menu must be rebuilt when the locale changes (wired in index.ts settingsChanged), or it keeps the old language. The context menu no longer has this problem: it is Angular now and reads I18nService directly.
  • eu/zh/ja are AI drafts pending native review (see the i18n README).
  • Silent auto-refresh must snapshot sessionId+cwd and discard a mismatched result. A normal refresh() is masked by busy (input blocked), but the silent variant (poll/watch) runs while the user can navigate — without the guard, a poll that started in dir A resolves and stamps A's listing over the dir B the user just opened. It also never calls scrollCursorIntoView (scroll must stay put) and returns early if busy() or already refreshing.

Build / tooling

  • Angular CLI rejects Bun's Node-compat version — scripts/node-version-shim.ts is preloaded via bunfig.toml at the root AND in ui/. Removing either breaks ng.
  • ssh2's optional native deps must stay external: ["cpu-features", "*.node"] in electrobun.config.ts — bundling them breaks the build; ssh2 falls back to pure JS at runtime.
  • bun run --cwd ui build (flag order matters; bun --cwd ui run build does not do what you want).
  • version.gen.ts is gitignored and regenerated by postinstall + every build script; a fresh checkout fails tsc until bun install (or any script) has run once. Never commit it — it would bump the commit count it records.
  • Icon pipeline is scripts/gen-icons.ts (resvg) — regenerate after touching assets/logo.svg or assets/tray/tray-glyph.svg; the .ico and tray PNGs are build inputs, not build outputs.