Problem/Motivation

We added a Drupal\Core\Url class in #2153891: Add a Url value object which will be used all over the codebase, but its name conflicts with Drupal\Component\Utility\Url. Even though they're in different namespaces, this clash is really unfortunate and bound to create confusion.

Proposed resolution

Rename Drupal\Component\Utility\Url to Drupal\Component\Utility\UrlHelper

Remaining tasks

None.

User interface changes

None.

API changes

This utility class is renamed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu’s picture

Status: Active » Needs review
FileSize
31.72 KB

Not sure if we need to wait for the other issue, this is a simple patch that makes sense on its own.

Status: Needs review » Needs work

The last submitted patch, 1: 2184653-rename_url_to_urlhelper.patch, failed testing.

amateescu’s picture

Status: Needs work » Needs review

Phpstorm--

amateescu’s picture

Issue summary: View changes
Issue tags: +DAT
Related issues: +#2153891: Add a Url value object
FileSize
32.69 KB

Rerolled since #2153891: Add a Url value object got in first.

amateescu’s picture

Issue summary: View changes
dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Perfect! (beside of silly 80 chars in documentation I do ignore)

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
grep -r 'Utility\\\Url;' ./core/
./core//modules/system/lib/Drupal/system/Tests/Common/RenderElementTypesTest.php:use Drupal\Component\Utility\Url;
./core//tests/Drupal/Tests/Core/EventSubscriber/ExceptionListenerTest.php:use Drupal\Component\Utility\Url;

Seems like we still have things to convert.

+++ b/core/modules/system/lib/Drupal/system/Tests/Common/XssUnitTest.php
@@ -54,12 +54,12 @@ function testT() {
-    // HTML. Ensure \Drupal\Component\Utility\Url::stripDangerousProtocols() can
+    // HTML. Ensure \Drupal\Component\Utility\UrlHelper::stripDangerousProtocols() can

Let's fix this

amateescu’s picture

Status: Needs work » Needs review
FileSize
35.93 KB
4.93 KB

All fixed :)

amateescu’s picture

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

YAR!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

It looks like these classes could be merged, at least partially. Probably best done later. Committed this patch to 8.x. as it helps resolve confusion.

Status: Fixed » Closed (fixed)

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