Drupal installation, Verify requirements step error message is not escaped properly

Kindly review a patch.

I'm not sure if flipping the order should be in scope of this issue. The weight is most likely going to be associated with the module/extension weight. I removed the Novice issue tag because I do not think there is enough information here for someone to pick up and work on with regard to requirements order.

I updated the version to 8.9.x. This would apply to the most recent version for development rather than a specific release.

I also found a related issue for inline template is used in requirements when using drupal console #3081572: Unexpected description format when parsing the unmet requirements message string.

#3 applied properly. LGTM + RTBC
Assuming set weight not covering in this issue. Thanks

+++ b/core/modules/media/media.install
@@ -58,6 +58,9 @@ function media_install() {
+  $requirements['media'] = [
+    'title' => t('Media system'),
+  ];

@@ -72,7 +75,14 @@ function media_requirements($phase) {
         $requirements['media']['description'] = $description;

This should be next to $requirements['media']['description'] = $description;

In fact this section deserves a little bit of a rework for clarity...

  if ($phase == 'install') {
    $error = NULL;
    $destination = 'public://media-icons/generic';
    \Drupal::service('file_system')->prepareDirectory($destination, FileSystemInterface::CREATE_DIRECTORY | FileSystemInterface::MODIFY_PERMISSIONS);
    if (!is_dir($destination)) {
      $error = t('The directory %directory does not exist.', ['%directory' => $destination]);
    }
    elseif(!is_writable($destination)) {
      $error = t('The directory %directory is not writable.', ['%directory' => $destination]);
    }
    if (isset($error)) {
      $description = t('An automated attempt to create this directory failed, possibly due to a permissions problem. To proceed with the installation, either create the directory and modify its permissions manually or ensure that the installer has the permissions to create it automatically. For more information, see INSTALL.txt or the <a href=":handbook_url">online handbook</a>.', [':handbook_url' => 'https://www.drupal.org/server-permissions']);
      $description = [
        '#type' => 'inline_template',
        '#template' => '{{ error }} {{ description }}',
        '#context' => [
          'error' => $error,
          'description' => $description,
        ],
      ];
      $requirements['media'] = [
        'title' => t('Media system'),
        'description' => $description,
        'severity' => REQUIREMENT_ERROR,
      ];
    }
//... the rest of the code.

I've re-rolled patch for 9.1

For #12

This has moved to \Drupal\media\Install\Requirements\MediaRequirements::getRequirements. It's also not in system.module so I'm updating the component.

I think now that this is in a much more isolated function we can use early returns for clarity. Something like this:

public static function getRequirements(): array {
  $requirements = [];
  $destination = 'public://media-icons/generic';
  \Drupal::service('file_system')->prepareDirectory($destination, FileSystemInterface::CREATE_DIRECTORY | FileSystemInterface::MODIFY_PERMISSIONS);
  $is_directory = is_dir($destination);
  $is_writable = is_writable($destination);
  if ($is_directory && $is_writable) {
    return $requirements;
  }
  $t_args = ['%directory' => $destination];
  $error = !$is_directory
    ? $this->t('The directory %directory does not exist.', $t_args)
    : $this->t('The directory %directory is not writable.', $t_args);

  // The rest of the inline template / description bits here.
}

Tried to ping in #core-development but no luck. Think good to unassign after all the years and since it moved to media maybe doesn't need sub-maintainer for system.

Can we cleanup the summary though seems like a different solution is being used now right?