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 error due to invalid attribute fetchpriority #792

Closed
pereirinha opened this issue Jul 28, 2023 · 4 comments
Closed

AMP error due to invalid attribute fetchpriority #792

pereirinha opened this issue Jul 28, 2023 · 4 comments
Labels
[Module] fetchpriority Issues for the fetchpriority module [Type] Bug An existing feature is broken

Comments

@pereirinha
Copy link
Member

Bug Description

The fetchpriority feature is currently in experimental mode, and it's currently not supported by AMP.

This will cause content pieces to be flagged with an error related to an invalid attribute, and there might be a conflict with the motto Enhancement for most, regression for none.

Steps to reproduce

  1. Ensure you have the Performal Lab and AMP plugins installed and active;
  2. Go to the AMP Settings page and make sure to use either Standard or Transitional template mode;
  3. Visit a published post/page in the FE that has an image;
  4. In the admin bar, client the menu AMP, and click View AMP version. If this menu option isn't available, it might be that you're already seeing the AMP version;
  5. You should see a yellow icon next to the AMP word. Expand the menu and click Validate URL;
  6. You should see a similar error as the attached screenshot;

Screenshots

Screenshot 2023-07-28 at 10 41 36
@pereirinha pereirinha added the [Type] Bug An existing feature is broken label Jul 28, 2023
@swissspidy
Copy link
Member

This should be reported at https://github.com/ampproject/amp-wp/

WordPress is adding fetchpriority correctly to <img> tags, but then the AMP plugin is turning those into <amp-img> and not removing the attribute.

FWIW you can use add_filter( 'amp_native_img_used', '__return_true' ); to keep using the native <img> even when using AMP — it's totally supported and valid AMP.

@swissspidy swissspidy closed this as not planned Won't fix, can't repro, duplicate, stale Jul 28, 2023
@swissspidy swissspidy added the [Module] fetchpriority Issues for the fetchpriority module label Jul 28, 2023
@felixarntz
Copy link
Member

Agreed, fetchpriority is in fact no longer experimental but part of the HTML standard. If amp-img does not support it, I think it needs to be resolved on that side.

Given that AMP is not associated with the WordPress core project and not a web standard either, I don't think it would be appropriate to fix this in the Performance Lab plugin.

@westonruter
Copy link
Member

This first has to be allowed by the AMP validator before it is allowed by the AMP plugin. For that, please see ampproject/amphtml#38715

@pereirinha
Copy link
Member Author

Thanks all for taking the time to evaluate this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Module] fetchpriority Issues for the fetchpriority module [Type] Bug An existing feature is broken
Projects
None yet
Development

No branches or pull requests

4 participants