ampproject / amp-wp

Enable AMP on your WordPress site, the WordPress way.
https://wordpress.org/plugins/amp/
GNU General Public License v2.0
1.79k stars 383 forks source link

Infinite Looping when the AMP plugin is activated #5767

Closed titoabreurj closed 3 years ago

titoabreurj commented 3 years ago

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

westonruter commented 3 years ago

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 commented 3 years ago

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

titoabreurj commented 3 years ago

Hi weston Thanks for answering. Really, sorry ... I thought it might take a while to reply and I ended up disabling AMP. But now, at this moment, I have AMP enabled. You will be able to see what happens when we try to access the site in AMP version.

Thank you


Tito Abreu - Consultor E-commerce 21 3878-9700 | 21 99757-9054

Em qua., 6 de jan. de 2021 às 22:27, Weston Ruter notifications@github.com escreveu:

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

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ampproject/amp-wp/issues/5767#issuecomment-755824513, or unsubscribe https://github.com/notifications/unsubscribe-auth/APXZQE6SCSIUXTBPXJVVEJ3SYUEYLANCNFSM4VYBESUA .

westonruter commented 3 years ago

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:

https://github.com/ampproject/amp-wp/blob/ac8cfe964c3eb034a963ac1f9fb5e254f36a783a/assets/src/mobile-redirection.js#L56-L67

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

titoabreurj commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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).

pierlon commented 3 years ago

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:

https://user-images.githubusercontent.com/16200219/104429144-30077480-557d-11eb-8888-7c65544bcaa0.mp4

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

https://user-images.githubusercontent.com/16200219/104429159-34cc2880-557d-11eb-9725-d15664b8d622.mp4

westonruter commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

@titoabreurj Please open a support topic: https://wordpress.org/support/plugin/amp/reviews/#new-post

titoabreurj commented 3 years ago

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