AREOI_Styles::add_block_styles() can fail in some non-single contexts

  • Resolved Butterfingers

    (@cantbelieveitsnotbutter)


    1 year, 6 months ago

    Hi @areoimiles,

    There appears to be a need for some logic for add_block_styles() on line 179 of file class.areoi.styles.php where parse_blocks( ) attempts to get_the_content(). In my case, which may be a bit of an edge case, I get the following when I attempt to load a taxonomy archive which has a custom search parameter applied to it:

    Fatal error: Uncaught TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in /var/www/web/wp/wp-includes/post-template.php:321

    In post-template.php on 290 there’s an $elements array that’s created if $post is null (which it is in my case) which includes the global $page, which is also null in my case. On line 321 there’s a count() on $page that throws the fatal error.

    I’m not sure whether post-template.php would benefit from some conditionals or whether it’s designed to fail loudly, but in any case here’s a potential fix, or at least a starting point for one, in class.areoi.styles.php:

    Replace line 179 with:

    global $page;
if ( null === $page ) {
	$standard_blocks = false;
} else {
	$standard_blocks = parse_blocks( get_the_content() );
}

    Replace line 227 with:

    if ( ! $standard_blocks ) {
	$blocks = array_merge( $page_reuseable_blocks, $reuseable_blocks, $template_blocks, $widget_blocks );
} else {
	$blocks = array_merge( $standard_blocks, $page_reuseable_blocks, $reuseable_blocks, $template_blocks, $widget_blocks );
}

    This solves it for me. I’m not sure if it introduces any other issues as I haven’t tested it extensively, but it seems to work fine.

    If you’d prefer to get pull requests for this sort of thing, is the plugin repo hosted anywhere public?

    Thanks for all the maintenance on this plugin. It’s truly great.

Viewing 11 replies - 1 through 11 (of 11 total)
  • Plugin Author Miles

    (@areoimiles)

    Hi @cantbelieveitsnotbutter

    Thank you for flagging this issue and for giving so much detail on the potential solution. I have an update to release within the next week so I will make sure this is implemented and I will give it a proper test to make sure it doesn’t break anything else.

    I will drop you a message once the update is ready. Thanks again for flagging this.

    Miles

    PS: We don’t currently have a public repo so adding tickets on here is perfect.

    Plugin Author Miles

    (@areoimiles)

    Hi @cantbelieveitsnotbutter

    I have just released an update which should resolve this problem for you. I wasn’t able to re-create the issue locally but I have put a fix in place based on the solution you included in your post.

    Hopefully this solves your issue but if you continue to have problems please don’t hesitate to get back in touch.

    Thanks
    Miles

    Thread Starter Butterfingers

    (@cantbelieveitsnotbutter)

    Thanks, @areoimiles, for the quick fix!

    Plugin Author Miles

    (@areoimiles)

    Hi @cantbelieveitsnotbutter

    I was hoping you could help me out. The change I made to resolve your issue has had a knock on effect for another user so I’m looking to put a different fix in place. I’m not able to replicate the issue you were having so I can’t test if the new fix works for your case too.

    Would you possibly be able to test it for me? To test it you just need to replace lines 184 and 185 in plugins/all-bootstrap-blocks/class.areoi.styles.php:

    global $page;
$standard_blocks = ( $page === null ? array() : parse_blocks( get_the_content() ) );

    With the below code:

    try{
     $standard_blocks = parse_blocks( get_the_content() );    
} catch (\Exception $ex) {
     $standard_blocks = array();
}

    I’m hoping to get a new release out in the next couple of days so if you could assist with this it would be greatly appreciated!

    Thanks in advance

    Miles

    Thread Starter Butterfingers

    (@cantbelieveitsnotbutter)

    Hi @areoimiles,

    I just tested the change. Unfortunately, it reintroduces the original problem, which is that if $page is null, get_the_content() encounters a fatal error because line 321 in wp-includes/post-template.php doesn’t handle null.

    Our of curiosity, what breaks in the current code? If it’s the global $page, would first checking whether $page is null solve it?

    For example:

    if ( null === $page ) {
	global $page;
	$standard_blocks = ( $page === null ? array() : parse_blocks( get_the_content() ) );
} else {
	try {
		$standard_blocks = parse_blocks( get_the_content() );
	} catch ( \Exception $ex ) {
		$standard_blocks = array();
	}
}

    That example might benefit from some refactoring, but maybe it’s a step towards a solution.

    Thread Starter Butterfingers

    (@cantbelieveitsnotbutter)

    @areoimiles , another thought is that it might be worth submitting a request to the WordPress repo to add null handling to line 321 in wp-includes/post-template.php. This probably wouldn’t be a quick solution or be guaranteed to be accepted, and so an interim solution is still needed, but it could possibly lead to being able to remove the null handling from class.areoi.styles.php.

    Plugin Author Miles

    (@areoimiles)

    Hi @cantbelieveitsnotbutter

    No problem, thank you very much for testing it. I will leave it how it is for now and try and work with the other user to find a different resolution.

    — EDIT —

    Sorry @cantbelieveitsnotbutter, another user has suggested changing it to this:

    $page = get_the_ID();
$standard_blocks = ( $page === false ? array() : parse_blocks( get_the_content() ) );

    Does that work for you?

    Thanks again

    Miles

    • This reply was modified 1 year, 5 months ago by Miles. Reason: Added potential solution
    Thread Starter Butterfingers

    (@cantbelieveitsnotbutter)

    Hi, @areoimiles. Sorry for the delayed response to your last post re: testing the code. The change appears to work fine, so all is well on my end. Thanks very much for checking and for working so well with various users of this plugin!

    Thread Starter Butterfingers

    (@cantbelieveitsnotbutter)

    Hi, @areoimiles. Looks like I spoke too soon. The original problem reported here has returned after the plugin update, which is due to$elements['pages']in functionget_the_contentbeing null.

    However, it appears there’s a solution. The “More Information” section of the docs forget_the_contentspecifies that if the function is used outside of the loop (which it is in this case), that “you must inform the post you want to get the content from using the optional$postparameter”.

    This small change to line 189 in class.areoi.styles addresses this:

    $standard_blocks = ( $page === null ? array() : parse_blocks( get_the_content(null, false, $page) ) );

    Please give this a try. Hopefully this solves it.

    Plugin Author Miles

    (@areoimiles)

    Hi @cantbelieveitsnotbutter,

    I’ve just released an update which includes this change, so hopefully this resolves your issue. Thanks for flagging it and providing a solution. Much appreciated.

    Miles

    Thread Starter Butterfingers

    (@cantbelieveitsnotbutter)

    Hi @areoimiles,

    Thanks very much for this update, and for all your work on this plugin!

Viewing 11 replies - 1 through 11 (of 11 total)
  • The topic ‘AREOI_Styles::add_block_styles() can fail in some non-single contexts’ is closed to new replies.