fix(panes): get the element out before the pane window's document dies

Docking a popped-out panel brought it home DEAD. It rendered perfectly —
right markup, right size, right place — and every control in it was inert:
the close button, the sliders, the presets, even the pop-out chip. A
photograph of a panel.

Closing a pane window tears down its document, and the panel was still
inside it. The node itself survives (the manager holds a reference), but
every event listener in its subtree goes with the document that hosted
them. Two paths did this:

  1. closePane() called the host's unplace() — which closes the window —
     BEFORE adopting the element back. Order is now reversed, and the
     comment says why so nobody helpfully "tidies" it back.

  2. The user closing the pane window themselves was only noticed by the
     `closed` poll, which by definition runs AFTER the document is gone.
     The window now gets a `beforeunload` listener that brings the element
     home while its document is still alive.

That listener has to be attached AFTER /pane loads: window.open() hands
back a throwaway about:blank document, and anything registered on it is
discarded when the real page replaces it. This is the same trap that made
the pane window blank in the first place — adopt into about:blank and the
panel is destroyed a moment later — and it is now handled in both places.

The `closed` poll stays, but only as a last-resort net for a CRASHED pane
window, where nothing can be saved.

Also fixed while chasing this:

  - The chip stamped `.fb-pane-detached` (display:none !important) onto the
    element to hide it in the main window — and that element is the one we
    move, so the class travelled with it and blanked the pane window. The
    chip now only hides an element the pane did NOT take, and marks the hole
    with its stub otherwise. "Did not take" is an ownerDocument test, not
    isConnected: a panel sitting in a pane window IS connected, just not
    here, and a plugin that rebuilds its panel (Camera Director does, on
    every mode change) re-runs attachChip while popped out.

  - The stub was inserted "before the element", which is nowhere — the
    element has left the document. The manager now hands over the element's
    recorded home, and the stub goes there.

  - GET /pane sent no cache headers. A stale copy is especially nasty here:
    the opener waits for an element inside that page before adopting, so an
    old cached version means the pane window just sits there blank.

Verified in the desktop app: pop out, use the controls in the pane window,
dock back, use them again. Panel comes home alive.

Signed-off-by: topkoa <topkoa@gmail.com>

server.py

@@ -1719,9 +1719,14 @@ def index_v3():
 
 @app.get("/pane")
 def pane_host():
-    # The document a popped-out pane runs in. Deliberately NOT the app shell with
+    # The document a popped-out pane is displayed in. It builds nothing: the opener
-    # a query flag (the splitscreen follower's approach): that loads the library,
+    # MOVES the real panel element into it (document.adoptNode) and copies the app's
-    # the highway and the whole v3 shell only to hide them again, and pays for it
+    # stylesheets across. See docs/plugin-panes.md.
-    # with anti-flash hacks in three files. This page loads the pane runtime and
+    #
-    # nothing else. See docs/plugin-panes.md.
+    # no-cache, matching the /static mount's contract (_RevalidatedStaticFiles). A
-    return FileResponse(str(STATIC_DIR / "panes" / "pane.html"))
+    # stale copy of this page is especially nasty: the opener waits for an element
+    # inside it before adopting, so an old cached version means the pane window
+    # simply sits there blank.
+    resp = FileResponse(str(STATIC_DIR / "panes" / "pane.html"))
+    resp.headers["Cache-Control"] = "no-cache"
+    return resp

static/panes/pane-chip.js

@@ -2,21 +2,27 @@
  * fee[dB]ack — the pop-out chip.
  *
  * One affordance, core-owned, identical everywhere: the small ⇱ button a plugin
- * drops into a dialog it already has.
+ * drops into the panel it already has.
  *
- *     feedBack.panes.register({ id: 'camera_director', title: 'Camera', mount, unmount });
+ *     feedBack.panes.register({ id: 'camera_director', title: 'Camera', element: () => panelEl });
- *     feedBack.panes.attachChip(myDialogEl, 'camera_director');
+ *     feedBack.panes.attachChip(panelEl, 'camera_director');
  *
- * That is the entire adoption cost. Clicking the chip opens the pane in its host
+ * That is the entire adoption cost. Clicking the chip pops the panel out; a stub
- * and hides `myDialogEl`; a stub takes its place so the user can find it again;
+ * takes its place so the user can find it again; closing the pane brings the panel
- * closing the pane un-hides the dialog and restores the chip. The plugin writes
+ * home and restores the chip. The plugin writes no show/hide logic — if it did,
- * no show/hide logic — if it did, every plugin would invent a slightly different
+ * every plugin would invent a slightly different one, which is exactly the
- * one, which is exactly the inconsistency this exists to prevent.
+ * inconsistency this exists to prevent.
  *
