Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite Looping when the AMP plugin is activated #5767

Closed
titoabreurj opened this issue Jan 4, 2021 · 14 comments · Fixed by #5775
Closed

Infinite Looping when the AMP plugin is activated #5767

titoabreurj opened this issue Jan 4, 2021 · 14 comments · Fixed by #5775
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. WS:Core Work stream for Plugin core
Milestone

Comments

@titoabreurj
Copy link

What's the issue?

AMP on Plugin Actived, access the site are the looping.
The homepage and others pages.
I checked with theme developement and the problem persists.

How do we reproduce the issue?

Access the site thoriugh the URL https://fullworks.net.br/?amp on a mobile device and you will see the loop happen!

What browsers are affected?

All browsers

Which AMP version is affected?

Version 2.0.9 and lowers

I'm talking about Brazil. It is now exactly 8:00 pm
Thanks

@kristoferbaxter kristoferbaxter transferred this issue from ampproject/amphtml Jan 6, 2021
@westonruter
Copy link
Member

I assume you have Mobile Redirection turned on?

On the non-AMP version of the page, in the Admin Bar click on the "Validate URL" submenu item under the AMP menu. That will take you to a screen that shows any validation issues with the page. Can you share a screenshot of that page?

@westonruter
Copy link
Member

It appears you have deactivated the AMP plugin so I can't reproduce the issue right now.

@westonruter westonruter added the Support Help with setup, implementation, or "How do I?" questions. label Jan 7, 2021
@titoabreurj
Copy link
Author

titoabreurj commented Jan 7, 2021 via email

@westonruter
Copy link
Member

OK, I'm seeing it now on a Year archive template: https://fullworks.net.br/2020/?amp=1

However, I am also getting an AMP page for a Singular template without the infinite redirect: https://fullworks.net.br/totem-para-alcool-gel-solucao-de-protecao-covid-19/?amp=1

But I'm also getting an AMP page without ?amp=1: https://fullworks.net.br/totem-para-alcool-gel-solucao-de-protecao-covid-19/

So it seems that you have a page caching plugin that is ignoring the ?amp=1 query parameter, this is resulting in non-AMP pages being cached for ?amp=1, and when that happens, an infinite redirect could occur.

So we can fix the symptom of the issue by checking that location.href does not already equal ampUrl in this logic:

const url = new URL( location.href );
if ( url.searchParams.has( noampQueryVarName ) && noampQueryVarValue === url.searchParams.get( noampQueryVarName ) ) {
// If the noamp query param is present, remember that redirection should be disabled.
sessionStorage.setItem( disabledStorageKey, '1' );
} else {
// Otherwise, since JS is running then we know it's not an AMP page and we need to redirect to the AMP version.
window.stop(); // Stop loading the page! This should cancel all loading resources.
// Replace the current page with the AMP version.
location.replace( ampUrl );
}

But you'll still need to fix the underlying page caching problem to ensure that AMP pages and non-AMP pages are cached separately.

@westonruter westonruter added the Bug Something isn't working label Jan 7, 2021
@westonruter westonruter added this to the v2.0.10 milestone Jan 7, 2021
@titoabreurj
Copy link
Author

Hi Weston 
Thank you very much for identifying a possible source of the problem.

However, I saw that my "mobile.redirection.js" file is different from what you sent in the link.
Could I replace the current code with the one you sent me?

!function(e){var n={};function r(t){if(n[t])return n[t].exports;var o=n[t]={i:t,l:!1,exports:{}};return e[t].call(o.exports,o,o.exports,r),o.l=!0,o.exports}r.m=e,r.c=n,r.d=function(e,n,t){r.o(e,n)||Object.defineProperty(e,n,{enumerable:!0,get:t})},r.r=function(e){"undefined"!=typeof Symbol&&Symbol.toStringTag&&Object.defineProperty(e,Symbol.toStringTag,{value:"Module"}),Object.defineProperty(e,"__esModule",{value:!0})},r.t=function(e,n){if(1&n&&(e=r(e)),8&n)return e;if(4&n&&"object"==typeof e&&e&&e.__esModule)return e;var t=Object.create(null);if(r.r(t),Object.defineProperty(t,"default",{enumerable:!0,value:e}),2&n&&"string"!=typeof e)for(var o in e)r.d(t,o,function(n){return e[n]}.bind(null,o));return t},r.n=function(e){var n=e&&e.__esModule?function(){return e.default}:function(){return e};return r.d(n,"a",n),n},r.o=function(e,n){return Object.prototype.hasOwnProperty.call(e,n)},r.p="",r(r.s=0)}([function(e,n){!function(e){var n=e.ampUrl,r=e.isCustomizePreview,t=e.isAmpDevMode,o=e.noampQueryVarName,i=e.noampQueryVarValue,a=e.disabledStorageKey,u=e.mobileUserAgents,s=e.regexRegex;if("undefined"!=typeof sessionStorage){var c=new RegExp(s);if(u.some((function(e){var n=e.match(c);if(n&&new RegExp(n[1],n[2]).test(navigator.userAgent))return!0;return navigator.userAgent.includes(e)}))){document.addEventListener("DOMContentLoaded",(function(){var e=document.getElementById("amp-mobile-version-switcher");if(e){e.hidden=!1;var n=e.querySelector("a[href]");n&&n.addEventListener("click",(function(){sessionStorage.removeItem(a)}))}}));var f=t&&["paired-browsing-non-amp","paired-browsing-amp"].includes(window.name);if(!(sessionStorage.getItem(a)||r||f)){var d=new URL(location.href);d.searchParams.has(o)&&i===d.searchParams.get(o)?sessionStorage.setItem(a,"1"):(window.stop(),location.replace(n))}}}}(AMP_MOBILE_REDIRECTION)}]);

