fix(panes): fail fast when the pane window is unreachable; validate opts.header

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 <topkoa@gmail.com>
This commit is contained in:
topkoa
2026-07-12 20:36:10 -04:00
parent 5049be0523
commit 3a50e593bf
2 changed files with 29 additions and 1 deletions
+7
View File
@@ -141,6 +141,13 @@
function attachChip(el, paneId, opts) { function attachChip(el, paneId, opts) {
opts = opts || {}; opts = opts || {};
if (!(el instanceof Element)) throw new TypeError('panes.attachChip: el must be an Element'); 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); const spec = panes.get(paneId);
if (!spec) { console.warn('[panes] attachChip: register the pane first:', paneId); return () => {}; } if (!spec) { console.warn('[panes] attachChip: register the pane first:', paneId); return () => {}; }
if (attached.has(paneId)) { console.warn('[panes] attachChip: already attached:', paneId); return () => {}; } if (attached.has(paneId)) { console.warn('[panes] attachChip: already attached:', paneId); return () => {}; }
+22 -1
View File
@@ -103,9 +103,19 @@
// So we do not trust readyState, and we do not trust 'load' (which may have // 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 // 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. // 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) { function _whenReady(w, onReady, onFail) {
const deadline = performance.now() + 10000; 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 = () => { const tick = () => {
if (w.closed) return; if (w.closed) return;
@@ -117,9 +127,20 @@
// browsing-context group), and no amount of waiting will fix it — // browsing-context group), and no amount of waiting will fix it —
// adoptNode can never work. // adoptNode can never work.
doc = null; doc = null;
if (!reachFailure) reachFailureAt = performance.now();
reachFailure = e; 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') { if (doc && doc.readyState !== 'loading') {
// Only ever adopt into the document we actually navigated TO. // Only ever adopt into the document we actually navigated TO.
// about:blank reports readyState 'complete' from the moment // about:blank reports readyState 'complete' from the moment