Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Currently template_preprocess_file_link()
returns a link generated with Link::fromTextAndUrl()->toString()
. This html can no be preprocessed by other themes/modules.
Proposed resolution
Return a renderable array in template_preprocess_file_link()
, so other themes can add classes or update the title.
Remaining tasks
- Write a patch
- Review
- Commit
User interface changes
None.
API changes
None.
Data model changes
template_preprocess_file_link()
returns a renderable array for the link instead of html.
Comments
Comment #2
geoffray CreditAttribution: geoffray as a volunteer commentedComment #3
geoffray CreditAttribution: geoffray as a volunteer commentedComment #7
stefan.kornThis issue should surely be addressed soon because
Drupal::l()
is deprecated.as per deprecated notice the replacement for
Drupal::l()
would be:Link::fromTextAndUrl($link_text, Url::fromUri($url, $options));
But i would suppose the patch provided by @geoffray would also suit the deprecation notice.
Comment #8
Olarin CreditAttribution: Olarin at Kosada commentedThis is a small, logical, and straightforward change, which shouldn't break anything for anybody (except possibly implementations of the file_link preprocessor that are specifically expecting to see the GeneratedLink object, and I can't really imagine what they could be usefully accomplishing with it), and is very useful to anyone who needs to modify that link for some reason. The attached patch worked well for me on a Drupal 8.6 installation.
Bumping to Major because of the deprecation issue, since D9 is theoretically less than a year away; apologies if that's inappropriate (the priority documentation doesn't really mention deprecation issues), but this has been sitting back-burner for a while and it seems like an easy win to me.
Comment #9
Olarin CreditAttribution: Olarin at Kosada commentedRe-rolling against 8.8. Seems somebody already addressed the deprecation issue by using Link::fromTextAndUrl, but I believe it would still be more appropriate to return a render array here.
Comment #10
stefan.kornYes, it seems this has been addressed very recently with this issue: https://www.drupal.org/node/3047801
So maybe this issue could be closed as duplicate.
Comment #11
idebr CreditAttribution: idebr at ezCompany commented#10 The problem described in the issue summary still applies: the generated link cannot be easily preprocessed or altered by themes.
Let's go with
toRenderable()
instead oftoString()
for the smallest possible change.Comment #12
stefan.kornYes, the problem still applies. I've overlooked the "toString()" call they put in with https://www.drupal.org/node/3047801.
Here comes the patch with toRenderable() which will result exactly in what is provided by the previous two patches.
But it seems even not to be necessary to use toRenderable() at all here, since Twig can handle the Link object too.
Comment #13
stefan.kornComment #14
idebr CreditAttribution: idebr at ezCompany commented#12 Looks good to me. Typically Drupal core returns renderable arrays instead of class objects, so let's go with
toRenderable()
in line with the issue summary.Comment #15
larowlanComment #16
larowlanCommitted 6bae96a and pushed to 8.8.x. Thanks!
Added and published a change record https://www.drupal.org/node/3074716