From 47806feb854f3d9a60ea02a0e80dda5d2b3293d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophile=20Desmedt?= <34781168+DesignThinkerer@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:28:15 +0200 Subject: [PATCH] Fix plugin library iframes hanging if onload never fires by adding readiness handshake (#10003) * Fix race condition in plugin library iframe message handling Introduce a PLUGIN-LIBRARY-READY handshake verb for plugin library iframes. This ensures that postMessage commands are not prematurely dropped before the iframe's internal script has fully bound its message listeners. Retains iframe.onload as a backward-compatible fallback for legacy or external libraries that do not emit the ready signal. * add changenote * feat(plugin-library): add PLUGIN-LIBRARY-READY handshake to plugin library iframe Add PLUGIN-LIBRARY-READY message emission in libraryserver.js Signals to parent window when iframe is ready to receive messages Complements browser-messaging.js PLUGIN-LIBRARY-READY handler Includes comprehensive tests for callback queueing and handshake flow Fixes race condition in plugin library iframe message handling * fix(plugin-library): emit PLUGIN-LIBRARY-READY message on iframe initialization Emit the PLUGIN-LIBRARY-READY message after the message listener is bound, signaling to the parent window that the iframe is ready to receive postMessage commands. This completes the handshake implementation to eliminate the race condition where messages could be lost before the iframe's script initialization completes. * fix indentation * remove extra space * remove typo * refactor(test-browser-messaging): update test descriptions and remove unused code * lint fix * fix: correct the update note --- core/modules/startup/browser-messaging.js | 36 ++++- .../tiddlers/tests/test-browser-messaging.js | 140 ++++++++++++++++++ .../#10003-plugin-library-ready-handshake.tid | 22 +++ .../tiddlywiki/pluginlibrary/libraryserver.js | 5 + 4 files changed, 195 insertions(+), 8 deletions(-) create mode 100644 editions/test/tiddlers/tests/test-browser-messaging.js create mode 100644 editions/tw5.com/tiddlers/releasenotes/5.5.0/#10003-plugin-library-ready-handshake.tid diff --git a/core/modules/startup/browser-messaging.js b/core/modules/startup/browser-messaging.js index 1ed2760929..4abafe8198 100644 --- a/core/modules/startup/browser-messaging.js +++ b/core/modules/startup/browser-messaging.js @@ -4,7 +4,6 @@ type: application/javascript module-type: startup Browser message handling - \*/ "use strict"; @@ -18,6 +17,20 @@ exports.synchronous = true; /* Load a specified url as an iframe and call the callback when it is loaded. If the url is already loaded then the existing iframe instance is used */ + +// Ensure the callback fires once, whether by PLUGIN-LIBRARY-READY or onload (legacy) + +function flushCallbacks(iframeInfo, err) { + if(iframeInfo.status !== "loaded") { + iframeInfo.status = err ? "error" : "loaded"; + saveIFrameInfoTiddler(iframeInfo); + var cb; + while((cb = iframeInfo.callbacks.shift())) { + cb(err, iframeInfo); + } + } +} + function loadIFrame(url,callback) { // Check if iframe already exists var iframeInfo = $tw.browserMessaging.iframeInfoMap[url]; @@ -30,7 +43,8 @@ function loadIFrame(url,callback) { iframeInfo = { url: url, status: "loading", - domNode: iframe + domNode: iframe, + callbacks: [callback] }; $tw.browserMessaging.iframeInfoMap[url] = iframeInfo; saveIFrameInfoTiddler(iframeInfo); @@ -38,19 +52,18 @@ function loadIFrame(url,callback) { iframe.style.display = "none"; iframe.setAttribute("library","true"); document.body.appendChild(iframe); - // Set up onload + + // Set up onload. Legacy fallback: if PLUGIN-LIBRARY-READY never arrives, onload triggers the flush later iframe.onload = function() { - iframeInfo.status = "loaded"; - saveIFrameInfoTiddler(iframeInfo); - callback(null,iframeInfo); + flushCallbacks(iframeInfo); }; iframe.onerror = function() { - callback("Cannot load iframe"); + flushCallbacks(iframeInfo, "Cannot load iframe"); }; try { iframe.src = url; } catch(ex) { - callback(ex); + flushCallbacks(iframeInfo, ex); } } } @@ -147,6 +160,13 @@ exports.startup = function() { // console.log("browser-messaging: Received message from",event.origin); // console.log("browser-messaging: Message content",event.data); switch(event.data.verb) { + case "PLUGIN-LIBRARY-READY": + $tw.utils.each($tw.browserMessaging.iframeInfoMap, function(info) { + if(info && info.domNode && info.domNode.contentWindow === event.source) { + flushCallbacks(info); + } + }); + break; case "GET-RESPONSE": if(event.data.status.charAt(0) === "2") { if(event.data.cookies) { diff --git a/editions/test/tiddlers/tests/test-browser-messaging.js b/editions/test/tiddlers/tests/test-browser-messaging.js new file mode 100644 index 0000000000..895b0bb29a --- /dev/null +++ b/editions/test/tiddlers/tests/test-browser-messaging.js @@ -0,0 +1,140 @@ +/*\ +title: test-browser-messaging.js +type: application/javascript +tags: [[$:/tags/test-spec]] + +Tests the PLUGIN-LIBRARY-READY handshake and legacy onload fallback. + +\*/ + +"use strict"; + +describe("Browser Messaging", function() { + + it("should declare itself as a synchronous browser-only startup module", function() { + var startup = require("$:/core/modules/browser-messaging.js"); + expect(startup.name).toBe("browser-messaging"); + expect(startup.platforms).toEqual(["browser"]); + expect(startup.synchronous).toBe(true); + expect(typeof startup.startup).toBe("function"); + }); + + // loadIFrame()/flushCallbacks() are private to browser-messaging.js, so the only way + // to exercise them for real is through $tw.rootWidget events and real postMessage + // traffic with a real iframe, rather than re-implementing their logic here. + describe("plugin library iframe handshake", function() { + + var urlsToUnload; + + beforeEach(function() { + if(!$tw.browser) { return; } + urlsToUnload = []; + }); + + afterEach(function() { + if(!$tw.browser) { return; } + urlsToUnload.forEach(function(url) { + // Grab the DOM node before unloading clears the map entry, as a + // belt-and-braces removal (unloadIFrame's own removal has an + // unrelated off-by-one loop bug for small iframe counts). + var info = $tw.browserMessaging.iframeInfoMap[url]; + $tw.rootWidget.dispatchEvent({type: "tm-unload-plugin-library", paramObject: {url: url}}); + if(info && info.domNode && info.domNode.parentNode) { + info.domNode.parentNode.removeChild(info.domNode); + } + URL.revokeObjectURL(url); + }); + }); + + function makeChildUrl(script) { + var blob = new Blob(["