User tests: Successful: Unsuccessful:
Since nobody else wants to make a pull request for this, I'm adding this code for review here.
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?
Labels |
Added:
?
|
Labels |
Added:
?
|
Category | ⇒ | Libraries |
Title |
|
||||||
Rel_Number | ⇒ | 6064 | |||||
Relation Type | ⇒ | Pull Request for |
Title |
|
Status | Pending | ⇒ | Ready to Commit |
Thanks here! Moving this to RTC.
Labels |
Added:
?
|
Labels |
Added:
?
|
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-04-13 15:06:00 |
Milestone |
Added: |
Labels |
Removed:
?
|
@test: patch tested succesfully.