Fix Optimization Detective compatibility with WooCommerce when Coming Soon page is served #1565

westonruter · 2024-09-28T23:02:05Z

When testing WooCommerce, I discovered an unexpected result when attempting to access the Shop page:

In my local testing site, I imported some sample products but didn’t “launch” the store yet, so WooCommerce is attempting to serve the Coming Soon page to a logged-out user. The relevant code is ComingSoonRequestHandler::handle_template_include() which filters template_include to conditionally return null on FSE themes:

		if ( $is_fse_theme ) {
			// Since we've already rendered a template, return null to ensure no other template is rendered.
			return null;
		} else {
			// In non-FSE themes, other templates will still be rendered.
			// We need to exit to prevent further processing.
			exit();
		}

This causes a fatal error in Optimization Detective:

PHP Fatal error:  Uncaught TypeError: od_buffer_output(): Argument #1 ($passthrough) must be of type string, null given, called in /var/www/html/wp-includes/class-wp-hook.php on line 324 and defined in /var/www/html/wp-content/plugins/optimization-detective/optimization.php:33
Stack trace:
#0 /var/www/html/wp-includes/class-wp-hook.php(324): od_buffer_output(NULL)
#1 /var/www/html/wp-includes/plugin.php(205): WP_Hook->apply_filters(NULL, Array)
#2 /var/www/html/wp-includes/template-loader.php(104): apply_filters('template_includ...', '/var/www/html/w...')
#3 /var/www/html/wp-blog-header.php(19): require_once('/var/www/html/w...')
#4 /var/www/html/index.php(17): require('/var/www/html/w...')
#5 {main}
  thrown in /var/www/html/wp-content/plugins/optimization-detective/optimization.php on line 33

This is because od_buffer_output() is erroneously defined with a strict string type for the method parameter. Well, I suppose WooCommerce is also erroneously returning null when it should instead return an empty string (which has the same effect) since the template_include filter is defined as passing a string and not string|null.

In any case, since we don’t actually do anything with the variable we can just remove the typing. The result is the page as expected:

…in template_include filter

github-actions · 2024-09-28T23:02:19Z

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 · 2024-09-28T23:12:10Z

Note that Optimization Detective does not operate on this Coming Soon page because WooCommerce is outputting it inside the template_include filter at priority 10, whereas Optimization Detective starts output buffering in the template_include filter at PHP_INT_MAX. If it were changed to PHP_INT_MIN then it would apply optimizations on this Coming Soon page as well. Something to consider for the future. I had chosen PHP_INT_MAX because I expected that when merged into core it would call ob_start() after the following line in wp-includes/template-loader.php:

$template = apply_filters( 'template_include', $template );

WooCommerce’s injecting a template override in the template_include filter as well as Optimization Detective starting output buffering in the template_include filter are both somewhat abuses of this filter.

westonruter · 2024-09-28T23:33:57Z

I also opened a Woo PR to fix this issue as it could cause problems with other plugins: woocommerce/woocommerce#51751

felixarntz · 2024-09-30T17:43:06Z

plugins/optimization-detective/optimization.php

 *
- * @param string $passthrough Value for the template_include filter which is passed through.
- * @return string Unmodified value of $passthrough.
+ * @param string|mixed $passthrough Value for the template_include filter which is passed through.


Would be good to include a note here explaining why that’s not always a string.

We do the |mixed type elsewhere for callbacks for filters. But in general the reason is that other plugins may return anything in filter callbacks, so there is no guarantee for the type.

So you want something like the following?

Suggested change

* @param string|mixed $passthrough Value for the template_include filter which is passed through.

* @param string|mixed $passthrough Value for the template_include filter which is passed through. (A plugin may have filtered this to return `null` or any other value type.)

Ah, nevermind then. I somehow was confused thinking this was a callback to a PHP function like ob_start(), but since it’s a filter callback, it’s fine without a comment.

Fix Optimization Detective compatibility with Woo which returns null …

e06a521

…in template_include filter

westonruter added [Type] Bug An existing feature is broken [Plugin] Optimization Detective Issues for the Optimization Detective plugin labels Sep 28, 2024

westonruter added this to the optimization-detective n.e.x.t milestone Sep 28, 2024

westonruter requested a review from felixarntz as a code owner September 28, 2024 23:02

westonruter mentioned this pull request Sep 28, 2024

Coming Soon: Return empty string from template_include filter instead of null to avoid PHP fatal error with conflicting plugins using strict types woocommerce/woocommerce#51751

Merged

12 tasks

felixarntz approved these changes Sep 30, 2024

View reviewed changes

westonruter merged commit 76685b0 into trunk Sep 30, 2024

westonruter deleted the fix/od-woo-coming-soon-page-compat branch September 30, 2024 17:49

티스토리 수익 글 보기

티스토리 수익 글 보기

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix Optimization Detective compatibility with WooCommerce when Coming Soon page is served #1565

Fix Optimization Detective compatibility with WooCommerce when Coming Soon page is served #1565

Uh oh!

westonruter commented Sep 28, 2024

Uh oh!

github-actions bot commented Sep 28, 2024 •

edited

Loading

Uh oh!

westonruter commented Sep 28, 2024

Uh oh!

westonruter commented Sep 28, 2024

Uh oh!

felixarntz Sep 30, 2024

Uh oh!

westonruter Sep 30, 2024 •

edited

Loading

Uh oh!

felixarntz Sep 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	* @param string\|mixed $passthrough Value for the template_include filter which is passed through.
	* @param string\|mixed $passthrough Value for the template_include filter which is passed through. (A plugin may have filtered this to return `null` or any other value type.)

티스토리 수익 글 보기

티스토리 수익 글 보기

Fix Optimization Detective compatibility with WooCommerce when Coming Soon page is served #1565

Fix Optimization Detective compatibility with WooCommerce when Coming Soon page is served #1565

Uh oh!

Conversation

westonruter commented Sep 28, 2024

Uh oh!

github-actions bot commented Sep 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

westonruter commented Sep 28, 2024

Uh oh!

westonruter commented Sep 28, 2024

Uh oh!

felixarntz Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

westonruter Sep 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixarntz Sep 30, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Sep 28, 2024 •

edited

Loading

westonruter Sep 30, 2024 •

edited

Loading