Skip to content

Remove no-cache and max-age=0 from bfcache failure conditions - #1826

Merged
westonruter merged 4 commits into
release/3.8.0from
fix/bfcache-conditions
Jan 26, 2025
Merged

westonruter merged 4 commits into
release/3.8.0from
fix/bfcache-conditions

Conversation

@westonruter

Copy link
Copy Markdown
Member

Summary

Fixes #1825

@westonruter westonruter added [Type] Bug An existing feature is broken [Plugin] Performance Lab [slug: performance-lab] skip changelog PRs that should not be mentioned in changelogs labels Jan 25, 2025
@westonruter westonruter added this to the performance-lab 3.8.0 milestone Jan 25, 2025
@github-actions

github-actions Bot commented Jan 25, 2025 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@westonruter
westonruter changed the base branch from trunk to release/3.8.0 January 25, 2025 21:47
@codecov

codecov Bot commented Jan 25, 2025 •

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (release/3.8.0@2b1bf18). Learn more about missing BASE report.

Additional details and impacted files
@@               Coverage Diff                @@
##             release/3.8.0    #1826   +/-   ##
================================================
  Coverage                 ?   65.86%           
================================================
  Files                    ?       88           
  Lines                    ?     6855           
  Branches                 ?        0           
================================================
  Hits                     ?     4515           
  Misses                   ?     2340           
  Partials                 ?        0           
Flag Coverage Δ
multisite 65.86% <100.00%> (?)
single 38.45% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This was referenced Jan 25, 2025

@felixarntz felixarntz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@westonruter Great catch, LGTM! Just one non-blocking feedback.

)
)
),
__( "The <code>Cache-Control</code> response header for an unauthenticated request to the home page includes the <code>no-store</code> directive. This can affect the performance of your site by preventing fast back/forward navigations (via the browser's bfcache).", 'performance-lab' ),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker, but for the code constructs using placeholders would be nice, since they shouldn't be translatable - plus it avoids the need for translators to deal with HTML in the string, which is always good if it can be avoided.

Suggested change
__( "The <code>Cache-Control</code> response header for an unauthenticated request to the home page includes the <code>no-store</code> directive. This can affect the performance of your site by preventing fast back/forward navigations (via the browser's bfcache).", 'performance-lab' ),
sprintf(
/* translators: 1: header name, 2: problematic directive */
__( "The %1$s response header for an unauthenticated request to the home page includes the %2$s directive. This can affect the performance of your site by preventing fast back/forward navigations (via the browser's bfcache).", 'performance-lab' ),
'<code>Cache-Control</code>',
'<code>no-store</code>'
),

@westonruter westonruter Jan 26, 2025 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works: ffa9c25 (with a couple tweaks)

image

Co-authored-by: Felix Arntz <felixarntz@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Plugin] Performance Lab [slug: performance-lab] skip changelog PRs that should not be mentioned in changelogs [Type] Bug An existing feature is broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test for bfcache compatibility incorrectly flags use of no-cache and max-age=0

2 participants