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

Include PreloadHeroImage among transformers omitted for SSR #5823

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented Jan 31, 2021

Summary

When SSR is disabled via add_filter( 'amp_enable_ssr', '__return_false' ); I found that the data-hero attributes were still getting added, along with prerendering and the i-amphtml-ssr attributes. This transformer should not be running when SSR is disabled.

The result is also an invalid AMP page since it is not marked as transformed:

image

Amends #5350 for #5055.

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added Bug Something isn't working WS:Perf Work stream for Metrics, Performance and Optimizer labels Jan 31, 2021
@westonruter westonruter added this to the v2.1 milestone Jan 31, 2021
@github-actions
Copy link
Contributor

Plugin builds for 2e83720 are ready 🛎️!

@westonruter westonruter merged commit 3729997 into develop Feb 2, 2021
@westonruter westonruter deleted the fix/disabling-ssr branch February 2, 2021 18:47
@pierlon pierlon self-assigned this Apr 24, 2021
@pierlon
Copy link
Contributor

pierlon commented Apr 26, 2021

QA Passed

Disabling SSR does not result in data-hero nor i-amphtml-ssr attributes being added to any image on the page, and the page is valid AMP.

I did notice however that when SSR is disabled, the page becomes invalid AMP when there are amp-bindings:

image

This seems to be because of the OptimizeAmpBind transformer still running even though SSR is disabled. I'll open a PR to fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working WS:Perf Work stream for Metrics, Performance and Optimizer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants