From 3a50e593bfdd35184da86580c4f82fc170963e13 Mon Sep 17 00:00:00 2001 From: topkoa Date: Sun, 12 Jul 2026 20:36:10 -0400 Subject: [PATCH] fix(panes): fail fast when the pane window is unreachable; validate opts.header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two from review. 1. _whenReady's own comment said a SecurityError means the pop-out is not reachable from this realm and "no amount of waiting will fix it" — and then it waited the full 10s deadline anyway. Ten seconds of a detached panel and a half-popped-out UI, for a condition we had already diagnosed as fatal. It now gives up after a 1s grace instead. Not instantly, deliberately: a throw *during* the navigation from about:blank to /pane would otherwise take down a pop-out that was about to work perfectly. A second is far more than that transition needs and far less than a user should spend staring at a detached panel. 2. attachChip() took opts.header on trust. It's a public plugin API, and a truthy non-Element header (a selector string, a wrapper object, a ref) is an easy mistake — one that surfaced as a confusing DOM exception from deep inside core instead of a TypeError naming the offending pane. Signed-off-by: topkoa --- static/panes/pane-chip.js | 7 +++++++ static/panes/pane-window-host.js | 23 ++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/static/panes/pane-chip.js b/static/panes/pane-chip.js index a079067..bfe4015 100644 --- a/static/panes/pane-chip.js +++ b/static/panes/pane-chip.js @@ -141,6 +141,13 @@ function attachChip(el, paneId, opts) { opts = opts || {}; if (!(el instanceof Element)) throw new TypeError('panes.attachChip: el must be an Element'); + // Validate here, not at the insertBefore below. This is a public plugin API, + // and a truthy non-Element `header` (a selector string, a jQuery-ish wrapper, + // a ref object) is an easy mistake to make — one that would otherwise surface + // as a confusing DOM exception from deep inside core. + if (opts.header != null && !(opts.header instanceof Element)) { + throw new TypeError('panes.attachChip(' + paneId + '): opts.header must be an Element'); + } const spec = panes.get(paneId); if (!spec) { console.warn('[panes] attachChip: register the pane first:', paneId); return () => {}; } if (attached.has(paneId)) { console.warn('[panes] attachChip: already attached:', paneId); return () => {}; } diff --git a/static/panes/pane-window-host.js b/static/panes/pane-window-host.js index 4201a64..df5148e 100644 --- a/static/panes/pane-window-host.js +++ b/static/panes/pane-window-host.js @@ -103,9 +103,19 @@ // So we do not trust readyState, and we do not trust 'load' (which may have // fired for about:blank before we could listen). We wait for the one thing that // only exists in the document we actually want: pane.html's #fb-pane-root. + // How long a "we cannot even see the pop-out's document" condition has to persist + // before we call it fatal. A SecurityError means the window is not reachable from + // this realm at all, and waiting cannot fix that — but we give it a moment anyway + // rather than bailing on the first tick, because a throw *during* the navigation + // from about:blank to /pane would otherwise take down a pop-out that was about to + // work perfectly. A second is far more than that transition needs, and far less + // than the 10s a user would otherwise stare at a detached panel for. + const UNREACHABLE_GRACE_MS = 1000; + function _whenReady(w, onReady, onFail) { const deadline = performance.now() + 10000; - let reachFailure = null; // why we could never see the pop-out's document + let reachFailure = null; // why we could never see the pop-out's document + let reachFailureAt = 0; // when we first couldn't const tick = () => { if (w.closed) return; @@ -117,9 +127,20 @@ // browsing-context group), and no amount of waiting will fix it — // adoptNode can never work. doc = null; + if (!reachFailure) reachFailureAt = performance.now(); reachFailure = e; } + // Unreachable, and it has stayed that way. Fail now rather than leaving + // the panel detached and the UI mid-pop-out for the full 10s deadline, + // when we already know this can never succeed. + if (reachFailure && !doc && performance.now() - reachFailureAt > UNREACHABLE_GRACE_MS) { + onFail(new Error('the pane window\'s document is NOT reachable from this window (' + + reachFailure.name + ': ' + reachFailure.message + + ') — it is in a separate process, so the element cannot be moved into it')); + return; + } + if (doc && doc.readyState !== 'loading') { // Only ever adopt into the document we actually navigated TO. // about:blank reports readyState 'complete' from the moment