Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Add support for script amp-onerror #1146

Closed
honeybadgerdontcare opened this issue Feb 24, 2021 · 7 comments · Fixed by #1195
Closed

Add support for script amp-onerror #1146

honeybadgerdontcare opened this issue Feb 24, 2021 · 7 comments · Fixed by #1195

Comments

@honeybadgerdontcare
Copy link

See ampproject/amphtml#22543 (comment).

Support adding <script amp-onerror>document.querySelector("script[src*='/v0.js']").onerror=function(){document.querySelector('style[amp-boilerplate]').textContent=''}</script> to the <head> in AMP Optimizer.
@sebastianbenz
Copy link
Collaborator

Thanks! When will this be available in the validator?

@honeybadgerdontcare
Copy link
Author

At the latest in two weeks. See cl/359398014.

@jridgewell
Copy link
Contributor

We should probably wait for b/163852894 before adding it further.

@westonruter westonruter mentioned this issue Feb 25, 2021
Closed
@westonruter
Copy link
Member

We should probably wait for b/163852894 before adding it further.

Fixed on March 5th.

This was referenced Apr 1, 2021
Closed
Closed
@schlessera
Copy link
Collaborator

I'm currently trying to implement this in the Node.JS and PHP versions of the Optimizer, but I'm having trouble producing the exact output the validator requires, as the DOM abstractions tend to introduce whitespace which is enough to invalidate the script.

@schlessera schlessera mentioned this issue Apr 9, 2021
Merged
@jridgewell
Copy link
Contributor

Unfortunately, we cannot have whitespace here without the SHA hash being changed: https://elfin-saber-afternoon.glitch.me/

@schlessera
Copy link
Collaborator

I turns out it was not the DOM, but rather a beautify script that was running after the fact on the test files. This seems to be resolved for now: #1195 (comment)

# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add AmpBoilerplateErrorHandler transformer schlessera/amp-toolbox
5 participants
@schlessera @jridgewell @westonruter @sebastianbenz @honeybadgerdontcare