Category Tree Menu plugin

[WonkoDSane]

Garvin & Co.

Since I'm not much of a PHP guru, I consulted with Judebert on this one to make sure I wasn't out of my depth.

The category tree menu plugin is great, but there've been a lot of posts about problems with it, sometimes people just abandon it altogether. I've found what seems to be a bug (at least on my system) in the way it handles the TreeMenu includes.

What follows is the text from my PM to Judebert:

Found something that might be causing the chokage we keep seeing on various installations of the category tree dhtml menu plugin...

in serendipity_plugin_category_dhtml_menu.php, starting at line 101 (now at line 106):

Code: Select all

        $pear = false;
        if (@include_once('HTML/TreeMenu.php')) {
            $pear = true;
        }
        if (@include_once('HTML_TreeMenu/TreeMenu.php')) {
            $pear = true;
        }
        if ($pear) {
                   rest of plugin code
        }

I find that if the first include works and the second fails $pear ceases to be true and the remainder of the functional code in the plugin is no longer executed.

I changed the second if to an elseif and it appears that the code works properly.

Why is this the case?

I'm not much of a PHP hack, consider myself barely literate, so I thought I'd ask you first rather than posting a bugfix. I was thinking to myself that maybe it was a 'feature'. (-:

Let me know.

I have absolutely no idea why $pear would cease to be true, but based on a couple of echo statements I inserted to test the problem that is exactly what happens. Changing that second if to an elseif eliminates the problem.

What do you folks think?

[garvinhicking]

Hi!

That fix is absolutely correct. I just committed it to version 1.8 of the plugin

Best regards,
Garvin

[judebert]

Hey, Garvin.

I understand and agree that the if should be an elseif. But I'm bewildered as to why the $pear should become "false" when it was previously set to "true". It looks like an optimizer bug to me.

Any comments?

[garvinhicking]

Hi Judebert!

Not sure if I understand correctly...$pear is set to 'false' so that it only can become 'True' if either of the two IFs succeed...?

Best regards,
Garvin

[WonkoDSane]

Garvin,

Yes, initially $pear is set to false, but (as in my post above), if the first include succeeds and sets $pear to true and then the second include fails, then $pear is set to false.

If you insert a couple of echo statements:

Code: Select all

        $pear = false;
        if (@include_once('HTML/TreeMenu.php')) {
            $pear = true;
            echo "First include succeeded";
        }
        if (@include_once('HTML_TreeMenu/TreeMenu.php')) {
            $pear = true;
            echo "Second include succeeded";
        }
        if ($pear) {
                   rest of plugin code
        } 

And your output is only the following because the second include failed:

First include succeeded

Then the rest of the plugin code will not be executed because something about the second include sets $pear to false when the include fails.

So the question is why would this be the case?

[garvinhicking]

Hi!

Yes, but now that elseif() is inserted in the code it should work, no?

Best regards,
Garvin

[WonkoDSane]

It does work, I was just curious why the stacked ifs failed to work. Seems weird that the variable $pear's state can be changed to false in anything other than an explicitly defined manner.

[judebert]

Exactly my question, too. I see how the elseif is more efficient. I understand we don't want to even attempt to include stuff we don't need, so the elseif is even more correct. But the stacked ifs shouldn't be resetting the $pear variable, if I'm reading that code correctly!