From a23af8519861f674d700cbe4fe183817f47458dc Mon Sep 17 00:00:00 2001 From: Ricardo Domingues Date: Wed, 19 Aug 2026 22:19:51 +0100 Subject: [PATCH] PLIA-4026: Read the Prepublish DOM from the WordPress origin getDom() built its hidden iframe from the url it was handed, which has the "Public URL" setting applied to it. On a site whose Public URL points at a different host than WordPress itself (a separate delivery domain, say), the frame was therefore cross-origin and reading contentWindow.document threw SecurityError. The throw happened inside the load listener and the promise had no reject path, so the await never settled: Prepublish appeared to do nothing, with no useful console output. The frame was also only torn down on success, so the failure left a height:100vh iframe covering the page, which is what made it look like a new page had loaded. Derive the fetch URL from window.location instead, via the URL API so the nonce survives a fragment, and build the iframe as an element rather than by interpolating into innerHTML. The Public URL remains what we report to Siteimprove, so results still land on the crawled URL. Also, so that any future failure here is visible rather than silent: wrap the document read in try/catch, add a 30s timeout, remove the frame on both paths, and have the click handler clear the spinner and log. Separately, the overlay-driven path called getDom(url) with no nonce at all, sending si_preview_nonce=undefined. Nonce verification then failed and the plugin's own scripts were enqueued into the very DOM being scraped, for every overlay-latest user rather than only split-domain ones. Capture and pass the nonce. Removing getDom's url argument left common(url) unused, so drop that parameter and its call sites. Bump the plugin to 2.1.4 with changelog entries. --- README.md | 5 + siteimprove/admin/js/siteimprove.js | 118 +++++++++++++++++---- siteimprove/includes/class-siteimprove.php | 2 +- siteimprove/readme.txt | 7 +- siteimprove/siteimprove.php | 2 +- 5 files changed, 109 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 8314927..d30abbf 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,11 @@ The code on this repository has to match the WordPress Coding Standards in order Every pull request will be checked against WPCS through GitHub Actions. ## Version History +### 2.1.4 +* Bugfix - Prepublish now reads the page from the WordPress domain rather than the configured Public URL, so the check works on setups where Public URL points at a separate delivery domain +* Bugfix - Prepublish now reports an error and clears the loading overlay when it cannot read the page, instead of waiting indefinitely +* Bugfix - Send the preview nonce when Prepublish is triggered from the overlay, so the checked page no longer contains the plugin's own scripts + ### 2.1.3 * Bugfix - Load the front-end overlay and Prepublish toolbar action for any user who can edit content (custom roles, multisite super admins), not just a fixed list of role names diff --git a/siteimprove/admin/js/siteimprove.js b/siteimprove/admin/js/siteimprove.js index 5914bff..d0dc29b 100644 --- a/siteimprove/admin/js/siteimprove.js +++ b/siteimprove/admin/js/siteimprove.js @@ -5,35 +5,96 @@ (function ($) { "use strict"; - const getDom = async function (url, nonce) { + // Upper bound on waiting for the hidden frame used by the Prepublish check. + const DOM_FETCH_TIMEOUT_MS = 30000; + + /** + * Builds the URL the Prepublish check loads in order to read the DOM. + * + * This deliberately derives from window.location and NOT from the url passed + * around elsewhere in this file, because that one has the "Public URL" setting + * applied to it. When Public URL points at a different host than the one + * serving WordPress (a separate delivery domain, for instance), the frame + * below becomes cross-origin and the browser forbids us from reading its + * document. window.location is always same-origin and, on a preview page, + * already carries the preview arguments needed to render the draft. + * + * The Public URL is still what we report to Siteimprove; only the fetch is + * local. See the contentcheck_flatdom call in the click handler below. + */ + const buildDomFetchUrl = function (nonce) { + const fetchUrl = new URL(window.location.href); + fetchUrl.searchParams.set("si_preview_nonce", nonce); + // A fragment would swallow the query string we just set. + fetchUrl.hash = ""; + return fetchUrl.href; + }; + + const getDom = async function (nonce) { const iframeContainer = document.createElement("div"); iframeContainer.setAttribute("id", "div_iframe"); document.body.appendChild(iframeContainer); - const separator = url.includes("?") ? "&" : "?"; - iframeContainer.innerHTML = ``; - const iframe = document.getElementById("domIframe"); + const iframe = document.createElement("iframe"); + iframe.setAttribute("id", "domIframe"); + iframe.setAttribute("style", "height:100vh; width:100%"); + iframe.src = buildDomFetchUrl(nonce); + iframeContainer.appendChild(iframe); const promise = new Promise(function (resolve, reject) { + // Without this the frame is only torn down on success, so a failure + // leaves a full-viewport iframe sitting on top of the page. + let removed = false; + const removeFrame = function () { + if (!removed) { + removed = true; + document.body.removeChild(iframeContainer); + } + }; + + // A frame that never fires load would otherwise leave this promise + // pending forever, and the caller waiting on it with no way to recover. + const timeoutId = setTimeout(function () { + removeFrame(); + reject( + new Error( + "Siteimprove: timed out after " + + DOM_FETCH_TIMEOUT_MS + + "ms loading " + + iframe.src + + " for the Prepublish check." + ) + ); + }, DOM_FETCH_TIMEOUT_MS); + iframe.addEventListener( "load", () => { - // In order to preserve the DOM node hierarchy for highlights, we have chosen to empty the #wp-admin-bar from the new DOM instead of outright removing it. - var adminBar = iframe.contentWindow.document.getElementById('wpadminbar'); - if (adminBar) { - adminBar.innerHTML = '
'; - adminBar.id = 'wpadminbar-disabled'; + clearTimeout(timeoutId); + try { + // In order to preserve the DOM node hierarchy for highlights, we have chosen to empty the #wp-admin-bar from the new DOM instead of outright removing it. + var adminBar = iframe.contentWindow.document.getElementById('wpadminbar'); + if (adminBar) { + adminBar.innerHTML = '
'; + adminBar.id = 'wpadminbar-disabled'; + } + const cleanDom = iframe.contentWindow.document; + removeFrame(); + resolve(cleanDom); + } catch (err) { + // Reading the frame's document throws if it turned out not to be + // same-origin, or if it was refused by X-Frame-Options / CSP. + // Surface it rather than hanging. + removeFrame(); + reject(err); } - const cleanDom = iframe.contentWindow.document; - document.body.removeChild(iframeContainer); - resolve(cleanDom); }, { once: true } ); }); - + const documentReturned = await promise; $(".si-overlay").remove(); return documentReturned; - }; + }; window.siteimprove = { input: function (url, token, version, is_content_page, nonce) { @@ -43,13 +104,13 @@ this.version = version; this.is_content_page = is_content_page; this.nonce = nonce; - this.common(url); + this.common(); }, domain: function (url, token) { this.url = url; this.token = token; this.method = "domain"; - this.common(url); + this.common(); }, clear: function (callback, token) { this.callback = callback; @@ -62,13 +123,13 @@ this.url = url; this.token = token; this.method = "recheck"; - this.common(url); + this.common(); }, recrawl: function (url, token) { this.url = url; this.token = token; this.method = "recrawl"; - this.common(url); + this.common(); }, contentcheck_flatdom: function (domReference, url, token, callback) { this.url = url; @@ -76,9 +137,9 @@ this.domReference = domReference; this.method = "contentcheck-flat-dom"; this.callback = callback; - this.common(url); + this.common(); }, - common: function (url) { + common: function () { const _si = window._si || []; if (this.method == "contentcheck-flat-dom") { _si.push([ @@ -106,8 +167,11 @@ }]); + // Captured here because getDomCallback is invoked later by the overlay, + // with no guarantee about what `this` will be bound to. + const nonce = this.nonce; const getDomCallback = function () { - return getDom(url); + return getDom(nonce); }; @@ -240,7 +304,17 @@ var si_prepublish_data = siGetCurrentUrlAndToken(); evt.preventDefault(); $("body").append('
'); - var dom = await getDom(si_prepublish_data.url, si_prepublish_data.nonce); + // The DOM is read from the current origin; the url reported to + // Siteimprove stays the public one, so results land on the crawled URL. + var dom; + try { + dom = await getDom(si_prepublish_data.nonce); + } catch (err) { + // Leaving the spinner up makes this look like it is still working. + $(".si-overlay").remove(); + console.error("Siteimprove: could not read the page for the Prepublish check.", err); + return; + } siteimprove.contentcheck_flatdom( dom, si_prepublish_data.url, diff --git a/siteimprove/includes/class-siteimprove.php b/siteimprove/includes/class-siteimprove.php index 8082c38..5abf17c 100755 --- a/siteimprove/includes/class-siteimprove.php +++ b/siteimprove/includes/class-siteimprove.php @@ -60,7 +60,7 @@ class Siteimprove { public function __construct() { $this->plugin_name = 'siteimprove'; - $this->version = '2.1.3'; + $this->version = '2.1.4'; $this->load_dependencies(); $this->set_locale(); diff --git a/siteimprove/readme.txt b/siteimprove/readme.txt index a69be96..f70afb6 100644 --- a/siteimprove/readme.txt +++ b/siteimprove/readme.txt @@ -3,7 +3,7 @@ Contributors: siteimprove Tags: accessibility, analytics, insights, spelling, seo Requires at least: 4.7.2 Tested up to: 6.8.1 -Stable tag: 2.1.3 +Stable tag: 2.1.4 License: GPLv2 or later License URI: https://www.gnu.org/licenses/gpl-2.0.html @@ -87,6 +87,11 @@ Please review whether you have JavaScript turned off in your browser. We use Jav == Changelog == += 2.1.4 = +* Bugfix - Prepublish now reads the page from the WordPress domain rather than the configured Public URL, so the check works on setups where Public URL points at a separate delivery domain +* Bugfix - Prepublish now reports an error and clears the loading overlay when it cannot read the page, instead of waiting indefinitely +* Bugfix - Send the preview nonce when Prepublish is triggered from the overlay, so the checked page no longer contains the plugin's own scripts + = 2.1.3 = * Bugfix - Load the front-end overlay and Prepublish toolbar action for any user who can edit content (custom roles, multisite super admins), not just a fixed list of role names diff --git a/siteimprove/siteimprove.php b/siteimprove/siteimprove.php index 81b8f2a..98469c4 100755 --- a/siteimprove/siteimprove.php +++ b/siteimprove/siteimprove.php @@ -9,7 +9,7 @@ * Plugin Name: Siteimprove * Plugin URI: https://www.siteimprove.com/integrations/cms-plugin/wordpress/ * Description: Integration with Siteimprove. - * Version: 2.1.3 + * Version: 2.1.4 * Author: Siteimprove * Author URI: http://www.siteimprove.com/ * Requires at least: 4.7.2