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