-
Epic
-
Resolution: Unresolved
-
Major
-
None
-
4.4.3
-
None
-
MOODLE_404_STABLE
-
Navigation API Issues
-
MDL-83100-main
Background
In order to understand the issues presented here, a high level understanding of the new navigation API introduced in 4.0 is required. The changes unfortunately aren't well documented so I'll do my best to summarise what the original intention of the navigation project was (this understanding was compiled from investigation by myself and trishamilan by digging thgough the codebase, combing the Moodle tracker, and personally asking key individuals from the project).
Navigation in Moodle
Moodle (code) contains a concept called a "navigation node", these nodes can have children (which are themselves navigation nodes) and they form trees. This has been around for ages and the way the trees are constructed is pretty fragmented. Various places in code just create nodes, and add children to them. Over time these became the navigation trees everyone knows and loves from older Moodle versions (Moodle <4.0). Two specific trees of interest (we will discuss them in subsequent sections) are the "global navigation" tree, and the "settings navigation tree". These trees are accessed via the global $PAGE object via $PAGE->navigation and $PAGE->settingsnav, respectively.
Navigation overhaul
In Moodle 4.0 there was a big push to redo navigation for a better UX. However, this was far from trivial because of the way the existing navigation trees were constructed. It was not practical to go through millions of lines of code to shift around the nodes manually. Not to mention doing so would cause breaking changes to any plugins that expect the navigation trees to be structured a specific way (as they have been forever).
Another key difference between the pre 4.0 navigation is the notion of an active navigation node. In Moodle 3.9 and earlier, the way the navigation trees are displayed to the user is very reflective of their actual structure; i.e., they are displayed as trees with expandable/collapsible nodes. In the old paradigm, to make it as clear as possible to the user which page they are on, one, and only one node should be active per tree; the parents of the active node should be inactive, and there is code built in to the navigation nodes themselves to do this. The new paradigm from 4.0 onwards is different in that the navigation trees are presented as as series of menus, each which "drop down" to display their child nodes when clicked. In this paradigm, it is important for both the active node and its parents to be active, as you cannot always see the active node itself.
Navigation views
Overview
The solution to the above was to introduce a concept known as a "view". Views themselves are navigation nodes (which ultimately become the new tree structures used by the boost theme), but they are embellished with extra abilities. The abstract base class, view (which other views must extend) defines the following contract:
- An initialise method which will be called by the global PAGE object to initialise the view. There is no default implementation.
- A default constructor which accepts a moodle_page object, and initialises a child node collection.
- A protected method to scan a given source navigation node for specific given node keys with an associated sort order. An array of nodes ordered by the specified sort order is returned
- A protected method to reactivate nodes to suit the new 4.0 paradigm
The navigation overhaul adds two important views to the global page object. These are primarynav and secondarynav. The primarynav view is used for the "top" level navigation in boost (things like "Home", "Dashboard", "Site administration") and secondarynav is what you see when looking at a course ("Course", "Settings", "Participants", etc) or Site administration ("General", "Users", "Course", etc). The secondarynav view in particular is special as it is responsible for creating an entirely new tree out of the existing settings navigation and global navigation trees. In particular it has a method which makes use of the ordered nodes from the base class (dot point 3 above) and add them to itself. The primarynav tree is not really like that and just creates nodes to add to itself directly; and it doesn't seem to cause any dramas.
These new views are accessible via $PAGE->primarynav and $PAGE->secondarynav. When boost is the active theme, generally these will be called from the various theme/boost/layout/ files except for a few rare cases (which also leads to some obscure bugs, expanded on in issues in this epic). When using classic, $PAGE->secondarynav is never called and the old $PAGE->navigation and $PAGE->settingsnav trees are used instead.
In addition to the view base class having the ability to reactivate the nodes appropriately, the page API was given a few new abilities:
- An "active tab" can be specified by calling $PAGE->set_secondary_active_tab('navigation node key here') that will "force" the view class to activate that specific node.
- Calling code can replace core's implementation of secondary navigation with their own, by calling $PAGE->set_secondarynav($someview).
Problems
Unfortunately, the implementation of views does not adhere to the requirement of not modifying existing navigation trees, and the navigation views do reorder the existing structures (which will be demonstrated in the issues in this epic). Additionally, the implementation of the method for ensuring nodes are activated/deactivated appropriately as per the paradigm mentioned above does not cover all cases sufficiently. These two issues lead to subtle and confusing bugs which are elaborated on in the issues of this epic.
Full flow
Here is a (high level and technical) summary of how the nodes are created, reordered, and eventually displayed to users. Note: to make it clearer which implementations of methods are actually being used, I will use notation like view::somemethod - this means that the code has actually called $this->somemethod() but it will use the impementation from view because we are in a class that extends view and did not overwrite that method anywhere):
- The class moodle_page in lib/pagelib.php is the entry point where navigation trees are built. It uses magic getters so that when calling code calls $PAGE->primarynav, $PAGE->secondarynav, $PAGE->navigation, or $PAGE->settigsnav the trees will be populated. Usually (but not always) the place that kicks it all of is a call to $PAGE->secondarynav in theme_boost (by way of a call to $OUTPUT->header() which would be in the specific PHP file navigated too in the browser, as it creates the page). These magic getters are responsible for calling the initialise method of navigation views
- The specific implementation of the secondary navigation view depends on where we are in the site, e.g., in module context it's possible to get a module specific implementation, but (at least in core) these module specific implementations all extend core's one and none of them override much of anything except for the methods used to specify node orderings, so the specific implementation of initialise is always the one from core's secondary navigation view
- Depending on the current page context, different things happen in the initialise method, however $PAGE->settingsnav is always ultimately called first, and this means global navigation gets loaded first because settings navigation will initialise it. Course context has an explicit call to $PAGE->navigation but at that point it's already been loaded
- Once $PAGE->navigation and $PAGE->settingsnav are loaded, in course, module, and category context, core's secondary navigation view implementation will call either $this->get_default_module_mapping or $this->get_default_course_mapping - these methods are interesting as the intention is for module specific implementations of secondary navigation (as per 2.) to override them, and thus be able to specify their own ordering for secondary navigation. The orderings simply associate a node key with a number; normally the number is an integer like 1, 2, 3 etc which can be use to sort the nodes, however sometimes the number is like 2.1, 2.2. In this case what's supposed to happen is that we (in this specific example) add the node to whatever node has the integer part as its sort order (2 in this case) and the order of the children should respect the decimal part (i.e., a node specified as 2.1 should be the first child of 2, 2.2 should be the second, etc)
- These orderings are then passed to view::get_leaf_nodes along with a node to search, and get_leaf_nodes will scan the given node, returning an array where the keys are the locations as specified in 5. and the values are the nodes.
- secondary::add_ordered_nodes is then passed the array from 7, and given a "root node" (if not provided, it uses itself) to add the ordered nodes to. It handle the logic of ensuring that the nodes are added to the correct parent. This should ultimately result in an entirely new navigation tree, independent from the original tree(s) which will be used as secondary navigation
- The last thing the initialise method does is call view::scan_for_active node - which is responsible for activating the nodes appropriately (as explained in the navigation overhaul section above).
Notes on the above
- Re: 2. It does not check for $this->activityname == null; this doesn't seem cause problems but it probably should be checked
- Re: 2. It is possible after the fact, to inject your own secondary navigation view class, but it won't be initialised; instead calling code has to initialise it, an example of this in lib/blocklib.php where the calling code has to initialise the view - why can't the page API just handle the initialisation as it does for core's default secondary navigation view implementation?
- Re: 6. The name get_leaf_nodes is a bit of a misnomer as the way it works is it takes an array which associates a node key with a "sort order", it calls get_default_course_mapping and it just uses those keys to do a ->find on the source node (also passed as an argument). It can and does return nodes that also have children
- Re: 7. It is odd that the method to add the ordered nodes lives in secondary and the method to get the ordered nodes lives in view - surely both can live in view
Supporting utilities
Direct links
- https://github.com/cameron1729/moodle/tree/nav-debug
- https://github.com/cameron1729/navigation-node-player
Explanation
Debugging the state of navigation trees in Moodle is extremely challenging. Even with a debugger, it's almost impossible to track what's going on, because sometimes there will be two navigation nodes that appear identical when inspected by a debugger, but are actually distinct. And a debugger doesn't allow you to easily visualise the whole tree.
To aid in debugging, we developed a quick and dirty debugging mechanism in the navigation API itself which is available here (it's just a branch off of latest main (at the time of writing)) and we have added debugging at key points in the navigation API lifecycle. Each debugging call dumps the state of the navigation trees to a text file which effectively produces a "movie" of the way the tree evolves. We developed a separate tool to "play" these "frames" which is a available here.
Specific instructions for how to use the player are contained in the README of the Github repository. In various issues in this epic, there may be a zip file attached which contains a debugging run of the navigation trees on a specific Moodle page and the issue description will make mention of it. You can use the player to "play" those files and follow along with the issue description.
Specific Issues
Refer to the MDLs in this epic.