diff options
| author | Agando <roduccomm@gmail.com> | 2026-11-07 19:03:38 +0100 |
|---|---|---|
| committer | Agando <roduccomm@gmail.com> | 2026-11-07 19:03:38 +0100 |
| commit | 05d72563637e88825ff852d803621c567de47e34 (patch) | |
| tree | 9ac21e6a6bcce3983d7df1a8304bb2896f2a9afa /modules/core/module-uishell.js | |
| parent | e0b87c7b5a902dea69ac45ffa2e525387edebf42 (diff) | |
fix(newos4): second review round — identity for every stock node, load dedup, sweep race, a11y
Four models (kimi-k2.7-code, deepseek-v4-pro, minimax-m3, glm-5.3) reviewed the shipped change
set. Review log with every accepted and rejected finding:
.hermes/plans/2026-11-07_163420-newos4-six-ui-modes.review-log.md
- identity guard now covers every snapshot node, not just #terminal-container: terminal.js also
binds #output, #command-input, #input-area and #status-bar, so a cloned one leaves the terminal
writing into a detached node while the page looks perfect
- _restoreStock anchors on the next stock node already home, then on the recorded trailing
sibling; a raw child index was measured wrong (terminal,input,status-bar,quick-actions)
- _loading map: a double-clicked card or a repeated 'ui easy' loads the mode once
- _sweepPending returns while that id is in flight instead of declaring failure at 6 s and
overwriting the stored preference
- _syncChrome() on every switch (quick-actions was left in the tab order behind the mode root)
- picker: role=dialog/aria-modal/labelledby/focus, shown after 600 ms instead of waiting for a
registration that never happens with lazy modes, and suppressed when ?ui= already applied
- ?ui= must be one of the six; _loadMode only accepts packages['ui-<id>']; hasOwnProperty on modes
- VFS names are shell-quoted before reaching api.exec (cat/edit)
- refresh generation no longer bumps per tick (slow captures left later panes at '...')
- easy: padding shorthand reset the 34 px footer reserve (footer overlapped the composer)
- cockpit panes get tabIndex and focus; F1/F12 left to the browser; nothing swallowed in inputs
- api.el lost its unused innerHTML sink; the test lost its _parseArgs stub (it hid the regression)
- v4.6.155
Diffstat (limited to 'modules/core/module-uishell.js')
| -rw-r--r-- | modules/core/module-uishell.js | 80 |
1 files changed, 52 insertions, 28 deletions
diff --git a/modules/core/module-uishell.js b/modules/core/module-uishell.js index 9914ec0..11bad38 100644 --- a/modules/core/module-uishell.js +++ b/modules/core/module-uishell.js @@ -19,6 +19,7 @@ class UIShellModule { this._cleanup = null; this._where = null; // [{ el, parent, next }] captured on first mount this._loadSeq = 0; // bumps on every apply(): a stale lazy load must never win + this._loading = {}; // id -> in-flight promise, so a double click loads a mode once this._terminalEl = null; // identity of the one real output node this._cssIds = []; this._pending = null; // mode requested before its ext module loaded @@ -52,7 +53,7 @@ class UIShellModule { const q = new URLSearchParams(location.search).get('ui'); // Validate before it reaches classList.add('wx11-ui-' + id): a space or a colon in the // query throws a DOMException there and boot dies with no interface at all. - if (q && /^[a-z][a-z0-9-]{0,15}$/.test(q)) { wanted = q; this._fromQuery = true; } + if (q && this.lazyIds().indexOf(q) !== -1) { wanted = q; this._fromQuery = true; } } catch (e) { /* no URLSearchParams: ignore the request */ } // persist:false — a shared link requests a view, it must not rewrite the user's own default // (the code comment already claimed this; the flag did the opposite). @@ -63,22 +64,17 @@ class UIShellModule { // kept as a dead preference forever. this._sweepT = setTimeout(() => this._sweepPending(), 6000); - // First run: show the picker only once the mode modules have actually registered, otherwise - // the user can tap a card before its module exists and get "not loaded". - if (!localStorage.getItem(this.seenKey)) { - let waited = 0; - const tryShow = () => { - if (localStorage.getItem(this.seenKey)) return; - waited += 400; - if (Object.keys(this.modes).length >= 4 || waited >= 4000) { this.showPicker(); return; } - this._pickerT = setTimeout(tryShow, 400); - }; - this._pickerT = setTimeout(tryShow, 800); + // First run: show the picker at once. Waiting for four registered modes was a dead condition — + // the six mode files are lazy now, so nothing registers before the user picks one, and every + // first-run visitor paid a 4.4 s delay for a list that was static anyway. + if (!localStorage.getItem(this.seenKey) && this.current === 'classic' && !this._pending) { + this._pickerT = setTimeout(() => this.showPicker(), 600); } } _sweepPending() { if (!this._pending) return; + if (this._loading[this._pending]) return; // still in flight: the load's own callback decides if (this.modes[this._pending]) return; // registered late: apply() already ran if (this.terminal) { this.terminal.print('Interface mode \'' + this._pending + '\' did not load — staying on ' + @@ -104,6 +100,10 @@ class UIShellModule { list() { return Object.keys(this.modes).map(id => this.modes[id]).sort((a, b) => a.level - b.level); } + // The six modes exist even before their files are fetched, so `ui list` / `ui next` / the picker + // must not be limited to what happens to be registered. One list, not four copies. + lazyIds() { return ['easy', 'chat', 'studio', 'workspace', 'commander', 'cockpit']; } + /* ---------- switching ---------- */ apply(id, opts) { const o = opts || {}; @@ -114,10 +114,12 @@ class UIShellModule { // A user-initiated switch cancels a pending boot restore, or a slow ext module would later // yank the interface back to whatever boot asked for. if (!o.silent) { this._pending = null; this._closePicker(); } - if (want !== 'classic' && !this.modes[want]) { - // Lazy load. The six mode files are NOT in extKeys: a full eager boot issues ~55 - // requests and trips nginx `limit_req zone=webx11 (60r/m, burst 50)`, which 503s every - // module after the limit — measured 2026-11-07 in the dev copy. + // hasOwnProperty: `this.modes` is a plain object, so ?ui=constructor / __proto__ passed the + // truthiness check and reached the mount path without any mode registering. + if (want !== 'classic' && !Object.prototype.hasOwnProperty.call(this.modes, want)) { + // Lazy load. The six mode files are NOT in extKeys: a full eager boot issues ~40-55 + // requests and the per-IP `limit_req` bucket (then burst=50, now 200) 503'd everything + // after it — measured 2026-11-07. if (o.retried) { if (this.terminal) this.terminal.print(`UI mode '${want}' is not available.`, 'warning'); return false; @@ -156,6 +158,8 @@ class UIShellModule { document.body.classList.add('wx11-ui-' + want); if (o.persist !== false) localStorage.setItem(this.key, want); + // _syncChrome on EVERY switch: it hides #quick-actions, which otherwise stays in the tab order + // and the accessibility tree behind the opaque mode root (found by review). if (want === 'classic') { this._hideFooter(); this._syncChrome(); return true; } const api = this.api(); @@ -171,13 +175,19 @@ class UIShellModule { // Identity, not just presence: a mode that appends its own <div id="terminal-container"> // would pass a presence check while terminal.js keeps printing into the detached original. - const out = document.getElementById('terminal-container'); - const inDoc = !!out && !!document.body.contains(out); - if (!out || !inDoc || (this._terminalEl && out !== this._terminalEl)) { - this._abortToClassic(want, 'removed or replaced the terminal pane'); + // Identity for EVERY stock node the terminal bound, not just the output pane: terminal.js also + // holds #output, #command-input, #input-area and #status-bar, so a mode that clones one of those + // leaves the terminal writing into a detached node while the page looks fine. + const lost = (this._where || []).filter((w) => { + const live = document.getElementById(w.el.id); + return !live || live !== w.el || !document.body.contains(w.el); + }); + if (lost.length) { + this._abortToClassic(want, 'removed or replaced ' + lost.map((w) => '#' + w.el.id).join(', ')); return false; } this._dockFooter(); + this._syncChrome(); // Every mode hides .prompt-label, so without this the real input has no accessible name. const inp = document.getElementById('command-input'); if (inp && !inp.getAttribute('aria-label')) inp.setAttribute('aria-label', 'Terminal command input'); @@ -204,7 +214,7 @@ class UIShellModule { } next() { - const order = ['classic'].concat(this.list().map(m => m.id)); + const order = ['classic'].concat(this.lazyIds()); const i = order.indexOf(this.current); return this.apply(order[(i + 1) % order.length]); } @@ -213,6 +223,13 @@ class UIShellModule { // never be served stale after a version bump. Public moduleManager methods only, and no // printing: a mode load must not litter the chat. async _loadMode(id) { + if (this._loading[id]) return this._loading[id]; // one load per id, however many taps + const p = this._loadModeOnce(id).finally(() => { delete this._loading[id]; }); + this._loading[id] = p; + return p; + } + + async _loadModeOnce(id) { const mm = this.terminal && this.terminal.moduleManager; if (!mm) return this._loadFail(id, 'no module manager'); const ver = localStorage.getItem('webx11_cache_ver') || mm.LATEST_VER || '0'; @@ -222,7 +239,9 @@ class UIShellModule { const idx = await (await fetch('packages.json?v=' + ver, { cache: 'no-store' })).json(); // Mode ids are bare (`easy`); package keys are prefixed (`ui-easy`). Looking up only the // bare id reported "no package entry" for every mode — measured 2026-11-07. - pkg = idx && idx.packages && (idx.packages['ui-' + id] || idx.packages[id]); + // Only `ui-<id>`: without this a crafted ?ui=calc fetches and initialises an unrelated ext + // module through the mode loader (same-origin, full page access). + pkg = idx && idx.packages && idx.packages['ui-' + id]; } catch (e) { return this._loadFail(id, 'packages.json unreadable: ' + e.message); } @@ -234,7 +253,7 @@ class UIShellModule { // load looks exactly like a broken button. let ok = await mm.loadModuleFromURL(url); if (!ok) { - await new Promise(r => setTimeout(r, 900)); + await new Promise(r => setTimeout(r, 1200)); // >= 1 token at 60 r/m before retrying ok = await mm.loadModuleFromURL(url); } if (!ok) return this._loadFail(id, 'loader refused ' + url); @@ -275,6 +294,7 @@ class UIShellModule { el, parent: el.parentNode, index: Array.prototype.indexOf.call(el.parentNode.children, el), + next: el.nextSibling || null, // trailing anchor: what followed the last stock node })); } @@ -287,7 +307,10 @@ class UIShellModule { const list = this._where.slice().sort((a, b) => a.index - b.index); list.forEach((w) => { if (!w.parent || w.el.parentNode === w.parent) return; // already in place - const anchor = list.find((c) => c.index > w.index && c.el.parentNode === w.parent); + // Anchor on the next stock node already home; for the LAST stock node fall back to the + // sibling that followed it originally, or the group lands after every trailing body child. + const anchor = list.find((c) => c.index > w.index && c.el.parentNode === w.parent) + || (w.next && w.next.parentNode === w.parent ? w.next : null); try { w.parent.insertBefore(w.el, anchor ? anchor.el : null); } catch (e) { /* detached */ } }); } @@ -341,7 +364,7 @@ class UIShellModule { // clip, not hidden: `overflow:hidden` still creates a scroll container, and the terminal's // scrollIntoView() on new output scrolled #ui-root by 464px — the title, the action cards // and the composer all left the screen (measured 2026-11-07). `clip` cannot be scrolled. - '#ui-root{position:fixed;inset:0;display:none;overflow:clip;z-index:40;background:var(--bg-color);color:var(--text-color)}', + '#ui-root{position:fixed;inset:0;display:none;overflow:hidden;overflow:clip;z-index:40;background:var(--bg-color);color:var(--text-color)}', // flex column, not block: every mode is a vertical stack (chrome / content / composer / // footer) and a block container makes each child's `flex:1 1 auto` inert — the terminal // collapsed to 0px and the composer sat at the very bottom of a 4 600px flow. @@ -349,7 +372,7 @@ class UIShellModule { '#ui-root > *{min-width:0;min-height:0}', // A non-classic mode owns the viewport: the page itself must not scroll, or terminal // output drags the action cards off-screen and the composer never comes back. - 'body[data-ui-mode]:not([data-ui-mode="classic"]){overflow:clip}', + 'body[data-ui-mode]:not([data-ui-mode="classic"]){overflow:hidden;overflow:clip}', 'body[data-ui-mode]:not([data-ui-mode="classic"]) #ui-root{overflow:clip}', 'body[data-ui-mode]:not([data-ui-mode="classic"]) #cmd-cards{display:none!important}', // A mode IS the onboarding: six cards behind the classic "Let's Get Started" modal is two @@ -435,7 +458,8 @@ class UIShellModule { const node = document.createElement(tag); if (props) for (const k of Object.keys(props)) { if (k === 'text') node.textContent = props[k]; - else if (k === 'html') node.innerHTML = props[k]; + // No `html` sink: zero modes needed it, and it was the only innerHTML entry point in + // the capability API. Dynamic text goes through textContent. else if (k === 'class') node.className = props[k]; else node.setAttribute(k, props[k]); } @@ -561,7 +585,7 @@ class UIShellModule { if (sub === 'next') return void this.next(); // A mode that is not registered yet is not an error: it is lazy-loaded on demand. Only an // id that is not one of the six is unknown. - const LAZY = ['easy', 'chat', 'studio', 'workspace', 'commander', 'cockpit']; + const LAZY = this.lazyIds(); if (available.indexOf(sub) === -1 && LAZY.indexOf(sub) === -1) { this.terminal.print(`Unknown mode '${sub}'. Available: ${available.join(', ')}`, 'warning'); return; |