?
Referenced as Related to: # 6427 Pull Request for: # 6440
avatar compojoom
compojoom
12 Feb 2015

After talking to Hannes about this one: https://groups.google.com/forum/#!topic/joomla-dev-general/WC0EI0NTiRE I realized that this is a bug.
It can be easily reproduced with com_content. Just go to any view (lets say article view) and on top of the file add:

$category = JCategories::getInstance('Content')->get(2);
$category = JCategories::getInstance('Content')->get()->getChildren(true);
$category = JCategories::getInstance('Content')->get()->getChildren(true);

note: change 2 with a category id that exist in your installation.

this will produce a fatal error like this one:

Fatal error: Call to a member function getChildren() on a non-object in ...\default.php on line 18

if we change the code to:

$category = JCategories::getInstance('Content')->get(2);
$category = JCategories::getInstance('Content')->get()->getChildren(true);

and we var_dump the category. This time we won't get a fatal error, but the array that we get, won't have all children, but it will just contain the category with id 2.

if we change the code to

$category = JCategories::getInstance('Content')->get()->getChildren(true);

then we get the expected array starting from the root node.

If we change the code to:

$category = JCategories::getInstance('Content')->get()->getChildren(true);
$category = JCategories::getInstance('Content')->get()->getChildren(true);

We won't get any fatal error. We'll get the expected array starting from the root.

So it seems that the fatal error only happens when we first try to fetch a particular category. And after that call JCategories::getInstance('Content')->get()->getChildren(true); twice. The second call won't work.

If we try to force the loading of the root node like this:

$category = JCategories::getInstance('Content')->get(2);
$category = JCategories::getInstance('Content')->get('root', true)->getChildren(true);
$category = JCategories::getInstance('Content')->get('root', true)->getChildren(true);

We'll still get the fatal error.

If we do the following modifications to JCategories.
line 179 to 183:

    // If this $id has not been processed yet, execute the _load method
        if ((!isset($this->_nodes[$id]) && !isset($this->_checkedCategories[$id])) || $forceload)
        {
            $this->_load($id, $forceload);
        }

Then we change the _load function to accept the forceload

    protected function _load($id, $forceload)

And then on line 311 we change the code to

// Create the node
                if (!isset($this->_nodes[$result->id]) || $forceload) 
                {

Then the error doesn't occur when we do the forceload. However this is not a solution to the problem in any way. It just points out that something is wrong with the way we build our nodes array.

Upon further investigation of the problem I think that the following is happening.
When we first do:

$category = JCategories::getInstance('Content')->get(2);

The database query returns all results starting from the root. Our nodes array contains root and id 2.
With the next line

$category = JCategories::getInstance('Content')->get()->getChildren(true);

We again have to execute the _load function since the root id hasn't been checked yet. This will return an array with the root element and all other categories. But then getChildren is being called. Since the _allchidlrendsloaded flag hasn't been set. The getChildren function will call get with the forceload set.

We'll again do a db query and fetch the results. This time however in our foreach we will end in the elseif statement where this code will kick in:

if (!isset($this->_nodes[$result->parent_id]))
                    {
                        unset($this->_nodes[$result->id]);
                        continue;
                    }

This will basically remove the root node from the array.
Now the second time we try to do this:

$category = JCategories::getInstance('Content')->get()->getChildren(true);

get won't try to load the results and will just return null, because the root node is not set, but _checkedCategories has been set... So it doesn't make sense to go and search for this element.

So, any ideas on how to actually properly fix this? Currently i'm thinking of changing line 355 to:

if (!isset($this->_nodes[$result->parent_id]) && $result->id != 'root')
                    {
                        unset($this->_nodes[$result->id]);
                        continue;
                    }

This way if we are dealing with the root element we won't remove it from the array, but I'm not sure if this the correct thing to do here. Anyone?

avatar compojoom compojoom - open - 12 Feb 2015
avatar joomla-cms-bot joomla-cms-bot - change - 12 Feb 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 12 Feb 2015
Category Libraries
avatar Erftralle
Erftralle - comment - 24 Feb 2015

I walked through your instructions to reproduce the bug and I can confirm it.
Issue #5530 seems also to deal with this error.

I would like to point out once again that, when calling

$category = JCategories::getInstance('Content')->get(2);
$category = JCategories::getInstance('Content')->get()->getChildren(true);

, the second call does not deliver the correct children, so the next call of

$category = JCategories::getInstance('Content')->get()->getChildren(true);

throwing the fatal error is just a subsequent error.
The reason for the incorrect deliverance of the children is, like @compojoom already said, that the root node is removed from the node list.

I also can confirm that his proposal for a fix works for me. Maybe it is better to code it in the same way it is done in lines 321-327 (just to keep things uniform)

                    // If the node's parent id is not in the _nodes list and the node is not root (doesn't have parent_id == 0),
                    // then remove the node from the list
                    if (!(isset($this->_nodes[$result->parent_id]) || $result->parent_id == 0))
                    {
                        unset($this->_nodes[$result->id]);
                        continue;
                    }

But I admit, I'm not 100% sure that the bug fix does not have any other side effects.
Maybe the author of the source code could check this as well.
However, the bug is in my opinion a serious one.

avatar brianteeman brianteeman - change - 25 Feb 2015
Labels Removed: ?
avatar brianteeman brianteeman - change - 25 Feb 2015
Labels Added: ?
avatar compojoom compojoom - reference | - 14 Mar 15
avatar compojoom compojoom - reference | - 15 Mar 15
avatar compojoom compojoom - reference | - 15 Mar 15
avatar zero-24 zero-24 - change - 15 Mar 2015
Status New Closed
Closed_Date 0000-00-00 00:00:00 2015-03-15 15:10:22
Closed_By zero-24
avatar joomla-cms-bot joomla-cms-bot - change - 15 Mar 2015
Status New Closed
Closed_Date 0000-00-00 00:00:00 2015-03-15 15:10:22
avatar joomla-cms-bot joomla-cms-bot - close - 15 Mar 2015
avatar joomla-cms-bot joomla-cms-bot - close - 15 Mar 2015
avatar zero-24
zero-24 - comment - 15 Mar 2015

Thanks for the PR http://issues.joomla.org/tracker/joomla-cms/6440 @compojoom So I'm closing here and adding the infos there.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6064.
avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Mar 2015

Set to "closed" on behalf of @zero-24 by The JTracker Application at issues.joomla.org/joomla-cms/6064

Add a Comment

Login with GitHub to post a comment