- Issue created by @jensschuppe
 - πΊπΈUnited States totten
Yeah, that `Route` metadata seems to go through several copy/filter steps, and I don't know the history on all of them, but you only need one of them to be more selective.
TitleResolver should check for route parameters being a scalar before copying them as translation arguments:
+1 - Sounds sensible to me. That seems like the place where you go from a broader domain (all possible arguments to a page-controller) to a narrower domain (values that can be interpolated into a string).
 - First commit to issue fork.
 - Open on Drupal.org βEnvironment: PHP 8.1 & MySQL 5.7last update
over 2 years ago Not currently mergeable. - @lisotton opened merge request.
 - last update
over 2 years ago 30,323 pass, 1 fail - last update
over 2 years ago 30,323 pass, 1 fail - last update
over 2 years ago 30,330 pass - π§πͺBelgium lisotton Brussels
I created a MR applying what was suggested by @jensschuppe.
The problem I see is that using the function "is_scalar", it will not enter even when the value is null and before if the value was null, it was using an empty string.
What do you think is the best option?
- Not set the value if is null or array
- Leave the value as empty string if is null and not set the value if is array
- Set the value as empty string if both null or array - Status changed to Needs review
over 2 years ago 8:07pm 8 May 2023 - π©πͺGermany jensschuppe
Thanks @lisotton for your MR! I'd opt for keeping current behavior when the value is
null, i. e. set it to the empty string. I'm not sure about non-scalar values though. If the empty string was meant to be a fallback, I'd say it should be the default value for every non-scalar value. If it was meant to be the string representation ofnull, non-scalar values should have their own defaults or be ignored. But maybe this should be answered by someone more familiar. Historically, only scalar values ornullwould have been "allowed" anyway ... - πΊπΈUnited States wells Seattle, WA
I'm still trying to track down how it is related but I also have a regression related to π copyRawVariables should support default route parameters Fixed -- in my case I've got a view with a path and menu entry and the renders menu item no longer sets
in_active_trailtoTRUEwhen accessing the view on a page with the menu. Reverting π copyRawVariables should support default route parameters Fixed resolves the issue. Will try to see if I can create clean reproduction steps as well... - πΊπΈUnited States wells Seattle, WA
I am able to reproduce the issue from #8 with the following steps on a fresh install of Drupal 9.5.x (using
drush siin my case):- Log in as admin.
 - Navigate to 
/admin/structure/views/add. - Add a view with options:
- View name: Site content
 - Create a page: checked
 - Create a menu link: checked
 - Menu: Main navigation
 
 - Navigate to 
