Skip to content

Eliminate use of empty() in Embed Optimizer and Optimization Detective #1391

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

Merged
westonruter merged 4 commits into from
Jul 23, 2024
Merged

Eliminate use of empty() in Embed Optimizer and Optimization Detective #1391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
555b9fe
Reduce code duplication
westonruter Jul 23, 2024
2a78df0
Remove empty() from embed-optimizer
westonruter Jul 23, 2024
2e8605f
Remove empty() from optimization-detective
westonruter Jul 23, 2024
d7f0240
Ignore remaining paths for empty.notAllowed errors
westonruter Jul 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions phpstan.neon.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,9 @@ parameters:
-
# TODO: Remove this to fix https://github.com/WordPress/performance/issues/1219
identifier: empty.notAllowed
paths:
- */tests/*
- plugins/dominant-color-images
- plugins/performance-lab
- plugins/speculation-rules
- plugins/webp-uploads
23 changes: 13 additions & 10 deletions plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,44 +60,47 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool {
* for GET requests, as POST requests are not likely to be part of the critical rendering path.
*/
$preconnect_hrefs = array();
if ( true === $processor->has_class( 'wp-block-embed-youtube' ) ) {
$has_class = static function ( string $wanted_class ) use ( $processor ): bool {
return true === $processor->has_class( $wanted_class );
};
Comment on lines +63 to +65
Copy link
Member

@swissspidy swissspidy Jul 23, 2024

Choose a reason for hiding this comment

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

Is this just to reduce repetition below, or actually because of PHPStan? 🤔

Copy link
Member Author

@westonruter westonruter Jul 23, 2024

Choose a reason for hiding this comment

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

Ah yes, just to reduce repetition.

if ( $has_class( 'wp-block-embed-youtube' ) ) {
$preconnect_hrefs[] = 'https://www.youtube.com';
$preconnect_hrefs[] = 'https://i.ytimg.com';
} elseif ( true === $processor->has_class( 'wp-block-embed-twitter' ) ) {
} elseif ( $has_class( 'wp-block-embed-twitter' ) ) {
$preconnect_hrefs[] = 'https://syndication.twitter.com';
$preconnect_hrefs[] = 'https://pbs.twimg.com';
} elseif ( true === $processor->has_class( 'wp-block-embed-vimeo' ) ) {
} elseif ( $has_class( 'wp-block-embed-vimeo' ) ) {
$preconnect_hrefs[] = 'https://player.vimeo.com';
$preconnect_hrefs[] = 'https://f.vimeocdn.com';
$preconnect_hrefs[] = 'https://i.vimeocdn.com';
} elseif ( true === $processor->has_class( 'wp-block-embed-spotify' ) ) {
} elseif ( $has_class( 'wp-block-embed-spotify' ) ) {
$preconnect_hrefs[] = 'https://apresolve.spotify.com';
$preconnect_hrefs[] = 'https://embed-cdn.spotifycdn.com';
$preconnect_hrefs[] = 'https://encore.scdn.co';
$preconnect_hrefs[] = 'https://i.scdn.co';
} elseif ( true === $processor->has_class( 'wp-block-embed-videopress' ) || true === $processor->has_class( 'wp-block-embed-wordpress-tv' ) ) {
} elseif ( $has_class( 'wp-block-embed-videopress' ) || $has_class( 'wp-block-embed-wordpress-tv' ) ) {
$preconnect_hrefs[] = 'https://video.wordpress.com';
$preconnect_hrefs[] = 'https://public-api.wordpress.com';
$preconnect_hrefs[] = 'https://videos.files.wordpress.com';
$preconnect_hrefs[] = 'https://v0.wordpress.com'; // This does not appear to be a load-balanced domain since v1.wordpress.com is not valid.
} elseif ( true === $processor->has_class( 'wp-block-embed-instagram' ) ) {
} elseif ( $has_class( 'wp-block-embed-instagram' ) ) {
$preconnect_hrefs[] = 'https://www.instagram.com';
$preconnect_hrefs[] = 'https://static.cdninstagram.com';
$preconnect_hrefs[] = 'https://scontent.cdninstagram.com';
} elseif ( true === $processor->has_class( 'wp-block-embed-tiktok' ) ) {
} elseif ( $has_class( 'wp-block-embed-tiktok' ) ) {
$preconnect_hrefs[] = 'https://www.tiktok.com';
// Note: The other domains used for TikTok embeds include https://lf16-tiktok-web.tiktokcdn-us.com,
// https://lf16-cdn-tos.tiktokcdn-us.com, and https://lf16-tiktok-common.tiktokcdn-us.com among others
// which either appear to be geo-targeted ('-us') _or_ load-balanced ('lf16'). So these are not added
// to the preconnected hosts.
} elseif ( true === $processor->has_class( 'wp-block-embed-amazon' ) ) {
} elseif ( $has_class( 'wp-block-embed-amazon' ) ) {
$preconnect_hrefs[] = 'https://read.amazon.com';
$preconnect_hrefs[] = 'https://m.media-amazon.com';
} elseif ( true === $processor->has_class( 'wp-block-embed-soundcloud' ) ) {
} elseif ( $has_class( 'wp-block-embed-soundcloud' ) ) {
$preconnect_hrefs[] = 'https://w.soundcloud.com';
$preconnect_hrefs[] = 'https://widget.sndcdn.com';
// Note: There is also https://i1.sndcdn.com which is for the album art, but the '1' indicates it may be geotargeted/load-balanced.
} elseif ( true === $processor->has_class( 'wp-block-embed-pinterest' ) ) {
} elseif ( $has_class( 'wp-block-embed-pinterest' ) ) {
$preconnect_hrefs[] = 'https://assets.pinterest.com';
$preconnect_hrefs[] = 'https://widgets.pinterest.com';
$preconnect_hrefs[] = 'https://i.pinimg.com';
Expand Down
3 changes: 2 additions & 1 deletion plugins/embed-optimizer/hooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ function embed_optimizer_update_markup( WP_HTML_Tag_Processor $html_processor, b

if ( 'IFRAME' === $html_processor->get_tag() ) {
$loading_value = $html_processor->get_attribute( 'loading' );
if ( empty( $loading_value ) ) {
// Per the HTML spec: "The attribute's missing value default and invalid value default are both the Eager state".
if ( 'lazy' !== $loading_value ) {
++$iframe_count;
if ( ! $html_processor->set_bookmark( $bookmark_names['iframe'] ) ) {
throw new Exception(
Expand Down
2 changes: 1 addition & 1 deletion plugins/optimization-detective/storage/data.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ function od_get_current_url(): string {
$parsed_url = array();
}

if ( empty( $parsed_url['scheme'] ) ) {
if ( ! isset( $parsed_url['scheme'] ) ) {
$parsed_url['scheme'] = is_ssl() ? 'https' : 'http';
}
if ( ! isset( $parsed_url['host'] ) ) {
Expand Down