Problem/Motivation

We need proper replacements for the following code pieces:

url(NULL, ['absolute' => TRUE]);
url(current_path());

Proposed resolution

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
3.67 KB

There we go.

dawehner’s picture

FileSize
2.94 KB

It was not intended to left in the prove that I tested it.

tim.plunkett’s picture

Lets do one conversion each.

Wim Leers’s picture

+++ b/core/lib/Drupal/Core/PathProcessor/PathProcessorNone.php
@@ -0,0 +1,27 @@
+      return NULL;

Wouldn't the empty string be more logical to return? NULL violates the interface of processOutbound(), which says to return the processed path, and paths must be strings.

(Plus, it is cast to a string anyway when used, so better to do that right away.)

I'm also reviewing this from the perspective of fixing #2335661: Outbound path & route processors must specify cacheability metadata, which will require path processors to indicate the cacheability of their alterations. If one of the path processors returns NULL instead of a string, that'll be more difficult to solve.

dawehner’s picture

FileSize
6.13 KB
9.61 KB

Started with some integration tests.

tim.plunkett’s picture

  1. +++ b/core/lib/Drupal/Core/PathProcessor/PathProcessorCurrent.php
    @@ -0,0 +1,33 @@
    +      debug($request_uri);
    +      debug($current_base_path);
    
    +++ b/core/modules/node/src/Controller/NodeController.php
    @@ -62,6 +62,8 @@ public static function create(ContainerInterface $container) {
    +    debug(\Drupal::url('<current>'));
    

    I think the new tests look good. Just remove these.

  2. +++ b/core/lib/Drupal/Core/PathProcessor/PathProcessorNone.php
    @@ -0,0 +1,27 @@
    +class PathProcessorNone implements OutboundPathProcessorInterface {
    ...
    +      return NULL;
    

    Wasn't that supposed to be return '';

dawehner’s picture

FileSize
12.96 KB
0 bytes

Added a second integration test. I think this is ready to fly.

tim.plunkett’s picture

  1. +++ b/core/core.services.yml
    @@ -819,6 +819,16 @@ services:
    +    arguments: ['@config.factory']
    ...
    +    arguments: ['@config.factory']
    

    Why do these need the config factory?

  2. +++ b/core/modules/node/src/Controller/NodeController.php
    @@ -62,6 +62,8 @@ public static function create(ContainerInterface $container) {
    +    debug(\Drupal::url('<current>'));
    

    Still need to kill this

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
12.23 KB
1.28 KB

You know, that's easy enough I can just do it and RTBC. I think @dawehner is traveling today.

xjm’s picture

Priority: Normal » Critical
Issue tags: +Pre-AMS beta sprint, +beta target

This will block #2343669: Remove _l() and _url(), so bumping to critical.

Do we have a followup somewhere to remove current_path()?

xjm’s picture

tim.plunkett’s picture

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

  • webchick committed 8bd6376 on 8.0.x
    Issue #2344487 by tim.plunkett, dawehner: Provide special route names...
Wim Leers’s picture

Status: Fixed » Closed (fixed)

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