/site-content. - Inspect "Site content" main navigation item with browser tools and confirm class 
primary-nav__menu-link--active-trailis not present onaelement. - Revert changes from 
            
              
              
              π
              copyRawVariables should support default route parameters
                Fixed
              
             (or just comment out 
core/lib/Drupal/Core/Routing/Enhancer/ParamConversionEnhancer.php#L71-75). - Rebuild caches.
 - Navigate to 
/site-content. - Inspect "Site content" main navigation item with browser tools and confirm class 
primary-nav__menu-link--active-trailis present onaelement. 
Still not sure if this belongs here or in a separate issue. Happy to create a new issue if it anyone thinks it should be separate.
 - π¨πSwitzerland znerol
In my case I've got a view with a path and menu entry and the rendered menu item no longer sets
in_active_trailtoTRUEwhen accessing the view on a page with the menu.Observed exactly the same issue today with a custom menu/route after rolling out a recent build. Glad that you found a repro relying on core only. I ended up reverting π copyRawVariables should support default route parameters Fixed to work around the issue for the moment.
 - π¨πSwitzerland znerol
I think that
$this->routeMatech->getRawParameters()->all()returns a different set of key-value pairs which in turn leads to an empty result from$this->menuLinkManager->loadLinksByroute()in MenuActiveTrail::getActiveLink():public function getActiveLink($menu_name = NULL) { [...] $route_parameters = $this->routeMatch->getRawParameters()->all(); // Load links matching this route. $links = $this->menuLinkManager->loadLinksByRoute($route_name, $route_parameters, $menu_name); // Select the first matching link. if ($links) { $found = reset($links); } [...] } - π§πͺBelgium lisotton Brussels
I had similar issues in many places where I manipulate retrieve the menu tree.
I have some routes that share the same Controller and Action, but in the route declaration it declares default parameters. For example I have the route
mymodule.news_list(/news) which have the controllerListPageController::entryPointwith a default parameter called type with valuenews-list. Then I have another route calledmymodule.articles_list(/articles) which shares the same controller, but the default parameter type with valuearticles-list.Before when I was calling
\Drupal::service('plugin.manager.menu.link')->loadLinksByRoute('news_list'), without any second second parameter (route params), it was returning me the menu links, but with the change in 9.5.9, I'm enforced to call\Drupal::service('plugin.manager.menu.link')->loadLinksByRoute('news_list', ['type' => 'news-list').For me this broke many small things, like breadcrumbs, custom menu blocks, etc.
 - π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
It sounds like there are two separate issues here.
The first one (the warning from title controller) is covered by this patch.
Can we get a separate issue to address the menu active trail issue?
 - πΊπΈUnited States wells Seattle, WA
Make sense. Thanks for chiming it, @larowlan. I have moved the active menu trail issue to π [regression] missing menu active trail in Drupal 9.5.9 Needs work .
 - π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Thanks, watching both actively to try and get these in asap
 - Status changed to RTBC
over 2 years ago 2:23am 13 May 2023 - πΊπΈUnited States smustgrave
Believe the issue on this particular issue has been addressed and issue summary matches what is in the MR.
 - last update
over 2 years ago 30,334 pass - last update
over 2 years ago 30,334 pass - last update
over 2 years ago 30,334 pass - last update
over 2 years ago 30,334 pass - last update
over 2 years ago 30,334 pass - last update
over 2 years ago 30,334 pass - last update
over 2 years ago 30,334 pass - last update
over 2 years ago 30,335 pass - Status changed to Needs work
over 2 years ago 9:42am 29 May 2023 - π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Can we get a version that applies cleanly to 11.x and 10.1.x too
thanks!
 - Open on Drupal.org βEnvironment: PHP 8.1 & MySQL 5.7last update
over 2 years ago Not currently mergeable. - @totten opened merge request.
 - last update
over 2 years ago 28,523 pass - last update
over 2 years ago 28,523 pass - πΊπΈUnited States totten
Added a forward-port (from !3953 on 9.5.x to !4129 on 10.0.x). The merge-conflict was fairly small. The new test passed locally, and I confirmed that one specific error scenario ( re: civicrm-core@5.60 π TypeError: htmlspecialchars(): Argument #1 ($string) must be of type string, array given in htmlspecialchars() (line 432 of core\lib\Drupal\Component\Utility\Html.php) Active ) worked with this fix.
Aside: This is the first time I've posted a patch to drupal core, so I'm not too familiar with the workflows. But I can say that the merge-conflict should be resolved -- you can cherry-pick the
10.0.xpatch on10.1.xor11.x. - last update
over 2 years ago 29,420 pass - @totten opened merge request.
 - last update
over 2 years ago 29,420 pass - last update
over 2 years ago 29,436 pass - @totten opened merge request.
 - Status changed to RTBC
over 2 years ago 9:24pm 8 June 2023 - πΊπΈUnited States totten
Added forward ports for 10.1.x and 11.x. Tests still passing.
 - last update
over 2 years ago 29,437 pass - Status changed to Fixed
over 2 years ago 3:16pm 12 June 2023 - π¬π§United Kingdom catch
Committed/pushed to 11.x, cherry-picked to 10.1.x and 10.0.x, committed the 9.5.x MR to 9.5.x, thanks!
 Automatically closed - issue fixed for 2 weeks with no activity.