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

amp-onerror AMP validation issues #7763

Closed
swissspidy opened this issue May 27, 2021 · 11 comments
Closed

amp-onerror AMP validation issues #7763

swissspidy opened this issue May 27, 2021 · 11 comments
Labels
AMP Output Issues related to AMP output and validation P0 Critical, drop everything Type: Bug Something isn't working

Comments

@swissspidy
Copy link
Collaborator

swissspidy commented May 27, 2021

Bug Description

Context:

ampproject/amphtml#22543
ampproject/amphtml#34531
ampproject/amp-toolbox-php#211

AMP upstream has very recently added amp-onerror validation, which causes validation errors on sites with transformed AMP that don't use ESM. That's us.

Example site: https://testsite.cmsdevrel.com/web-stories/thailand-copy/

There have been multiple user reports in the forums so far about the AMP validation errors this causes.

Possible Solutions

  1. Re-enable ESM usage (Re-enable AMP ES Module usage #7649)
  2. Update AmpBoilerplateErrorHandler transformer as per `add amp-onerror validation for v0.js or v0.mjs ampproject/amphtml#34531 (comment) / Add support for amp-onerror on transformed pages (with ESM scripts) ampproject/amp-toolbox-php#211
  3. Disable AmpBoilerplateErrorHandler transformer altogether

Expected Behaviour

No AMP validation errors.

Steps to Reproduce

  1. Validate any web story when using v1.7.2 of the plugin
    Example: https://testsite.cmsdevrel.com/web-stories/thailand-copy/

Screenshots

Additional Context

  • Plugin Version: 1.7.2
  • WordPress Version: 5.7
  • Operating System: macOS
  • Browser: Chrome

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance Criteria

Implementation Brief

@swissspidy swissspidy added Type: Bug Something isn't working P0 Critical, drop everything AMP Output Issues related to AMP output and validation labels May 27, 2021
@swissspidy
Copy link
Collaborator Author

FYI @choumx

@karinclimber
Copy link

karinclimber commented May 28, 2021

We have also now seen this same problem on 150 sites.

@zdenys
Copy link

zdenys commented May 28, 2021

We are seeing this on WordPress.com sites too.

In one case specifically, on attempting to validate a page with https://search.google.com/test/amp there is a validation error The mandatory text inside tag 'script amp-onerror' is missing or incorrect.. The related HTML is:

<script amp-onerror="">document.querySelector("script[src*='/v0.js']").onerror=function(){document.querySelector('style[amp-boilerplate]').textContent=''}</script><style amp-boilerplate="">body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate="">body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>

@LuckynaSan
Copy link
Collaborator

@karinclimber @zdenys Thank you both for your reports. While we are actively looking into how to best handle this issue, you can download and install this mini-plugin as a temporary workaround. We appreciate your patience ask to please standby for further updates.

@karinclimber
Copy link

It looks like that does fix it - I tested it on one site. But we will wait for you guys to get things sorted on your end before installing it and then later uninstalling it on 150 sites ;) Thanks for working on it!

@joannathenewsvoice
Copy link

This workaround helped to see the errors valid in the tool but every time when google hits the page from the webmaster the page is first considered as “invalid” web story and we have to fetch manually and test live its a valid page.

– Google first attempt is crawl the page is invalid so the pages don't consider as valid amp or amp story-
– if we fetch manually then it comes a valid but how many we fetch every day and we cant control what pages google is considering them as valid.

check the screenshots here.

Google first time hit – https://jmp.sh/U8ucb4t invalid web-story
Manually testing the page – https://jmp.sh/LwUrrlh

the work around is for plugin but not for google.

All the stories that we are publishing are considered as invalid and going waste.

– Joanna

@mobiliodevelopment
Copy link

Quick fix change script to:
<script amp-onerror>[].slice.call(document.querySelectorAll("script[src*='/v0.js'],script[src*='/v0.mjs']")).forEach(function(s){s.onerror=function(){document.querySelector('style[amp-boilerplate]').textContent=''}})</script>
And this will fix error ASAP.

Peter

@westonruter
Copy link
Collaborator

This is an issue with the AMP Validator, not with the Web Stories plugin per se. See ampproject/amphtml#22543 (comment). The AMP Validator should be fixed in the next day or so. In the mean time, patching the plugin per above or disabling amp-onerror via #7763 (comment) will prevent the error.

@mobiliodevelopment
Copy link

@westonruter Thank you!

@swissspidy
Copy link
Collaborator Author

As per ampproject/amphtml#22543 (comment), this has already been fixed on https://validator.amp.dev. It should be fixed in Search Console as well by the end of the week.

So you can safely keep the mini-plugin active and then remove it again next week.

@swissspidy
Copy link
Collaborator Author

Closing now as this has been fixed in Search Console.

Feel free to delete the mini-plugin accordingly on your site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMP Output Issues related to AMP output and validation P0 Critical, drop everything Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants