Fixing The Asides

Hey, did you notice the “aside” formatting is working again on that last post?

You might remember my saying, way back when, that I had noticed that the upgrade to WP 2.8 had broken AsideShop, the plugin I used to do styling for my “aside” posts (Oh yeah, this is going to be some deep WP nerdery–run away!) and I was waiting for the power of open source to fix it.

Well, I got tired of waiting–the bug has been known for over three months now, and no action.

Now that I have a few minutes of blog time again–or at least I’m forcing there to be some–I figured the appropriate thing to do was just fix the problem. That’s the other side of open source–when waiting for someone else to do it doesn’t work, you have no excuse for not getting in there are doing it yourself.

Of course, I’m no PHP guru, nor WordPress wiz, but I do kind of design and implement radically more complex stuff every day, so how hard could it be, right? Well, two hours of hair-pulling later, I think I have the problem sorted out. At least it all seems to be working now. And, of course, once I figured out what the code was trying to do, and why it wanted to do that, it the fix turned out to be terribly, terribly simple. If you just want the fix, scroll down. Or keep reading for some explanation.

The problem essentially appears to be a classic case of “I used to have to do something ‘wrong’ to get the right behaviour, but the underlying code has since been fixed so there is a right way to do it and the wrong old thing I used to have to do is now broken”. This is something that comes up all the time in the lifecycle of code built on top of underlying infrastructures, when the underlying code is undergoing constant refactoring.

In this case, the real issue comes down to how Asideshop gets the chance to intercept post formatting and then do it’s own thing with the posts that are flagged as asides.

What AsideShop wants to do is hook into WordPress’ “loop_start” action, so that when the loop over the posts to be displayed is starting the code in the plugin can replace the query object (which essentially drives identifying the posts to display, and getting them sent to the screen) with the plugin’s own extended version of the query object. It really only wants to do this so that it can override the “the_post” method on the query object, so that whenever that method is called, the extended query object gets a chance to first do the normal work, and then add some of its own code after to do the aside processing.

The problem is that at some point along the way–and since this all broke in 2.8 that might be a good guess as to when–it turned out that the handling for plugin code that runs on the “loop_start” action started actually being triggered from the “the_post” method of the query object. So obviously trying to replace the query object from there was too late, and trying to do anything in there that might call “the_post” on the query object was just asking for an infinite loop–which, of course, is what we were seeing.

So, the code is trying to hook into “loop_start” so it can override the query, so it can extend “the_post” with extra behaviour at the end, but that just won’t work anymore.

Of course, this begs the question: why not just hook into an action that runs at the end of “the_post” and do extra processing that you want to do at the end of “the_post” there? Skip all that loop_start and “replacing the query object” stuff–which was pretty clearly just there to allow “the_post” to be extended anyway–and go directly to extending “the post”.

So I did that. And it seems to work.

Why wasn’t the code already like that? I certainly don’t know. Like I said, I’m no WP Zen master, and I don’t know the history of the WP code itself. It’s entirely possible that there didn’t use to be a hook on “the_post”, so you had to do the query replace thing. It’s possible that I’m missing a use case that won’t work with this solution–although I don’t think so. The point is that this change makes sense to me, and it makes the code work.

So what’s the change?

Inside asideshop.php, take the code that looks like this:

// Initiate upon The Loop start
add_action('loop_start', 'asideshop_loop_start');

And replace it with code that looks like this:

// Hook up the_post processing
add_action('the_post', 'asideshop_the_post');

That’s actually enough to make everything work, as far as I can tell. Essentially we’re skipping any attempt to hook into loop_start and just going directly to the_post.

Now, for the more conservative types, I should mention that there’s code in the plugin that manipulates output buffers–essentially to capture the normal output from posts that are being “asided” and throw it away. While things will actually work fine with just the change above, the comments in the code indicate that we need to be careful about output buffer usage to prevent other plugins from breaking with AsideShop. I’ll come clean here–I don’t know enough about that to safely disregard that comment. I mean, my site, with the plugins I have now, seems to work with just the above change, but let’s clean things up a bit, just in case there’s something to that.

In order to do this, we’ll leave the loop_start hook in place, but only to do the output buffer stuff. So the action list now adds the the_post handler, rather than replacing the loop_start handler with it. So from this:

// Initiate upon The Loop start
add_action('loop_start', 'asideshop_loop_start');

// Initiate upon The Loop end
add_action('loop_end', 'asideshop_loop_end');

to this:

// Hook up the_post processing
add_action('the_post', 'asideshop_the_post');

// Initiate upon The Loop start
add_action('loop_start', 'asideshop_loop_start');

// Initiate upon The Loop end
add_action('loop_end', 'asideshop_loop_end');

And then we need to make the asideship_loop_start method into something that just does the buffer management, so we go from this:

function asideshop_loop_start()
{
	if (asideshop_is_enabled() && !is_admin() && !is_feed() && asideshop_is_displayable()) {
		global $wp_query, $asideshop_ob_started;
		
		ob_start();
		
		$asideshop_ob_started++;
		
		$wp_query = new WP_Query_AsideShop($wp_query);

		asideshop_the_post();
	}
}

to this:

function asideshop_loop_start()
{
	if (asideshop_is_enabled() && !is_admin() && !is_feed() && asideshop_is_displayable()) {
		global $asideshop_ob_started;
		
		ob_start();
		
		$asideshop_ob_started++;
	}
}

And, if you want to be really neat, you can entirely remove the WP_Query_AsideShop class, since nothing is calling it anymore.

Now everything will work like you expect, except that the code will also try to enforce the aside formatting on the single post pages–you probably don’t want this–I certainly don’t. On the main page, in search results, and in archives, sure, but on the single post page (i.e. “permalink page”) I don’t–if for no other reason than that I want my “next post” and “previous post” navigation links to show up on the single post page, and they aren’t part of the aside template formatting.

So, a quick pop into the source lets me change:

function asideshop_the_post()
{
	if (asideshop_is_enabled() && !is_admin() && !is_feed()) {

to:

function asideshop_the_post()
{
	if (asideshop_is_enabled() && !is_admin() && !is_feed()&&!is_single()) {

and no more trying to aside-up the single post pages.

And there you go: no guarantees, no warranties, but it seems to be working for me. And maybe now if someone else Googles up the problem, this might help them.

Update: If you want a patched version of the php file, rather than messing with it yourself, you can get one here. Download it, rename it with a .php extension, and drop it in overtop of the broken version and you should be good.