Issue #3470231 by pdureau, just_like_good_vibes, g4mbini, grimreaper,...

Issue #3470231 by pdureau, just_like_good_vibes, g4mbini, grimreaper, smustgrave: Attributes normalization

modules/ui_patterns_library/templates/ui-patterns-component-table.html.twig

@@ -26,7 +26,7 @@
         </tr>
       {% endfor %}
       {% for prop_id, prop in component.props.properties %}
-        {% if prop_id != 'attributes' %}
+        {% if prop_id != 'attributes' and prop_id != 'variant' %}
           <tr class="ui_patterns_component_table__prop">
             <td>
               <code>{{ prop_id }}</code>

src/ComponentPluginManager.php

@@ -143,9 +143,7 @@ class ComponentPluginManager extends SdcPluginManager implements CategorizingPlu
     if (isset($definition["variants"])) {
       $definition['props']['properties']['variant'] = $this->buildVariantProp($definition);
     }
-    if (!isset($definition['props']['properties'])) {
-      return $definition;
-    }
+    $definition['props']['properties'] = $this->addAttributesProp($definition);
     foreach ($definition['props']['properties'] as $prop_id => $prop) {
       $definition['props']['properties'][$prop_id] = $this->annotateProp($prop_id, $prop, $fallback_prop_type_id);
     }
@@ -186,8 +184,31 @@ class ComponentPluginManager extends SdcPluginManager implements CategorizingPlu
     return $prop;
   }
 