As you requested in a previous message, this is the screenshot of the "Validate URL"
URL-validada-por-AMP-—-WordPress

Thank you so much again.

@westonruter
Copy link
Member

The mobile redirection JS being served on your site is the built version, so it is minified. That's why it is different.

I'm surprised to see validation errors being attributed to Autoptimize, since that plugin is supposed to short-circuit it's optimizations on AMP pages.

For the Wordfence validation errors, I did write a little plugin that avoids the validation errors from being reported: https://gist.github.com/westonruter/3bad77db53496c7103dd64ca1a0e1cf1

Other plugins you may want to consider suppressing on the AMP settings screen if their functionality is completely broken on the AMP version of a page.

Regardless, none of these issues are related to the infinite redirection issue. That's apparently due to a caching plugin.

@titoabreurj
Copy link
Author

Hi Weston
You were very assertive. I suppressed the Autoptimize plugin in the AMP settings and the Infinite Looping stopped.
I don't want to abuse your goodwill. But what other validation errors from other plugins should I consider, suppress or ignore? Since the looping problem no longer exists?
I am very grateful for your help. I had been trying to solve this problem for months, trying different supports from other plugins even from my hosting and nobody was able to identify the problem. Congratulations.
Thank you very much

@westonruter
Copy link
Member

westonruter commented Jan 8, 2021

I suppressed the Autoptimize plugin in the AMP settings and the Infinite Looping stopped.

@titoabreurj Suppressing Autoptimize is not having an effect.

I'm seeing a non-AMP page at both of these URLs (after first navigating to the non-AMP version):

  1. https://fullworks.net.br/totem-para-alcool-gel-solucao-de-protecao-covid-19/
  2. https://fullworks.net.br/totem-para-alcool-gel-solucao-de-protecao-covid-19/?amp=1

In contrast, I'm seeing an AMP page at both of these URLs, after first visiting the AMP version:

  1. https://fullworks.net.br/merchandising-no-ponto-de-venda/?amp=1
  2. https://fullworks.net.br/merchandising-no-ponto-de-venda/

So again, your page caching plugin is not configured properly to vary caches by the amp query parameter.

I don't want to abuse your goodwill. But what other validation errors from other plugins should I consider, suppress or ignore? Since the looping problem no longer exists?

The validation errors are unrelated to the redirect issue. You'll have to review them to see if the functionality related to them is needed on the page or not. If the functionality still works with the invalid scripts removed, you can leave them as "Removed" and mark the validation errors as Reviewed. If the functionality is broken, then you should instead suppress the plugin from AMP pages.

For additional support for configuring the AMP plugin, please instead open a topic on the support forum This issue you opened we'll limit the scope to preventing the infinite redirection symptom you described (but again, the underlying cause has to be fixed in your page caching plugin).

@westonruter westonruter self-assigned this Jan 11, 2021
@westonruter westonruter added WS:Core Work stream for Plugin core and removed Support Help with setup, implementation, or "How do I?" questions. labels Jan 11, 2021
@westonruter westonruter removed their assignment Jan 12, 2021
@pierlon
Copy link
Contributor

pierlon commented Jan 13, 2021

QA passed

With the Autoptimize plugin modified to always cache the non-AMP version of a page, prior to #5775 it can be observed that the page infinitely navigates to the AMP URL:

before.mp4

And with the fix in place it can be seen that the issue no longer occurs:

after.mp4

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Jan 13, 2021
@westonruter
Copy link
Member

@pierlon So Autoptimize is providing the page caching here? I didn't know that that Autoptimize included page caching, since the its readme states:

If you consider performance important, you really should use one of the many caching plugins to do page caching. Some good candidates to complement Autoptimize that way are e.g. Speed Booster pack, KeyCDN’s Cache Enabler, WP Super Cache or if you use Cloudflare WP Cloudflare Super Page Cache.

But if that's the case, @titoabreurj I recommend opening a support topic with Autoptimize to report that its page caching is failing to vary the cache by the amp query parameter.

@pierlon
Copy link
Contributor

pierlon commented Jan 13, 2021

@pierlon So Autoptimize is providing the page caching here?

Oh no I simply modified the plugin to do so. I attempted to replicate the issue with the (unmodified) WP Super Cache plugin but was unable to do so.

@titoabreurj
Copy link
Author

Hello
I'm having problems with the search console under Indexing request.
When the AMP plugin is activated, the Search console presents an error, without specifying what.
But when I disable the plugin, I can inspect the page, do the published URL test and request indexing without any problem.

Can anyone help me identify the problem?

Captura de tela 2021-02-25 153216

@westonruter
Copy link
Member

@titoabreurj
Copy link
Author

Thanxs Westonruter.
I followed the link and opened a new topic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Changelogged Whether the issue/PR has been added to release notes. WS:Core Work stream for Plugin core
Projects
None yet
3 participants