Twig template link() call to accept markup

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 %}

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.

+++ 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?

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 . '>');

+++ 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.

+++ 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?

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.

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

@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.

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