Problem/Motivation

Route name is useful to know in outbound route processing.

dawehner would like this for #2345753: Remove url(current_path()) and url(NULL) with <current> and <none>.

Proposed resolution

Add the route name!

Remaining tasks

User interface changes

API changes

Additional $route_name parameter added to OutboundRouteProcessorInterface

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, d8-route-name-route-processor.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
8.15 KB
2.38 KB

Meh.

dawehner’s picture

+++ b/core/lib/Drupal/Core/Access/RouteProcessorCsrf.php
@@ -37,7 +37,7 @@ function __construct(CsrfTokenGenerator $csrf_token) {
-  public function processOutbound(Route $route, array &$parameters) {
+  public function processOutbound(Route $route, array &$parameters, $route_name) {

MEH, don't we want to have route_name first?

damiankloip’s picture

Priority: Normal » Critical
damiankloip’s picture

FileSize
8.2 KB
7.09 KB

Sure, I don't mind either way.

dawehner’s picture

+++ b/core/tests/Drupal/Tests/Core/RouteProcessor/RouteProcessorManagerTest.php
@@ -34,11 +34,12 @@ protected function setUp() {
+      10 => $this->getMockProcessor($route, $parameters, $route_name),
+      5 => $this->getMockProcessor($route, $parameters, $route_name),
+      0 => $this->getMockProcessor($route, $parameters, $route_name),

@@ -56,14 +57,16 @@ public function testRouteProcessorManager() {
+  protected function getMockProcessor($route, $parameters, $route_name) {
     $processor = $this->getMock('Drupal\Core\RouteProcessor\OutboundRouteProcessorInterface');
     $processor->expects($this->once())
       ->method('processOutbound')
-      ->with($route, $parameters);
+      ->with($route_name, $route, $parameters);
 

Can we change this order as well?

damiankloip’s picture

FileSize
8.25 KB
1.8 KB

Here we go.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Thank you man.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

https://www.drupal.org/node/2238759 looks like the right change notice to update, but already very out of date.

andypost’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.