From c5300ef235c1ea91a37fa3fc85e95c8d8cedbc15 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 4 Nov 2020 21:52:06 -0800 Subject: [PATCH] Remove obsolete ampPageHasErrors and improve isAmpDocument passing --- assets/src/admin/paired-browsing/app.js | 49 +++------------------- assets/src/admin/paired-browsing/client.js | 36 ++-------------- includes/class-amp-theme-support.php | 5 ++- includes/templates/amp-paired-browsing.php | 4 -- 4 files changed, 11 insertions(+), 83 deletions(-) diff --git a/assets/src/admin/paired-browsing/app.js b/assets/src/admin/paired-browsing/app.js index f52dac52365..a3832aafac7 100644 --- a/assets/src/admin/paired-browsing/app.js +++ b/assets/src/admin/paired-browsing/app.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { addQueryArgs, hasQueryArg, removeQueryArgs } from '@wordpress/url'; +import { addQueryArgs, removeQueryArgs } from '@wordpress/url'; /** * Internal dependencies */ @@ -79,7 +79,6 @@ class PairedBrowsingApp { constructor() { this.nonAmpIframe = document.querySelector( '#non-amp iframe' ); this.ampIframe = document.querySelector( '#amp iframe' ); - this.ampPageHasErrors = false; // @todo This is obsolete. Remove this and invalid-amp span. this.currentNonAmpUrl = this.nonAmpIframe.src; this.currentAmpUrl = this.ampIframe.src; @@ -211,16 +210,9 @@ class PairedBrowsingApp { const isClientConnected = this.isClientConnected( iframe ); if ( ! isClientConnected ) { - if ( this.ampIframe === iframe && this.ampPageHasErrors ) { - this.disconnectText.general.classList.toggle( 'hidden', true ); - this.disconnectText.invalidAmp.classList.toggle( 'hidden', false ); - } else { - this.disconnectText.general.classList.toggle( 'hidden', false ); - this.disconnectText.invalidAmp.classList.toggle( 'hidden', true ); - } - // Show the 'Go Back' button if the parent window has history. this.disconnectButtons.goBack.classList.toggle( 'hidden', 0 >= window.history.length ); + // If the document is not available, the window URL cannot be accessed. this.disconnectButtons.exit.classList.toggle( 'hidden', null === iframe.contentDocument ); @@ -246,7 +238,7 @@ class PairedBrowsingApp { * @param {HTMLIFrameElement} iframe The iframe. */ isClientConnected( iframe ) { - if ( this.ampIframe === iframe && this.ampPageHasErrors ) { + if ( this.ampIframe === iframe ) { return false; } @@ -292,16 +284,6 @@ class PairedBrowsingApp { return parsedUrl.href; } - /** - * Checks if a URL has the 'amp_validation_errors' query variable. - * - * @param {string} url URL string. - * @return {boolean} True if such query var exists, false if not. - */ - urlHasValidationErrorQueryVar( url ) { - return hasQueryArg( url, 'amp_validation_errors' ); - } - /** * Replace location. * @@ -365,28 +347,9 @@ class PairedBrowsingApp { return; } + // Force the AMP iframe to always have an AMP URL. if ( ! isAmpDocument ) { - if ( this.urlHasValidationErrorQueryVar( ampUrl ) ) { - /* - * If the AMP page has validation errors, mark the page as invalid so that the - * 'disconnected' overlay can be shown. - */ - this.ampPageHasErrors = true; - this.toggleDisconnectOverlay( this.ampIframe ); - return; - } else if ( ampUrl ) { - // Force the AMP iframe to always have an AMP URL, if an AMP version is available. - this.replaceLocation( sourceIframe, ampUrl ); - return; - } - - /* - * If the AMP iframe has loaded a non-AMP page and none of the conditions above are - * true, then explicitly mark it as having errors and display the 'disconnected - * overlay. - */ - this.ampPageHasErrors = true; - // this.toggleDisconnectOverlay( this.ampIframe ); + this.replaceLocation( sourceIframe, ampUrl ); return; } @@ -394,8 +357,6 @@ class PairedBrowsingApp { // Update the AMP link above the iframe used for exiting paired browsing. this.ampLink.href = removeQueryArgs( ampUrl, noampQueryVar ); - - this.ampPageHasErrors = false; } else { // Stop if the URL has not changed. if ( this.currentNonAmpUrl === nonAmpUrl ) { diff --git a/assets/src/admin/paired-browsing/client.js b/assets/src/admin/paired-browsing/client.js index 646a155c33f..c1afebdcf1c 100644 --- a/assets/src/admin/paired-browsing/client.js +++ b/assets/src/admin/paired-browsing/client.js @@ -9,36 +9,7 @@ import domReady from '@wordpress/dom-ready'; import { isNonAmpWindow, isAmpWindow } from './utils'; const { parent, ampPairedBrowsingClientData } = window; -const { ampUrl, nonAmpUrl } = ampPairedBrowsingClientData; - -/** - * Validates whether or not the window document is AMP compatible. - * - * @return {boolean} True if AMP compatible, false if not. - */ -function documentIsAmp() { - return Boolean( document.querySelector( 'head > script[src="https://cdn.ampproject.org/v0.js"]' ) ); -} - -// /** -// * Get amphtml link URL. -// * -// * @return {string|null} URL or null if link not present. -// */ -// function getAmphtmlLinkHref() { -// const link = document.querySelector( 'head > link[rel=amphtml]' ); -// return link ? link.href : null; -// } -// -// /** -// * Get canonical link URL. -// * -// * @return {string|null} URL or null if link not present. -// */ -// function getCanonicalLinkHref() { -// const link = document.querySelector( 'head > link[rel=canonical]' ); -// return link ? link.href : null; -// } +const { ampUrl, nonAmpUrl, isAmpDocument } = ampPairedBrowsingClientData; /** * Modify document for paired browsing. @@ -47,7 +18,7 @@ function modifyDocumentForPairedBrowsing() { // Scrolling is not synchronized if `scroll-behavior` is set to `smooth`. document.documentElement.style.setProperty( 'scroll-behavior', 'auto', 'important' ); - if ( documentIsAmp() ) { + if ( isAmpDocument ) { // Hide the paired browsing menu item. const pairedBrowsingMenuItem = document.getElementById( 'wp-admin-bar-amp-paired-browsing' ); if ( pairedBrowsingMenuItem ) { @@ -148,8 +119,7 @@ function sendHeartbeat() { parent, 'heartbeat', { - isAmpDocument: documentIsAmp(), - //locationHref: window.location.href, + isAmpDocument, ampUrl, nonAmpUrl, documentTitle: document.title, diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index bf251a2e0ea..b3c99033089 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -2370,8 +2370,9 @@ public static function setup_paired_browsing_client() { 'amp-paired-browsing-client', 'ampPairedBrowsingClientData', [ - 'ampUrl' => $amp_url, - 'nonAmpUrl' => $non_amp_url, + 'isAmpDocument' => $is_amp_request, + 'ampUrl' => $amp_url, + 'nonAmpUrl' => $non_amp_url, ] ); diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index f3807ec6e28..63e9aa96be4 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -43,10 +43,6 @@ - - - -