Wrong breadcrumb due to route alterations

Here is a patch.

#2 patch work fine on Drupal Core 9.5.11.

xurizaemon → credited Nasreddine123 → .

xurizaemon → credited nicrodgers → .

I pulled @nicrodgers sort before comparison of the route and raw parameters over from 🐛 Current page added twice due to route alterations Closed: duplicate .

xurizaemon → credited annoul4 → .

xurizaemon → credited joaopauloscho → .

xurizaemon → credited linhnm → .

xurizaemon → credited mike.vindicate → .

xurizaemon → credited webflo → .

Over in [#] there's a suggestion from @webflo @joaopauloscho to use !array_diff_assoc($routeParams, $rawParams) instead of $routeParams == $rawParams.

$routeParams == $rawParams ensures the keys and values match and doesn't care about order.

!array_diff_assoc($routeParams, $rawParams) ensures that all entries in $routeParams are matched in $rawParams (and allows for additional $rawParams entries).

php > $routeParams = ['b' => 'b', 'a' => 'a'];
php > $rawParams = ['a' => 'a', 'b' => 'b'];

php > var_dump($routeParams == $rawParams);
bool(true)

php > var_dump(!array_diff_assoc($routeParams, $rawParams));
bool(true)

php > $rawParams['c'] = 'c';
# All values in routeParams exist in rawParams.
php > var_dump(!array_diff_assoc($routeParams, $rawParams));
bool(true)
# Some values in rawParams exist in routeParams.
php > var_dump(!array_diff_assoc($rawParams, $routeParams));
bool(false)

Which comparison is more correct here?

Seems this can be repro'd as easily as taxonomy/term/1 - debug here from \Drupal\menu_breadcrumb\MenuBasedBreadcrumbBuilder::addMissingCurrentPage() shows that route and raw parameters may be sorted differently. Added test \Drupal\Tests\menu_breadcrumb\Functional\MenuBreadcrumbTest to cover this.

xurizaemon → closed merge request !17

how can I test? I have the same problem

@zenimagine, here are docs for patching modules when using composer →

Should I apply patch #2 ?

Moving this back from RTBC since there are changes to be reviewed!

@zenimagine, on this issue a Merge Request ("MR") has superseded the patch submitted in #2. So review should focus on the changes available from the MR. On the MR URL, you'll see tabs for eg "Commits" (the initial change from #2, and later changes) and "Changes" (the current proposed changes from this MR).

The required patch is linked on this issue (text "plain diff" near MR !15), which is the MR URL with .diff appended.

That file can be saved and used as a patch in your local codebase, or (quicker but not recommended for production builds) referenced directly in your composer patches configuration.

azovsky → changed the visibility of the branch 3392139-wrong-breadcrumb-due to hidden.

azovsky → changed the visibility of the branch 3392139-wrong-breadcrumb-due to active.