+  /**
+   * Add attributes prop.
+   *
+   * 'attribute' is one of the 2 'magic' props: its name and type are already
+   * set. Always available because automatically added by
+   * ComponentsTwigExtension::mergeAdditionalRenderContext().
+   */
+  private function addAttributesProp(array $definition): array {
+    // Let's put it at the beginning (for forms).
+    return array_merge(
+     [
+       'attributes' => [
+         'title' => 'Attributes',
+         '$ref' => "ui-patterns://attributes",
+       ],
+     ],
+      $definition['props']['properties'] ?? [],
+    );
+  }
+
   /**
    * Build variant prop.
+   *
+   * 'variant' is one of the 2 'magic' props: its name and type are already set.
+   * Available if at least a variant is set in the component definition.
    */
   private function buildVariantProp(array $definition): array {
     $enums = [];

src/Element/ComponentElementAlter.php

@@ -4,8 +4,8 @@ declare(strict_types=1);
 
 namespace Drupal\ui_patterns\Element;
 
+use Drupal\Core\Plugin\Component;
 use Drupal\Core\Security\TrustedCallbackInterface;
-use Drupal\Core\Template\Attribute;
 use Drupal\Core\Theme\ComponentPluginManager;
 use Drupal\ui_patterns\Plugin\UiPatterns\PropType\SlotPropType;
 
@@ -31,6 +31,18 @@ class ComponentElementAlter implements TrustedCallbackInterface {
    * Alter SDC component element.
    */
   public function alter(array $element): array {
+    $element = $this->normalizeSlots($element);
+    $component = $this->componentPluginManager->find($element['#component']);
+    $element = $this->processAttributesProp($element, $component);
+    $element = $this->processAttributesRenderProperty($element);
+    $element = $this->normalizeProps($element, $component);
+    return $element;
+  }
+
+  /**
+   * Normalize slots.
+   */
+  public function normalizeSlots(array $element): array {
     foreach ($element["#slots"] as $slot_id => $slot) {
       // Because SDC validator is sometimes confused by a null slot.
       if (is_null($slot)) {
@@ -44,8 +56,13 @@ class ComponentElementAlter implements TrustedCallbackInterface {
       }
       $element["#slots"][$slot_id] = SlotPropType::normalize($slot);
     }
-    $component = $this->componentPluginManager->find($element['#component']);
-    $element = $this->processAttributes($element);
+    return $element;
+  }
+
+  /**
+   * Normalize props.
+   */
+  public function normalizeProps(array $element, Component $component): array {
     $props = $component->metadata->schema['properties'] ?? [];
     foreach ($element["#props"] as $prop_id => $prop) {
       if (!isset($props[$prop_id])) {
@@ -58,7 +75,25 @@ class ComponentElementAlter implements TrustedCallbackInterface {
   }
 
   /**
-   * Process #attributes property.
+   * Process attributes prop.
+   */
+  public function processAttributesProp(array $element, Component $component): array {
+    $element["#props"]["attributes"] = $element["#props"]["attributes"] ?? [];
+    // Attribute PHP objects are rendered as strings by SDC ComponentValidator,
+    // this is raising an error: "InvalidComponentException: String value
+    // found, but an object is required".
+    if (is_a($element["#props"]["attributes"], '\Drupal\Core\Template\Attribute')) {
+      $element["#props"]["attributes"] = $element["#props"]["attributes"]->toArray();
+    }
+    // Attributes prop must never be empty, to avoid the processing of SDC's
+    // ComponentsTwigExtension::mergeAdditionalRenderContext() which is adding
+    // an Attribute PHP object.
+    $element["#props"]["attributes"]['data-component-id'] = $component->getPluginId();
+    return $element;
+  }
+
+  /**
+   * Process #attributes render property.
    *
    * #attributes property is an universal property of the Render API, used by
    * many Drupal mechanisms from Core and Contrib, but not processed by SDC
@@ -66,15 +101,17 @@ class ComponentElementAlter implements TrustedCallbackInterface {
    *
    * @todo Move this to Drupal Core.
    */
-  public function processAttributes(array $element): array {
+  public function processAttributesRenderProperty(array $element): array {
     if (!isset($element["#attributes"])) {
       return $element;
     }
-    $attributes = new Attribute($element["#attributes"]);
-    if (isset($element["#props"]["attributes"])) {
-      $attributes = (new Attribute($element["#props"]["attributes"]))->merge($attributes);
+    if (is_a($element["#attributes"], '\Drupal\Core\Template\Attribute')) {
+      $element["#attributes"] = $element["#attributes"]->toArray();
     }
-    $element["#props"]["attributes"] = $attributes;
+    $element["#props"]["attributes"] = array_merge(
+      $element["#attributes"],
+      $element["#props"]["attributes"]
+    );
     return $element;
   }
 

src/Plugin/UiPatterns/PropType/AttributesPropType.php

@@ -48,20 +48,33 @@ class AttributesPropType extends PropTypePluginBase {
     Attributes are defined as a mapping ('object' in JSON schema). So, source
     plugins are expected to return a mapping to not break SDC prop validation
     against the prop type schema.
+     */
+    if (is_a($value, '\Drupal\Core\Template\Attribute')) {
+      return $value->toArray();
+    }
+    return $value;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function preprocess(mixed $value): mixed {
+    /*
     However, when they land in the template, it is safer to have them as
     Attribute objects:
     - if the template use create_attribute(), it will not break thanks to
     "#3403331: Prevent TypeError when using create_attribute Twig function"
     - if the template directly calls object methods, it will work because it
     is already an object
-    - ArrayAccess interface allows manipulation as an array,
-    It is possible because PropTypeInterface::normalize() is called by
-    ComponentElementAlter::alter() after SDC is doing the validation.
+    - ArrayAccess interface allows manipulation as an array.
      */
+    if (is_a($value, '\Drupal\Core\Template\Attribute')) {
+      return $value;
+    }
     if (is_array($value)) {
       return new Attribute($value);
     }
-    return $value;
+    return new Attribute();
   }
 
 }

src/Plugin/UiPatterns/PropType/BooleanPropType.php

@@ -21,4 +21,12 @@ use Drupal\ui_patterns\PropTypePluginBase;
   typed_data: ['boolean']
 )]
 class BooleanPropType extends PropTypePluginBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function normalize(mixed $value): mixed {
+    return (bool) $value;
+  }
+
 }

src/Plugin/UiPatterns/PropType/LinksPropType.php

@@ -136,6 +136,9 @@ class LinksPropType extends PropTypePluginBase implements ContainerFactoryPlugin
     if (!array_key_exists($property, $item)) {
       return $item;
     }
+    if (is_a($item[$property], '\Drupal\Core\Template\Attribute')) {
+      $item[$property] = $item[$property]->toArray();
+    }
     // Empty PHP arrays are converted in JSON arrays instead of JSON objects
     // by json_encode(), so it is better to remove them.
     if (is_array($item[$property]) && empty($item[$property])) {
@@ -201,6 +204,9 @@ class LinksPropType extends PropTypePluginBase implements ContainerFactoryPlugin
       $item["link_attributes"] = $url_attributes;
       return $item;
     }
+    if (is_a($item["link_attributes"], '\Drupal\Core\Template\Attribute')) {
+      $item["link_attributes"] = $item["link_attributes"]->toArray();
+    }
     if (is_array($item["link_attributes"])) {
       $item["link_attributes"] = array_merge(
         $item["link_attributes"],
@@ -208,10 +214,6 @@ class LinksPropType extends PropTypePluginBase implements ContainerFactoryPlugin
       );
       return $item;
     }
-    if (is_a($item["link_attributes"], '\Drupal\Core\Template\Attribute')) {
-      $item["link_attributes"]->merge(new Attribute($url_attributes));
-      return $item;
-    }
     return $item;
   }
 
@@ -274,4 +276,26 @@ class LinksPropType extends PropTypePluginBase implements ContainerFactoryPlugin
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public static function preprocess(mixed $value): mixed {
+    foreach ($value as $index => &$item) {
+      if (!is_array($item)) {
+        continue;
+      }
+      if (array_key_exists("attributes", $item) && is_array($item['attributes'])) {
+        $item["attributes"] = new Attribute($item["attributes"]);
+      }
+      if (array_key_exists("link_attributes", $item) && is_array($item['link_attributes'])) {
+        $item["link_attributes"] = new Attribute($item["link_attributes"]);
+      }
+      if (array_key_exists("below", $item)) {
+        $item["below"] = self::preprocess($item["below"]);
+      }
+      $value[$index] = $item;
+    }
+    return $value;
+  }
+
 }

src/Plugin/UiPatterns/Source/AttributesWidget.php

@@ -47,17 +47,33 @@ class AttributesWidget extends SourcePluginBase {
       '#type' => 'textfield',
       '#default_value' => $this->getSetting('value'),
     ];
-    // Allow anything in attributes values, which are between simple and double
-    // quotes. Forbid some characters in attributes names.
-    // See https://www.w3.org/TR/2011/WD-html5-20110525/syntax.html#attributes-0
-    $forbidden_characters = "<>&/`";
-    $pattern = "^(([^" . $forbidden_characters . "]+)=[\"'].+[\"']\s*)*?$";
-    $form['value']['#pattern'] = $pattern;
-    $form['value']['#description'] = $this->t("Values must be present and quoted.");
+    $form['value']['#pattern'] = $this->buildRegexPattern();
+    // To allow form errors to be displayed correctly.
+    $form['value']['#title'] = '';
+    $form['value']['#placeholder'] = 'class="hidden" title="Lorem ipsum"';
+    $form['value']['#description'] = $this->t("HTML attributes with double-quoted values.");
     $this->addRequired($form['value']);
     return $form;
   }
 
+  /**
+   * Build regular expression pattern.
+   *
+   * See https://html.spec.whatwg.org/#attributes-2
+   */
+  protected function buildRegexPattern(): string {
+    // Attribute names are a mix of ASCII lower and upper alphas.
+    $attr_name = "[a-zA-Z]+";
+    // Allow anything in attributes values, which are between double quotes.
+    $double_quoted_value = '"[\s\w]*"';
+    $space = "\s*";
+    $attr = $attr_name . "=" . $double_quoted_value . $space;
+    // The pattern must match the entire input's value, rather than matching a
+    // substring - as if a ^(?: were implied at the start of the pattern and )$
+    // at the end.
+    return $space . "(" . $attr . ")*";
+  }
+
   /**
    * Convert a string to an attribute mapping.
    */

src/PropTypeInterface.php

@@ -44,11 +44,22 @@ interface PropTypeInterface extends WithJsonSchemaInterface, PluginInspectionInt
   public function getSummary(array $definition): array;
 
   /**
-   * Normalize the prop type value.
+   * Normalize the prop type value before validation.
    *
    * @return mixed
-   *   The normalized prop type value.
+   *   The JSON schema valid prop type value.
    */
   public static function normalize(mixed $value): mixed;
 
+  /**
+   * Preprocess the prop type value before the rendering.
+   *
+   * Called after the validation, before being sent to the template, in order to
+   * ease the work of template owners.
+   *
+   * @return mixed
+   *   The processed prop type value.
+   */
+  public static function preprocess(mixed $value): mixed;
+
 }

src/PropTypePluginBase.php

@@ -71,4 +71,11 @@ abstract class PropTypePluginBase extends PluginBase implements PropTypeInterfac
     return $value;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public static function preprocess(mixed $value): mixed {
+    return $value;
+  }
+
 }

