Wrong active trail with multiple menu items with the same path

The "deepest, heaviest menu link which matches the deepest trail path url" should be used, not just the "first menu link". Is the intended behaviour broken?

This is the behaviour since the d7-version, changing this would break backward compatibility and thus requiring a new major release.

Can you convince me why this should be changed, provided with some examples?

@davy-r I can report special use case, for example if you use the submenu links as a mega menu, then every link get the active trail. I think this is meant in the issue above.

I also noticed this issue, but I can only reproduce it with a deep level menu.

To reproduce:
- create a menu structure, where root items have different paths (/branch1/... and /branch2/...). Example attached.
- add a node ("test"), and assign it to one of the branches (branch 1_1, as attached), using the node form
- add a menu link ("test fake") to the other branch (branch 2_1, as attached) with a path to the previously created node

When accessing the node, expected active trail is:
root > branch1 > branch 1_1 > test
but following the steps above I get:
root > branch1 > branch 1_2 > branch 2_1 > test fake

I've modified the previous patch to simplify it a bit and to make it work with other modules, since it is limited to "menu_link_content" entities and it won't work with "menu_link" module.

The core solution let us to save a menu item adding an existing alias manually in the Link field (instead of using autocomplete).

The following solution extends the logic in MenuTrailByPathActiveTrail::getBestMatch() to check this case and return the menu item if the entered alias is equal with the request url.

Adding the correct patch + interdiff

@akozma I'm getting this error with your patch

Recoverable fatal error: Argument 2 passed to Drupal\menu_trail_by_path\MenuTrailByPathActiveTrail::getBestMatch() must be an instance of Drupal\menu_trail_by_path\string, string given

@rutiolma there is an issue with your patch, when $match->getParent() return an empty value , MenuLinkManager throw an exception

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Drupal\Component\Plugin\Exception\PluginNotFoundException</em>: Plugin ID &#039;&#039; was not found. in <em class="placeholder">Drupal\Core\Menu\MenuLinkManager-&gt;getDefinition()</em> (line <em class="placeholder">207</em> of <em class="placeholder">core/lib/Drupal/Core/Menu/MenuLinkManager.php</em>).

Updating patch to handle the error reported in #12. I am not seeing the error in #11 to be able to offer much there.

These aren't really different from those in 13, but the naming conventions are more correct and stray characters in the innerdiff fixed.

I'm uploading a new patch with a check for the loaded menu_link_content object. In order to provide compatibility with "menu_link" module (and others?), this check is necessary.

This patch also fixes some coding standards and replaces an old service call with a dependency injection.

Tests still needed.

Forgot to upload the interdiff for the the previous patch.

Patch doesn't apply to 8.x-1.x.

Indeed my last patch was done against another version.
Uploading the correct patch.

I have the same issue as in #6 – when there's more than one menu link for a node, this module selects the deepest menu link, whereas core selects the one with the smallest ID.
What confuses me is the fact that the issue appears with nodes that do have menu links, while the main purpose of this module is to set a menu tail for nodes that don't – or at least that's my understanding of it.
Shouldn't we handle nodes with menu links exactly as in core, and only provide a menu trail if there is no menu link?

This is clearly not BC, so attached patch is against 2.x-dev.

Same Issue, any news on this?

#20 did the trick on drupal 9.4.11. Thank you!

Patches in #19 and #20 do not pursue the same goal.

I'm re-rolling patch #19 for version 2.x-dev.

Previous patch was missing a use statement, don't use #24.

I also just had a case in a project where the current behavior was unexpected. #20 wouldn't work for me, because it wasn't a direct link, but part of the trait. In my case, the deeper menu link wasn't actually want I wanted, it was the one higher up that was in a completely different section of the site. The deeper one was actually a cross-reference from a different section of the website.

So what would work for me is just removing the array_reverse(), which basically makes the behavior consistent with core. But that seems rather arbitrary either way. In my case, the deeper link also had extra query arguments. The logic in #19 and rerolls also seems rather arbitrary, which might or might not be what you expect.

There's #2823791: Allow other modules to alter the active link trail as well, if you want to apply extra rules on matching. For this to be committed in a BC way, the behavior would probably need to be configurable and especially if it gets as complex as latest patches, requires pretty extensive test coverage to cover all those extra checks.

I re-rolled patch #25 for version 2.2.0.