Account for `OPTIMIZATION_DETECTIVE_VERSION` not always being defined for Embed Optimizer #1908

westonruter · 2025-03-05T22:28:16Z

I was doing some testing with Optimization Detective deactivated but with Embed Optimizer active. I started getting a fatal error:

PHP Fatal error:  Uncaught Error: Undefined constant "OPTIMIZATION_DETECTIVE_VERSION" in /code/wp-content/plugins/embed-optimizer/hooks.php:193

This issue was introduced in #1872. It is an issue unique to Embed Optimizer because it does not require Optimization Detective but merely suggests it. This means the OPTIMIZATION_DETECTIVE_VERSION constant is not guaranteed to be defined. It's not needed by Image Prioritizer since it initializes at the od_init action so we guaranteed that the constant will be available.

…or Embed Optimizer

codecov · 2025-03-05T22:37:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.63%. Comparing base (ad006e6) to head (20fea77).
Report is 6 commits behind head on trunk.

Additional details and impacted files

@@           Coverage Diff           @@
##            trunk    #1908   +/-   ##
=======================================
  Coverage   69.63%   69.63%           
=======================================
  Files          86       86           
  Lines        6991     6991           
=======================================
  Hits         4868     4868           
  Misses       2123     2123

Flag	Coverage Δ
multisite	`69.63% <100.00%> (ø)`
single	`39.57% <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.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2025-03-05T22:37:24Z

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 <[email protected]>
Co-authored-by: felixarntz <[email protected]>

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

felixarntz · 2025-03-05T23:05:39Z

plugins/embed-optimizer/hooks.php

 	// As of 1.0.0-beta3, next_tag() allows $query and is beginning to migrate to skip tag closers by default.
 	// In versions prior to this, the method always visited closers and passing a $query actually threw an exception.
-	$tag_query = version_compare( OPTIMIZATION_DETECTIVE_VERSION, '1.0.0-beta3', '>=' )
+	$tag_query = ! defined( 'OPTIMIZATION_DETECTIVE_VERSION' ) || version_compare( OPTIMIZATION_DETECTIVE_VERSION, '1.0.0-beta3', '>=' )


Why is this function's logic running even when Optimization Detective is not active? Is it not something that relies on that plugin?

Also, looking at this again: Why does the Optimization Detective matter for this, given it's passed to the WP_HTML_Tag_Processor instance, which should depend on Core, not OD? Maybe I'm missing something here.

Embed Optimizer does not require Optimization Detective. Without Optimization Detective, it lazy-loads all embeds.

When OD is not active, a WP_HTML_Tag_Processor instance is passed. When OD is active, then an OD_HTML_Tag_Processor instance is passed (a subclass of WP_HTML_Tag_Processor).

Makes sense, thanks.

I didn't know Embed Optimizer was doing this already on its own (albeit less reliably of course).

I'm just realizing that this change and #1872 probably fix Embed Optimizer to work properly when OD is not active, actually. Because the default behavior of next_tag() is to not visit tag closers, and yet the OD subclass was visiting tag closers by default... since no query was being passed in, when OD is not active then it would be skipping all tag closers and the logic that checks $processor->is_tag_closer() would always return false.

Nevermind. The only part of the code that uses is_tag_closer() is for the non-OD context:

performance/plugins/embed-optimizer/hooks.php

Lines 212 to 234 in 20fea77

// This condition ensures that when iterating over an embed inside a larger document that we stop once we reach

// closing </figure> tag. The $processor is an OD_HTML_Tag_Processor when Optimization Detective is iterating

// over all tags in the document, and this embed_optimizer_update_markup() is used as part of the tag visitor

// from Embed Optimizer. On the other hand, if $html_processor is not an OD_HTML_Tag_Processor then this is

// iterating over the tags of the embed markup alone as is passed into the embed_oembed_html filter.

if ( ! $is_isolated ) {

if ( 'FIGURE' === $html_processor->get_tag() ) {

if ( $html_processor->is_tag_closer() ) {

--$figure_depth;

if ( $figure_depth <= 0 ) {

// We reached the end of the embed.

break;

}

} else {

++$figure_depth;

// Move to next element to start looking for IFRAME or SCRIPT tag.

continue;

}

}

if ( 0 === $figure_depth ) {

continue;

}

}

So there wouldn't have been a bug.

felixarntz

Thanks, LGTM!

Account for OPTIMIZATION_DETECTIVE_VERSION not always being defined f…

20fea77

…or Embed Optimizer

westonruter added [Type] Bug An existing feature is broken [Plugin] Embed Optimizer Issues for the Embed Optimizer plugin (formerly Auto Sizes) skip changelog PRs that should not be mentioned in changelogs labels Mar 5, 2025

westonruter added this to the embed-optimizer n.e.x.t milestone Mar 5, 2025

westonruter requested a review from felixarntz March 5, 2025 22:28

westonruter requested a review from adamsilverstein as a code owner March 5, 2025 22:28

westonruter mentioned this pull request Mar 5, 2025

Update OD_HTML_Tag_Processor::next_tag() to allow $query arg and prepare to skip visiting tag closers by default #1872

Merged

felixarntz reviewed Mar 5, 2025

View reviewed changes

felixarntz approved these changes Mar 5, 2025

View reviewed changes

westonruter merged commit 1a7f906 into trunk Mar 5, 2025
31 checks passed

westonruter deleted the fix/undefined-od-version-constant branch March 5, 2025 23:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Account for `OPTIMIZATION_DETECTIVE_VERSION` not always being defined for Embed Optimizer #1908

Account for `OPTIMIZATION_DETECTIVE_VERSION` not always being defined for Embed Optimizer #1908

Uh oh!

westonruter commented Mar 5, 2025 •

edited

Loading

Uh oh!

codecov bot commented Mar 5, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 5, 2025 •

edited

Loading

Uh oh!

felixarntz Mar 5, 2025

Uh oh!

westonruter Mar 5, 2025

Uh oh!

felixarntz Mar 5, 2025

Uh oh!

westonruter Mar 6, 2025

Uh oh!

westonruter Mar 10, 2025

Uh oh!

felixarntz left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	// This condition ensures that when iterating over an embed inside a larger document that we stop once we reach
	// closing </figure> tag. The $processor is an OD_HTML_Tag_Processor when Optimization Detective is iterating
	// over all tags in the document, and this embed_optimizer_update_markup() is used as part of the tag visitor
	// from Embed Optimizer. On the other hand, if $html_processor is not an OD_HTML_Tag_Processor then this is
	// iterating over the tags of the embed markup alone as is passed into the embed_oembed_html filter.
	if ( ! $is_isolated ) {
	if ( 'FIGURE' === $html_processor->get_tag() ) {
	if ( $html_processor->is_tag_closer() ) {
	--$figure_depth;
	if ( $figure_depth <= 0 ) {
	// We reached the end of the embed.
	break;
	}
	} else {
	++$figure_depth;
	// Move to next element to start looking for IFRAME or SCRIPT tag.
	continue;
	}
	}
	if ( 0 === $figure_depth ) {
	continue;
	}
	}

Account for OPTIMIZATION_DETECTIVE_VERSION not always being defined for Embed Optimizer #1908

Account for OPTIMIZATION_DETECTIVE_VERSION not always being defined for Embed Optimizer #1908

Uh oh!

Conversation

westonruter commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felixarntz Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

westonruter Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

felixarntz Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

westonruter Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

westonruter Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Account for `OPTIMIZATION_DETECTIVE_VERSION` not always being defined for Embed Optimizer #1908

Account for `OPTIMIZATION_DETECTIVE_VERSION` not always being defined for Embed Optimizer #1908

westonruter commented Mar 5, 2025 •

edited

Loading

codecov bot commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 5, 2025 •

edited

Loading