Problem/Motivation

Twig function link($text, $url, $attributes) where $text can be only plaintext there is no way of providing markup within the link within a Twig template.

Proposed resolution

When the incoming $text value is Twig_Markup object, cast it to TwigMarkupInterface when passing it back to the render system to preserve markup.

User interface changes

N/A

API changes

N/A

Original Issue Summary
Twig is suppose to provide a separation between front/back end logic but with menus this stops at the menu links. Because they utilize a Twig function link($text, $url, $attributes) where $text can be only plaintext I have no way of modifying markup within the link through Twig template logic.

1) Is there a work around? Specifically I need to use Twig logic to insert some markup within the <a> tag into certain links other than just the plaintext title.

2) Longer term we really need to work getting this and other markup more easily accessible on the Twig templates themselves. Drupal has a long history of not being that friendly for front-end developers and while 8.x has come a long way by using Twig there seems to still be a lot of unnecessary complexity when it comes to making simple markup changes thus as this.

Comments

Soundvessel created an issue. See original summary.

webflo’s picture

Status: Active » Needs review

I had the same issue, i think we should convert Twig_Markup object to Drupal Markup object. Twig_Markup contains safe html already. Here is an example how it works in the menu.html.twig template.

{% for item in items %}
  <li{{ item.attributes.addClass('menu__item') }}>
    {% set title %}
      {{ item.title }} <strong>my awesome markup.</strong>
    {% endset %}
    {{ link(title, item.url, null) }}
  {% endif %}
  </li>
{% endfor %}
webflo’s picture

LinkGenerator::generate generates the link in this case. LinkGenerator itself should not depend on the rendering engine, we have to convert the safe twig object to a safe drupal object.

webflo’s picture

Version: 8.1.2 » 8.2.x-dev
webflo’s picture

dawehner’s picture

+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -273,6 +274,11 @@ public function getLink($text, $url, $attributes = []) {
+      $text = Markup::create($text);

should we call (string) $text maybe?

webflo’s picture

I don't think so this would lead to double escaping see LinkGenerator::generate.

if (!($variables['text'] instanceof MarkupInterface)) {
  $variables['text'] = Html::escape($variables['text']);
}
$attributes = new Attribute($attributes);
// This is safe because Attribute does escaping and $variables['text'] is
// either rendered or escaped.
return $generated_link->setGeneratedLink('<' . $generated_link::TAG . $attributes . '>' . $variables['text'] . '</' . $generated_link::TAG . '>');
joelpittet’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/lib/Drupal/Core/Utility/LinkGeneratorInterface.php
@@ -31,7 +31,7 @@
-   * @param string|array $text
+   * @param string|array|\Drupal\Component\Render\MarkupInterface $text

This is already accepted, this is just documenting it properly (see the rest of the description). Not sure if this needs a CR or not?

This seems like the right solution to me for now. The only better would be if we could replace \Twig_Markup() with our MarkupInterface, but that's not possible.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update
+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -273,6 +274,11 @@ public function getLink($text, $url, $attributes = []) {
+    // The text has been processed by twig already, convert it to a safe object
+    // for the render system.
+    if ($text instanceof \Twig_Markup) {
+      $text = Markup::create($text);
+    }

The fix does not match the issue summary and title. The solution looks sensible but I wonder if we need to convert the object? Maybe we can make the bits that chose whether to do auto-escape Twig_Markup aware?

dawehner’s picture

The fix does not match the issue summary and title. The solution looks sensible but I wonder if we need to convert the object? Maybe we can make the bits that chose whether to do auto-escape Twig_Markup aware?

Well, conceptually I think mixing twig and render level objects is wrong. The render system should not know about how twig works internally. Converting from the domain of twig to the render system is what this patch is doing, right in the spot which still knows about twig, aka. the twig extensions system.

joelpittet’s picture

Title: Missing TWIG template for menu link markup » Twig template link() call to accept markup
Component: menu system » theme system
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
Issue tags: -Front end, -template, -Needs issue summary update +frontend

Fixed the issue summary and title. Though I like the intent and idea behind #9, I agree with @dawehner's rational in #10

alexpott’s picture

Status: Reviewed & tested by the community » Patch (to be ported)

@dawehner good point about twig and render.

Committed 1742237 and pushed to 8.3.x. Thanks!

Leaving as patch to be ported for cherry-pick to 8.2.x once 8.2.0 is released.

  • alexpott committed 1742237 on 8.3.x
    Issue #2744517 by webflo: Twig template link() call to accept markup
    
alexpott’s picture

Status: Patch (to be ported) » Fixed

Committed 5429c93 and pushed to 8.2.x. Thanks!

  • alexpott committed 5429c93 on 8.2.x
    Issue #2744517 by webflo: Twig template link() call to accept markup
    
    (...

Status: Fixed » Closed (fixed)

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