src/Template/ComponentNodeVisitor.php

+<?php
+
+namespace Drupal\ui_patterns\Template;
+
+use Drupal\Core\Plugin\Component;
+use Drupal\Core\Render\Component\Exception\ComponentNotFoundException;
+use Drupal\Core\Template\ComponentNodeVisitor as CoreComponentNodeVisitor;
+use Drupal\Core\Theme\ComponentPluginManager;
+use Twig\Environment;
+use Twig\Node\Expression\ConstantExpression;
+use Twig\Node\Expression\FunctionExpression;
+use Twig\Node\ModuleNode;
+use Twig\Node\Node;
+use Twig\Node\PrintNode;
+use Twig\NodeVisitor\NodeVisitorInterface;
+
+/**
+ * Provides a ComponentNodeVisitor to change the generated parse-tree.
+ *
+ * @internal
+ */
+class ComponentNodeVisitor implements NodeVisitorInterface {
+
+  /**
+   * Node name: expr.
+   */
+  const NODE_NAME_EXPR = 'expr';
+
+  /**
+   * The component plugin manager.
+   */
+  protected ComponentPluginManager $componentManager;
+
+  /**
+   * Constructs a new ComponentNodeVisitor object.
+   *
+   * @param \Drupal\Core\Theme\ComponentPluginManager $component_plugin_manager
+   *   The component plugin manager.
+   */
+  public function __construct(ComponentPluginManager $component_plugin_manager) {
+    $this->componentManager = $component_plugin_manager;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function enterNode(Node $node, Environment $env): Node {
+    return $node;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function leaveNode(Node $node, Environment $env): ?Node {
+    if (!$node instanceof ModuleNode) {
+      return $node;
+    }
+    $component = $this->getComponent($node);
+    if (!($component instanceof Component)) {
+      return $node;
+    }
+    $line = $node->getTemplateLine();
+    $function = $this->buildPreprocessPropsFunction($line, $component);
+    $node = $this->injectFunction($node, $function);
+    return $node;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getPriority(): int {
+    $priority = &drupal_static(__METHOD__);
+    if (!isset($priority)) {
+      $original_node_visitor = new CoreComponentNodeVisitor($this->componentManager);
+      // Ensure that this component node visitor's priority is higher than
+      // core's node visitor class for components, because this class has to run
+      // core's class.
+      $priority = $original_node_visitor->getPriority() + 1;
+    }
+    return is_numeric($priority) ? (int) $priority : 0;
+  }
+
+  /**
+   * Finds the SDC for the current module node.
+   *
+   * A duplicate of \Drupal\Core\Template\ComponentNodeVisitor::getComponent()
+   *
+   * @param \Twig\Node\Node $node
+   *   The node.
+   *
+   * @return \Drupal\Core\Plugin\Component|null
+   *   The component, if any.
+   */
+  protected function getComponent(Node $node): ?Component {
+    $component_id = $node->getTemplateName();
+    if (!preg_match('/^[a-z]([a-zA-Z0-9_-]*[a-zA-Z0-9])*:[a-z]([a-zA-Z0-9_-]*[a-zA-Z0-9])*$/', $component_id)) {
+      return NULL;
+    }
+    try {
+      return $this->componentManager->find($component_id);
+    }
+    catch (ComponentNotFoundException $e) {
+      return NULL;
+    }
+  }
+
+  /**
+   * Build the _ui_patterns_preprocess_props Twig function.
+   *
+   * @param int $line
+   *   The line .
+   * @param \Drupal\Core\Plugin\Component $component
+   *   The component.
+   *
+   * @return \Twig\Node\Node
+   *   The Twig function.
+   */
+  protected function buildPreprocessPropsFunction(int $line, Component $component): Node {
+    $component_id = $component->getPluginId();
+    $function_parameter = new ConstantExpression($component_id, $line);
+    $function_parameters_node = new Node([$function_parameter]);
+    $function = new FunctionExpression('_ui_patterns_preprocess_props', $function_parameters_node, $line);
+    return new PrintNode($function, $line);
+  }
+
+  /**
+   * Injects custom Twig nodes into given node as child nodes.
+   *
+   * The function will be injected direct after  validate_component_props
+   * function already injected by SDC's ComponentNodeVisitor.
+   *
+   * @param \Twig\Node\Node $node
+   *   The node where we will inject the function in.
+   * @param \Twig\Node\Node $function
+   *   The Twig function.
+   *
+   * @return \Twig\Node\Node
+   *   The node with the function inserted.
+   */
+  protected function injectFunction(Node $node, Node $function): Node {
+    $insertion = new Node([$node->getNode('display_start'), $function]);
+    $node->setNode('display_start', $insertion);
+    return $node;
+  }
+
+}

src/Template/TwigExtension.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Drupal\ui_patterns\Template;
 
+use Drupal\ui_patterns\ComponentPluginManager;
 use Twig\Extension\AbstractExtension;
 use Twig\TwigFilter;
 use Twig\TwigFunction;
@@ -17,6 +18,16 @@ class TwigExtension extends AbstractExtension {
 
   use AttributesFilterTrait;
 
+  /**
+   * Creates TwigExtension.
+   *
+   * @param \Drupal\ui_patterns\ComponentPluginManager $componentManager
+   *   The component plugin manager.
+   */
+  public function __construct(
+    protected ComponentPluginManager $componentManager,
+  ) {}
+
   /**
    * {@inheritdoc}
    */
@@ -24,15 +35,22 @@ class TwigExtension extends AbstractExtension {
     return 'ui_patterns';
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getNodeVisitors(): array {
+    return [
+      new ComponentNodeVisitor($this->componentManager),
+    ];
+  }
+
   /**
    * {@inheritdoc}
    */
   public function getFunctions() {
     return [
-      new TwigFunction('component', [
-        $this,
-        'renderComponent',
-      ]),
+      new TwigFunction('component', [$this, 'renderComponent']),
+      new TwigFunction('_ui_patterns_preprocess_props', [$this, 'preprocessProps'], ['needs_context' => TRUE]),
     ];
   }
 
@@ -70,4 +88,31 @@ class TwigExtension extends AbstractExtension {
     ];
   }
 
+  /**
+   * Preprocess props.
+   *
+   * This function must not be used by the templates authors. In a perfect
+   * world, it would not be necessary to set such a function. We did that to be
+   * compatible with SDC's ComponentNodeVisitor, in order to execute props
+   * preprocessing after SDC's validate_component_props Twig function.
+   *
+   * @param array $context
+   *   The context provided to the component.
+   * @param string $component_id
+   *   The component ID.
+   *
+   * @throws \Drupal\Core\Render\Component\Exception\InvalidComponentException
+   */
+  public function preprocessProps(array &$context, string $component_id): void {
+    $component = $this->componentManager->find($component_id);
+    $props = $component->metadata->schema['properties'] ?? [];
+    foreach ($context as $variable => $value) {
+      if (!isset($props[$variable])) {
+        continue;
+      }
+      $prop_type = $props[$variable]['ui_patterns']['type_definition'];
+      $context[$variable] = $prop_type->preprocess($value);
+    }
+  }
+
 }

tests/src/Unit/PropTypeNormalization/LinksPropTypeNormalizationTest.php

@@ -110,9 +110,9 @@ final class LinksPropTypeNormalizationTest extends UnitTestCase {
       [
         "title" => "With a relative URL",
         "url" => "/foo/bar",
-        "attributes" => new Attribute([
+        "attributes" => [
           "foo" => "bar",
-        ]),
+        ],
       ],
       [
         "title" => "With empty attributes",
@@ -183,12 +183,10 @@ final class LinksPropTypeNormalizationTest extends UnitTestCase {
       [
         "title" => "‹‹",
         "url" => "/articles?page=0",
-        "attributes" => new Attribute(),
       ],
       [
         "title" => "››",
         "url" => "/articles?page=2",
-        "attributes" => new Attribute(),
       ],
     ];
     return [
@@ -223,26 +221,23 @@ final class LinksPropTypeNormalizationTest extends UnitTestCase {
     ];
     $expected = [
       [
-        "title" => 0,
+        "title" => '0',
         "url" => "?page=0",
-        "attributes" => new Attribute(),
       ],
       [
-        "title" => 1,
+        "title" => '1',
         "url" => "?page=1",
-        "attributes" => new Attribute([
+        "attributes" => [
           "aria-current" => "page",
-        ]),
+        ],
       ],
       [
-        "title" => 2,
+        "title" => '2',
         "url" => "?page=2",
-        "attributes" => new Attribute(),
       ],
       [
-        "title" => 3,
+        "title" => '3',
         "url" => "?page=3",
-        "attributes" => new Attribute(),
       ],
     ];
     return [
@@ -279,22 +274,18 @@ final class LinksPropTypeNormalizationTest extends UnitTestCase {
     ];
     $expected = [
       [
-        "attributes" => new Attribute(),
         "title" => "« First",
         "url" => "?page=0",
       ],
       [
-        "attributes" => new Attribute(),
         "title" => "‹‹",
         "url" => "?page=0",
       ],
       [
-        "attributes" => new Attribute(),
         "title" => "››",
         "url" => "?page=2",
       ],
       [
-        "attributes" => new Attribute(),
         "title" => "Last »",
         "url" => "?page=3",
       ],
@@ -319,7 +310,7 @@ final class LinksPropTypeNormalizationTest extends UnitTestCase {
         "is_expanded" => FALSE,
         "is_collapsed" => FALSE,
         "in_active_trail" => FALSE,
-        "attributes" => new Attribute(),
+        "attributes" => [],
         "title" => "My account",
         "url" => Url::fromUserInput("/user", ["set_active_class" => TRUE]),
         "below" => [],
@@ -328,7 +319,7 @@ final class LinksPropTypeNormalizationTest extends UnitTestCase {
         "is_expanded" => FALSE,
         "is_collapsed" => FALSE,
         "in_active_trail" => FALSE,
-        "attributes" => new Attribute(),
+        "attributes" => [],
         "title" => "Log out",
         "url" => Url::fromUserInput("/user/logout", ["set_active_class" => TRUE]),
         "below" => [],
@@ -359,7 +350,6 @@ final class LinksPropTypeNormalizationTest extends UnitTestCase {
     $value = [
       [
         "href" => "?page=0",
-        "attributes" => new Attribute(),
         "link_attributes" => new Attribute([
           'class' => [
             'display-flex',
@@ -371,16 +361,15 @@ final class LinksPropTypeNormalizationTest extends UnitTestCase {
     ];
     $expected = [
       [
-        "title" => 0,
+        "title" => '0',
         "url" => "?page=0",
-        "attributes" => new Attribute(),
-        "link_attributes" => new Attribute([
+        "link_attributes" => [
           'class' => [
             'display-flex',
             'flex-align-center',
             'flex-no-wrap',
           ],
-        ]),
+        ],
       ],
     ];
     return [

ui_patterns.services.yml

@@ -72,6 +72,8 @@ services:
     class: Drupal\ui_patterns\Template\TwigExtension
     tags:
       - { name: twig.extension }
+    arguments:
+      - "@plugin.manager.sdc"
 
   ui_patterns.sample_entity_generator:
     class: Drupal\ui_patterns\Entity\SampleEntityGenerator