Skip to content

Introduce AVIF header health check #1612

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.

Sign up for GitHub

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

Jump to bottom
Merged
mukeshpanchal27 merged 15 commits into from
Oct 28, 2024
Merged

Introduce AVIF header health check #1612

mukeshpanchal27 merged 15 commits into from
Oct 28, 2024

Conversation

@adamsilverstein
Copy link
Member

@adamsilverstein adamsilverstein commented Oct 21, 2024 •
edited
Loading

Summary

Add a health check to verify that AVIF images are served correctly.

Fixes #1263

Relevant technical choices

Currently this offers miinimal suggestions for how to fix the issue.

@github-actions
Copy link

github-actions bot commented Oct 21, 2024 •
edited
Loading

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: adamsilverstein <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: mukeshpanchal27 <[email protected]>
Co-authored-by: swissspidy <[email protected]>

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

@adamsilverstein adamsilverstein added [Plugin] Performance Lab Issue relates to work in the Performance Lab Plugin only [Type] Enhancement A suggestion for improvement of an existing feature labels Oct 21, 2024
@swissspidy
Copy link
Member

swissspidy commented Oct 21, 2024

Great idea! Definitely re-using this for https://github.com/swissspidy/media-experiments to check the content type of WASM files.

@swissspidy swissspidy added this to the performance-lab n.e.x.t milestone Oct 21, 2024
@swissspidy swissspidy mentioned this pull request Oct 21, 2024
Closed
westonruter
westonruter reviewed Oct 21, 2024
View reviewed changes
plugins/performance-lab/includes/site-health/avif-headers/helper.php
*/
function avif_headers_check_avif_headers_test(): array {
$result = array(
'label' => __( 'Your site sends AVIF image headers', 'performance-lab' ),
Copy link
Member

@westonruter westonruter Oct 21, 2024

Choose a reason for hiding this comment

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

I can't remember the specifics of escaping fields like this.

Copy link
Member Author

@adamsilverstein adamsilverstein Oct 22, 2024

Choose a reason for hiding this comment

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

I will match other instances

Copy link
Member Author

@adamsilverstein adamsilverstein Oct 23, 2024

Choose a reason for hiding this comment

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

Escaping for labels is only included in plugins/performance-lab/tests/data/class-site-health-mock-responses.php, maybe we should remove it there.

mukeshpanchal27
mukeshpanchal27 reviewed Oct 22, 2024
View reviewed changes
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Left some feedbacks.

Does it possible that Site health shows Your site does not support AVIF as failed but Your site sends AVIF image headers return headers 🤔

@adamsilverstein
Copy link
Member Author

adamsilverstein commented Oct 22, 2024

Does it possible that Site health shows Your site does not support AVIF as failed but Your site sends AVIF image headers return headers 🤔

@mukeshpanchal27 yes, it is possible - AVIF server support and AVIF file header support are two very separate things.

@swissspidy
Copy link
Member

swissspidy commented Oct 22, 2024

Does it possible that Site health shows Your site does not support AVIF as failed but Your site sends AVIF image headers return headers 🤔

@mukeshpanchal27Image yes, it is possible - AVIF server support and AVIF file header support are two very separate things.

Begs the question: is it worth adding this health check if the server doesn't support AVIF files anyway?

@mukeshpanchal27
Copy link
Member

mukeshpanchal27 commented Oct 22, 2024

Begs the question: is it worth adding this health check if the server doesn't support AVIF files anyway?

That's my next question. Thanks @swissspidy for asking.

@adamsilverstein
Copy link
Member Author

adamsilverstein commented Oct 22, 2024

Begs the question: is it worth adding this health check if the server doesn't support AVIF files anyway?

I thought about that and I'd say yes - one reason is you could potentially migrate a site with AVIF images to a new server where they still need to get served. Also, AVIF files could be included in the theme even if not supported by the server for uploads, eg as part of a template.

westonruter
westonruter reviewed Oct 22, 2024
View reviewed changes
@mukeshpanchal27 mukeshpanchal27 merged commit 692a9c5 into WordPress:trunk Oct 28, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@westonruter westonruter westonruter approved these changes

@mukeshpanchal27 mukeshpanchal27 mukeshpanchal27 approved these changes

@felixarntz felixarntz Awaiting requested review from felixarntz felixarntz is a code owner

@thelovekesh thelovekesh Awaiting requested review from thelovekesh thelovekesh is a code owner

@swissspidy swissspidy Awaiting requested review from swissspidy

Assignees

No one assigned

Labels

[Plugin] Performance Lab Issue relates to work in the Performance Lab Plugin only [Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Milestone

performance-lab 3.6.0

Development

Successfully merging this pull request may close these issues.

Site Health: detect web server AVIF mime type support

4 participants

@adamsilverstein @swissspidy @mukeshpanchal27 @westonruter