Problem/Motivation

There a few _l() calls in the file and system modules, as for #2343669: Remove _l() and _url() they need to be removed.

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#5 2404041-remove_l-5.patch519 bytespcambra
#5 interdiff.txt527 bytespcambra
#4 2404041-remove_l-4.patch505 bytespcambra
#4 interdiff.txt1.32 KBpcambra
#1 2404041-remove_l-1.patch1.31 KBpcambra

Comments

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Active » Needs review
StatusFileSize
new 2404041-remove_l-1.patch1.31 KB

For the system, it's a test relying on _l() which if it's gone I don't see the point in keeping it, the link generator is tested elsewhere.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
commented

Are you sure this is an absolute link already always?

+++ b/core/modules/file/file.module
@@ -1243,7 +1243,7 @@ function template_preprocess_file_link(&$variables) {
+  $variables['link'] = \Drupal::linkGenerator()->generate($link_text, Url::fromUri($url, $options));

You can use \Drupal::l() directly

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Title: Replace _l() calls in system and file modules » Replace _l() calls in file module
StatusFileSize
new interdiff.txt1.32 KB
new 2404041-remove_l-4.patch505 bytes

According to the point 2 in #2364161: Replace nearly all existing _l calls the test would be removed when the _l() function is gone itself, so taking that change out of here.

Modified the patches with the comment in #3, thanks @dawehner

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
StatusFileSize
new interdiff.txt527 bytes
new 2404041-remove_l-5.patch519 bytes

Missed one bit

The last submitted patch, 4: 2404041-remove_l-4.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

Are you sure this is an absolute URL already always?

yesct’s picture

yesct commented
Parent issue: » #2343669: Remove _l() and _url()
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Priority: Normal » Critical

This is a hard blocker to a critical issue at #2343669: Remove _l() and _url() so escalating to critical, too.

idebr’s picture

idebr commented

@dawehner file_create_url currently always returns an absolute url, but there is an open issue to convert this to a relative url at #1377840: Caching does not properly respect protocol (a problem when dealing with https)

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/file/file.module
@@ -1243,7 +1243,7 @@ function template_preprocess_file_link(&$variables) {
-  $variables['link'] = _l($link_text, $url, $options);
+  $variables['link'] = \Drupal::l($link_text, Url::fromUri($url, $options));
 }

So $url is already file_create_url() so Url::fromUri() is perfect

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 0584b18 and pushed to 8.0.x. Thanks!

  • alexpott committed 0584b18 on 8.0.x 
    Issue #2404041 by pcambra: Replace _l() calls in file module

Status: Fixed » Closed (fixed)

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