Bug Description

There is an observed bug in the moremenu functionality (singleactivity course format) of Moodle where checkmarks indicating the current menu item are displayed without consistency.

This issue arises specifically when using the back and forward buttons in the browser, likely due to a back-forward cache problem. As a result, two or more menu elements from Course/Activity can be marked as current simultaneously, which represents a significant accessibility issue.

Reproduction steps

Scenario 1

1. Navigate to a course in Moodle (single activity format).
2. Click on Activity and select a menu item (e.g., "Quiz", "Questions").
3. Click now on the Course menu and select settings.
4. Use the browser's back button to return to the previous page.
5. Observe there are two checkmarks under Course and Activity set of menus.

Scenario 2

1. Navigate to a course in Moodle (single activity format).
2. Click on Activity and select a menu item (e.g., "Settings").
3. Use the browser's back button to return to the previous page.
4. Observe there are two checkmarks under Activity.

Expected Behavior: Only one menu element should be marked as current, clearly indicating the user's current location within the site.

Actual Behavior: Two or more menu elements are marked as current, causing confusion for users, especially those using screen readers or other assistive technologies. This inconsistency can mislead users about their current location and disrupt their navigation experience.

Impact: This bug creates a significant accessibility issue as it violates the ARIA specification for aria-current and can cause confusion for users relying on visual cues and assistive technologies. Ensuring that only one menu element is marked as current is crucial for maintaining an accessible and user-friendly interface.

Analysis

It appears that the root cause of this issue is that the checkmarks are added immediately when clicking on a menu item (via onclick). This behaviour has caused issues at least once in the past: MDL-73355. However, what MDL-73355 failed to identify is that the exact same problem happens not only with links that open new windows, but any link which is opened in a new tab (i.e., the checkmark is added on the current page while the new page is opened in a separate tab).

The checkmark being added immediately in this way causes problems with the back-forward cache, as when going back to the previous page, if it's been cached, you'll see that erroneously added checkmark.

It appears that the code which drives the more menu is perhaps striving to be too generic and handle a case which it doesn't actually need to (namely dynamically adding checkmarks). And then this generic solution was applied to the navigation menus without considering that it isn't necessary to add checkmarks when clicking a navigation item (as at the specific moment you click it, you have not navigated away from the current page). The checkmarks should only be added on page load, via data from the back end.

Additionally, the code that adds checkmarks is supposed to remove the previously active checkmark but it doesn't always work, resulting in two checkmarks. Indeed MDL-77732 attempted to fix this - however the solution integrated there was more of a band-aid than a true fix (it uses JavaScript to remove the erronously added checkmarks rather than preventing them from being added in the first place).

Further cases of "double checkmarks" have been idenfitied due to inconsistent markup generated by the moremenu_children template. Sometimes menu items are in unordered lists, and sometimes they are not. However the previously mentioned JavaScript can only handle the case where the menuitems are children of the individual nodes of a menu. Not when the menuitems are the nodes of a menu. This is a separate issue and should be addressed in a separate MDL. However by not adding the checkmarks on click we effectively solve it because the second checkmark will never be added.

Solution

The solution should simply be to not add the checkmarks on click. The menuItemHelper from the menu_navigation module already has code to do this so we just need to update the template to add disableactive for all menu nodes. Additionally the code added in MDL-77732 should be removed.