Account for OPTIMIZATION_DETECTIVE_VERSION not always being defined for Embed Optimizer

[westonruter]

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.

[codecov]

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]

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.

[felixarntz]

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.

[westonruter]

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).

[felixarntz]

Makes sense, thanks.

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

[westonruter]

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.

[westonruter]

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!