- * Hiding uses `.fb-pane-detached`, NOT the `hidden` class or `[hidden]`, because
+ * The panel a chip is attached to is USUALLY the very element the pane moves into
- * the dialogs being hidden here already toggle those themselves (the core mixer
+ * the pop-out window — so most of the time there is nothing here left to hide, and
- * popover, every rail popover). Two owners of one class is a bug waiting for a
+ * the job is simply to mark the hole it left. Hiding it would in fact be actively
- * bad day; a dedicated class composes cleanly with whatever the dialog does.
+ * harmful: `.fb-pane-detached` is `display:none !important`, and it would travel
+ * with the node straight into the pane window and blank it.
+ *
+ * When the chip IS attached to something the pane didn't take (a wrapper, a
+ * launcher row), that element stays put and is hidden with `.fb-pane-detached` —
+ * a dedicated class, not `.hidden`/[hidden], because the panels we attach to
+ * already toggle those themselves.
  */
 (function () {
     'use strict';
@@ -69,12 +75,46 @@
         return s;
     }
 
-    function _onOpened(rec) {
+    // The pane is out. Leave a stub where its panel used to be.
-        rec.el.classList.add('fb-pane-detached');
+    //
-        if (!rec.stub.isConnected) rec.el.parentNode.insertBefore(rec.stub, rec.el);
+    // The subtlety: the panel a chip is attached to is USUALLY the very element the
+    // pane moved into the pop-out window. It is no longer in this document at all —
+    // so hiding it would be worse than pointless (the `display:none` travels with
+    // the node and blanks the pane window, which is exactly the bug this fixes), and
+    // the stub cannot be inserted "before it", because it is not here to be before.
+    //
+    // Hence `home`: the manager tells us where the element used to live, and the
+    // stub goes there. If the chip is attached to something the pane did NOT take —
+    // a wrapper, a launcher row — that element is still here, and we hide it as
+    // before.
+    function _onOpened(rec, detail) {
+        // "Moved" means the element is not in THIS document — either because this
+        // open took it, or because it is already sitting in a pane window from an
+        // earlier one. The ownerDocument test is what makes re-attaching a chip
+        // safe: a plugin that rebuilds its panel (Camera Director does, on every
+        // mode change) re-runs attachChip while the pane is still popped out, and
+        // an isConnected test would say "still here" — it IS connected, to the pane
+        // window — and we would stamp display:none onto the live pane.
+        const moved = (detail && detail.el === rec.el) || rec.el.ownerDocument !== document;
+
+        if (!moved && rec.el.isConnected) {
+            rec.el.classList.add('fb-pane-detached');
+            if (!rec.stub.isConnected && rec.el.parentNode) rec.el.parentNode.insertBefore(rec.stub, rec.el);
+            return;
+        }
+
+        // Mark the hole the element left. `home` comes with the event, or from the
+        // manager when we are reconciling after the fact.
+        const home = (detail && detail.home) || panes.homeOf(rec.spec.id);
+        if (!rec.stub.isConnected && home && home.parent && home.parent.isConnected) {
+            const next = (home.next && home.next.parentNode === home.parent) ? home.next : null;
+            home.parent.insertBefore(rec.stub, next);
+        }
     }
 
     function _onClosed(rec) {
+        // The element is back. Whatever we did to hide it, undo — including a class
+        // it might have carried out of the document and back.
         rec.el.classList.remove('fb-pane-detached');
         rec.stub.remove();
     }
@@ -108,7 +148,7 @@
 
         // Reconcile immediately: register() reopens a pane the user left open at
         // last unload, and that can land before (or after) attachChip runs.
-        if (panes.isOpen(paneId)) _onOpened(rec);
+        if (panes.isOpen(paneId)) _onOpened(rec, null);
 
         return () => {
             if (attached.get(paneId) !== rec) return;
@@ -123,7 +163,7 @@
     if (bus && typeof bus.on === 'function') {
         bus.on('panes:opened', (e) => {
             const rec = attached.get(e.detail && e.detail.id);
-            if (rec) _onOpened(rec);
+            if (rec) _onOpened(rec, e.detail);
         });
         bus.on('panes:closed', (e) => {
             const rec = attached.get(e.detail && e.detail.id);

static/panes/pane-manager.js

@@ -129,6 +129,12 @@
         // a docked panel reappears at the bottom of its container, or not at all.
         const home = { parent: el.parentNode, next: el.nextSibling };
 
+        // An element on its way OUT of this document must not carry a class whose
+        // whole job is to hide it IN this document. `.fb-pane-detached` is
+        // `display:none !important`, and it travels with the node — straight into
+        // the pane window, which then renders nothing at all.
+        el.classList.remove('fb-pane-detached');
+
         try {
             host.place(spec, el);
         } catch (e) {
@@ -139,7 +145,10 @@
         open.set(id, { spec, hostId: host.id, el, home });
         if (opts.remember !== false) _rememberHost(id, host.id);
         if (spec.onHost) { try { spec.onHost(host.id, el); } catch (e) { console.error('[panes]', id, 'onHost threw', e); } }
-        _emit('panes:opened', { id: id, host: host.id });
+        // `home` rides along because the element has LEFT this document — anything
+        // that wants to mark the hole it left (the chip's stub) needs to know where
+        // the hole is, and can no longer ask the element itself.
+        _emit('panes:opened', { id: id, host: host.id, el: el, home: home });
         return true;
     }
 
@@ -149,17 +158,21 @@
         if (!entry) return false;
         open.delete(id);
 
-        const host = hosts.get(entry.hostId);
+        // ORDER IS LOAD-BEARING: bring the element home BEFORE the host lets go of
-        try { if (host) host.unplace(id, entry.el); } catch (e) { console.error('[panes] host', entry.hostId, 'threw releasing', id, e); }
+        // it. The host's unplace() closes the pane window, and closing a window
-
+        // tears down its document — with the element still inside it. The node
-        // Put the element back where it came from. Re-adopting it into THIS
+        // survives (we hold a reference) but comes back stripped of its event
-        // document is what undoes the pop-out: a node adopted by another window
+        // listeners, so the panel returns looking perfect and completely dead: no
-        // has that window's document as its owner, and appending it here without
+        // buttons, no sliders, nothing.
-        // adopting first would throw in some engines and leave it in a half-moved
+        //
-        // state in others.
+        // Adopt first, while the pane window is still alive, and the node moves out
+        // of a living document into a living document, which is the only case the
+        // DOM actually guarantees.
         const home = entry.home;
         if (home && home.parent && home.parent.isConnected) {
             try {
+                // adoptNode, not appendChild: the node's owner is currently the pane
+                // window's document, and adopting is what transfers ownership back.
                 const node = document.adoptNode(entry.el);
                 if (home.next && home.next.parentNode === home.parent) home.parent.insertBefore(node, home.next);
                 else home.parent.appendChild(node);
@@ -168,9 +181,13 @@
             }
         }
 
+        const host = hosts.get(entry.hostId);
+        try { if (host) host.unplace(id, entry.el); } catch (e) { console.error('[panes] host', entry.hostId, 'threw releasing', id, e); }
+
         if (opts.remember !== false) _rememberHost(id, null);
         if (entry.spec.onHost) { try { entry.spec.onHost(null, entry.el); } catch (e) { /* non-fatal */ } }
         _emit('panes:closed', { id: id, host: entry.hostId });
+
         return true;
     }
 
@@ -255,6 +272,9 @@
         focus: focusPane,
         isOpen: (id) => open.has(id),
         hostOf: (id) => { const e = open.get(id); return e ? e.hostId : null; },
+        // Where an open pane's element came from. The chip needs this to mark the
+        // hole the element left, since it can no longer ask the element itself.
+        homeOf: (id) => { const e = open.get(id); return e ? e.home : null; },
         get: (id) => specs.get(id) || null,
         list: () => Array.from(specs.values()).map((s) => ({
             id: s.id, title: s.title, icon: s.icon,

static/panes/pane-window-host.js

@@ -80,13 +80,53 @@
     // only exists in the document we actually want: pane.html's #fb-pane-root.
     function _whenReady(w, onReady, onFail) {
         const deadline = performance.now() + 10000;
+        let reachFailure = null;   // why we could never see the pop-out's document
         const tick = () => {
             if (w.closed) return;
-            let root = null;
+
-            try { root = w.document && w.document.getElementById('fb-pane-root'); }
+            let doc = null;
-            catch (e) { root = null; }   // mid-navigation: the document is being swapped
+            try { doc = w.document; }
-            if (root) { onReady(root); return; }
+            catch (e) {
-            if (performance.now() > deadline) { onFail(new Error('the pane window never loaded')); return; }
+                // A SecurityError here is the one that matters: it means the pop-out
+                // is not reachable from this realm at all (a separate process /
+                // browsing-context group), and no amount of waiting will fix it —
+                // adoptNode can never work.
+                doc = null;
+                reachFailure = e;
+            }
+
+            if (doc && doc.readyState !== 'loading') {
+                // Only ever adopt into the document we actually navigated TO.
+                // about:blank reports readyState 'complete' from the moment
+                // window.open() returns, and adopting into it means the panel is
+                // destroyed when /pane replaces it a moment later.
+                const href = (doc.location && doc.location.href) || '';
+                const isPaneDoc = href.indexOf('/pane') >= 0;
+                if (isPaneDoc) {
+                    // Prefer pane.html's own root, but never fail for want of it —
+                    // a stale cached copy of the page (or a future rename) must not
+                    // leave the user with a blank window and no panel.
+                    const root = doc.getElementById('fb-pane-root') || doc.body;
+                    if (root) { onReady(root); return; }
+                }
+            }
+
+            if (performance.now() > deadline) {
+                let why;
+                if (reachFailure) {
+                    why = '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';
+                } else if (!doc) {
+                    why = 'the pane window exposed no document at all';
+                } else {
+                    why = 'the pane window never loaded /pane (it is showing '
+                        + ((doc.location && doc.location.href) || 'an unknown URL')
+                        + ', readyState ' + doc.readyState + ')';
+                }
+                onFail(new Error(why));
+                return;
+            }
             setTimeout(tick, 25);
         };
         tick();
@@ -107,6 +147,67 @@
 
         root.appendChild(doc.adoptNode(el));
         doc.title = spec.title + ' — fee[dB]ack';
+
+        // THE ELEMENT MUST LEAVE BEFORE THE DOCUMENT DIES.
+        //
+        // When the user closes a pane window, its document is torn down — and the
+        // panel is inside it. The node itself survives (we hold a reference) and
+        // comes home looking perfect: right markup, right classes, right size. But
+        // it comes home DEAD: every event listener in the subtree is gone with the
+        // document that hosted them. A panel that renders and does nothing.
+        //
+        // The `closed` poll cannot save us: by the time `w.closed` is true, the
+        // document is already gone. `beforeunload` fires while it is still alive, so
+        // this is the last moment we can get the element out — and panes.close()
+        // adopts it back into the main document synchronously.
+        //
+        // We attach it HERE, not when the window was opened: back then the window
+        // still held its throwaway about:blank document, and a listener registered
+        // on that is discarded when /pane replaces it.
+        w.addEventListener('beforeunload', () => {
+            if (panes.isOpen(spec.id)) panes.close(spec.id);
+        });
+
+        // THE ELEMENT MUST LEAVE BEFORE THE DOCUMENT DIES.
+        //
+        // When the user closes a pane window, its document is torn down — and the
+        // panel is inside it. The node itself survives (we hold a reference) and
+        // comes home looking perfect: right markup, right classes, right size. But
+        // it comes home DEAD: every event listener in the subtree is gone with the
+        // document that hosted them. A panel that renders and does nothing.
+        //
+        // The `closed` poll cannot save us: by the time `w.closed` is true, the
+        // document is already gone. `beforeunload` fires while it is still alive, so
+        // this is the last moment we can get the element out — and panes.close()
+        // adopts it back into the main document synchronously.
+        //
+        // We attach it HERE, not when the window was opened: back then the window
+        // still held its throwaway about:blank document, and a listener registered
+        // on that is discarded when /pane replaces it.
+        w.addEventListener('beforeunload', () => {
+            if (panes.isOpen(spec.id)) panes.close(spec.id);
+        });
+
+        // Measure LATE. The pane window has not laid out yet at this point (it is
+        // still being created and shown), so anything read now reports 0x0 whether
+        // or not there is a real problem.
+        setTimeout(() => {
+            if (w.closed || !el.isConnected) return;
+            const view = doc.defaultView;
+            const cs = view.getComputedStyle(el);
+            const rootCs = view.getComputedStyle(root);
+            console.info('[panes] adopted', spec.id,
+                '| el:', el.id || el.className,
+                '| size:', el.offsetWidth + 'x' + el.offsetHeight,
+                '| display:', cs.display, '| visibility:', cs.visibility, '| opacity:', cs.opacity,
+                '| position:', cs.position, '| w/h:', cs.width + '/' + cs.height,
+                '| children:', el.childElementCount,
+                '| hidden attr:', el.hasAttribute('hidden'),
+                '| inline style:', el.getAttribute('style') || '(none)',
+                '| root size:', root.offsetWidth + 'x' + root.offsetHeight, '/', rootCs.display,
+                '| window inner:', view.innerWidth + 'x' + view.innerHeight,
+                '| styles:', doc.querySelectorAll('link[rel="stylesheet"], style').length);
+        }, 400);
     }
 
     function place(spec, el) {