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

Adding an amp-access-scroll element to the page fails to add required scripts #269

Closed
westonruter opened this issue Mar 19, 2021 · 4 comments
Milestone

Comments

@westonruter
Copy link
Member

Bug Description

As reported on support forum, when adding amp-access for Scroll to the page, the amp-access-scroll component script fail to be added automatically.

For example, given this code:

add_action('wp_head', function () {
	?>
	<script id="amp-access" type="application/json">
		{
			"vendor": "scroll",
			"namespace": "scroll"
		}
	</script>
	<?php
});

add_action( 'wp_footer', function () {
	?>
	<section amp-access="NOT scroll.scroll">
		ad goes here
	</section>
	<?php
} );

The result is only that the main amp-access script is added to the page:

<script async custom-element="amp-access" src="https://cdn.ampproject.org/v0/amp-access-0.1.js"></script>

Expected Behaviour

Expected the amp-access-scroll to also be added:

<script async custom-element="amp-access-scroll" src="https://cdn.ampproject.org/v0/amp-access-scroll-0.1.js"></script>

Steps to reproduce

  1. Put the above PHP code in a plugin.
  2. View an AMP page in Standard, Transitional, or non-legacy Reader mode.

Workaround

There is a workaround and that is to manually add the amp-access-scroll component to the page. This can either be done by enqueueing the script:

wp_enqueue_script( 'amp-access-scroll' );

Also, the script can just be manually printed out anywhere in the page:

<script async custom-element="amp-access-scroll" src="https://cdn.ampproject.org/v0/amp-access-scroll-0.1.js"></script>

The AMP plugin will automatically move it to the head.

Something to investigate is why the amp-access-scroll script is not removed automatically given that it was not added automatically in the first place. The AMP plugin is supposed to be removing components automatically which are not needed in the document.


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

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@westonruter
Copy link
Member Author

This should rather be solved in the Optimizer with the AutoExtension transformer logic.

@westonruter
Copy link
Member Author

I just tried pasting the above snippets into the AMP playground and the only script which was automatically added was:

<script async custom-element="amp-access" src="https://cdn.ampproject.org/v0/amp-access-0.1.js"></script>

And the document remains valid even though it lacks the amp-access-scroll extension.

@schlessera
Copy link
Collaborator

Opened an issue here: ampproject/amphtml#33801

@westonruter westonruter changed the title Adding an amp-access-scroll element to the page fails to add required scripts Adding an amp-access-scroll element to the page fails to add required scripts Jun 8, 2021
@westonruter westonruter transferred this issue from ampproject/amp-wp Jul 3, 2021
@schlessera
Copy link
Collaborator

Fixed as a work-around via #297

@schlessera schlessera added this to the 0.8.0 milestone Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants