CMS MADE SIMPLE FORGE

CMS Made Simple Core

 
Back Back to List

[#12620] Version 2 breaks MenuManager with childrenof

avatar
Created By: R Hamster (Hammy)
Date Submitted: Tue May 30 11:42:57 -0400 2023

Assigned To:
Version: 2.2.16
CMSMS Version: 2.2.16
Severity: None
Resolution: Won't Fix
State: Open
Summary:
Version 2 breaks MenuManager with childrenof
Detailed Description:
Sorry if this has been covered before - I searched but couldn't see anything
relevant.

I've just upgraded from 1.12.2 to 2.2.16 which auto-updated fine.

On displaying my site the left nav menu is not displaying correctly. This is
called with
     {menu loadprops=0 template='custom_left_menu' childrenof=$page_alias}  

custom_left_menu uses $node->depth and $node->prevdepth to control the level
indentation. (It's a slightly modified version of the old "high_tour // ht" menu
template.)

After some digging it appears the problem is that $node->depth is being
calculated incorrectly. Instead of the expected value of '1' for the first item
in the childrenof list, MM is now returning '-1'.

You can see the issue if you print out $mnode after the FillNode() call on line
163 in MM::action.default.php.



This used to work in 1.12.2 but is broken in 2.2.16

Digging around in modules/MenuManager/action.default.php it seems that the
'depth' is calculated in FillNode() using the origdepth
$onenode->depth = count(explode('.', $content->Hierarchy())) - ($origdepth
- 1);

$origdepth is set in action.default.php line 159.
     $prevdepth = $origdepth = $onenode->GetLevel() + 1;  
On first pass this is calculating $prevdepth and $origdepth as 4 !

So it appears the issue is with lib/classes/get_level() which walks the tree
counting parents. For some reason, with childrenof, it is calculating too many,
so the subsequent calculation of depth (from prevdepth and origdepth) is wrong.

Has something changed in the structure of the node tree between 1.12 and 2.2
such that 'parent' counting is now failing?

Printing 'all levels' works fine - I'm only having problems with childrenof

 

Replacing L159 in action.default.php
     $prevdepth = $origdepth = $onenode->GetLevel() + 1;  
with 
$prevdepth = $origdepth =  count(explode('.', $onenode->getHierarchy()));

Fixes the problem (i.e. childrenof $node->depth is then correctly set to 1) and
my menu displays correctly. But I know not if that works in all cases (I only
tested 'childrenof').



I know you will suggest changing to use Navigator instead of MenuManager but
Navigator doesn't create $node->depth at all, and I'm not ready yet to change
all my templates just to use Navigator instead of MenuManager (by which I mean
there is no benefit if the site will look and perform just the same as now).

Thanks.


History

Comments
avatar 
Date: 2023-05-30 14:15
Posted By: Fernando Morgado (JoMorg)

Sorry but MenuManger has been deprecated a long time ago. There are alternative
ways to determine indentation of sub items in menus menus without the need of
$node->depth and $node->prevdepth. It can even be determined by using recursive
calls to smarty {function ...} calls and a bit of clever smarty logic. There are
a few sample Navigator templates that might give some clues about how to achieve
it.
Thanks
avatar 
Date: 2023-05-31 07:49
Posted By: R Hamster (Hammy)

Many thanks for your reply, and for your suggestions. I was hoping to just
install 2.2 without having to make any template changes.

Unfortunately, I don't have time at the moment to rewrite my (several) menu
templates to use Navigator's hierarchical node structure, so I guess I'll have
to stick with v1.12.
avatar 
Date: 2023-06-01 05:08
Posted By: R Hamster (Hammy)

Ha ha...the issue was hidden in plain sight.

In v2.2.16 lib/classes/class.cms_tree.php get_level() says

    /**
     * Find the depth of the current node.
     *
* This method counts all of the parents in the tree until there are no more
parents.
     *
     * @return int
     */
    public function get_level()
    {
  	$n = 1;
        $node = $this;
        while( $node->_parent ) {
            $n++;
            $node = $node->_parent;
         }
         return $n;
    }


whereas in v1.12.2 get_level() says

    [...]
    public function get_level()
    {
        $n = -1;
        $node = $this;
        while( $node->_parent )
            {
            $n++;
            $node = $node->_parent;
            }
        return $n;
    }


You will note the different starting point for $n!

$n = -1; works
$n = 1; does not work (calculates $n as 2 too many)

Inspection of the cms_content_tree object verifies it should start from -1  (to
skip over the first two _parent nodes before the root)


Changing get_level() to $n=-1 my v1.12 MM menus now work in v2.2 MM without any
template changes. hurrah!

Why was $n changed from -1 to +1 in version 2?    Rhetorical question - I don't
have svn log :-)
Updates

Updated: 2023-05-30 14:15
resolution_id: => 8
severity_id: 2 => 12