Issue #2552579 by alexpott: Remove SafeMarkup::placeholder(), deprecate...

Issue #2552579 by alexpott: Remove SafeMarkup::placeholder(), deprecate drupal_placeholder() and stop drupal_placeholder() from marking safe

core/includes/bootstrap.inc

@@ -7,6 +7,7 @@
 use Drupal\Component\Datetime\DateTimePlus;
 use Drupal\Component\Utility\Crypt;
 use Drupal\Component\Utility\Environment;
+use Drupal\Component\Utility\Html;
 use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Component\Utility\Unicode;
 use Drupal\Core\DrupalKernel;
@@ -953,10 +954,16 @@ function drupal_static_reset($name = NULL) {
 /**
  * Formats text for emphasized display in a placeholder inside a sentence.
  *
- * @see \Drupal\Component\Utility\SafeMarkup::placeholder()
+ * @deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0. Use
+ *   \Drupal\Component\Utility\SafeMarkup::format() or Twig's "placeholder"
+ *   filter instead. Note this method should not be used to simply emphasize a
+ *   string and therefore has few valid use-cases. Note also, that this method
+ *   does not mark the string as safe.
+ *
+ * @see \Drupal\Component\Utility\SafeMarkup::format()
  */
 function drupal_placeholder($text) {
-  return SafeMarkup::placeholder($text);
+  return '<em class="placeholder">' . Html::escape($text) . '</em>';
 }
 
 /**

core/lib/Drupal/Component/Utility/SafeMarkup.php

@@ -197,8 +197,8 @@ public static function checkPlain($text) {
    *   formatting depends on the first character of the key:
    *   - @variable: Escaped to HTML using self::escape(). Use this as the
    *     default choice for anything displayed on a page on the site.
-   *   - %variable: Escaped to HTML and formatted using self::placeholder(),
-   *     which makes the following HTML code:
+   *   - %variable: Escaped to HTML wrapped in <em> tags, which makes the
+   *     following HTML code:
    *     @code
    *       <em class="placeholder">text output here.</em>
    *     @endcode
@@ -232,7 +232,7 @@ public static function format($string, array $args) {
         case '%':
         default:
           // Escaped and placeholder.
-          $args[$key] = static::placeholder($value);
+          $args[$key] = '<em class="placeholder">' . static::escape($value) . '</em>';
           break;
 
         case '!':
@@ -251,21 +251,4 @@ public static function format($string, array $args) {
     return $output;
   }
 
-  /**
-   * Formats text for emphasized display in a placeholder inside a sentence.
-   *
-   * Used automatically by self::format().
-   *
-   * @param string $text
-   *   The text to format (plain-text).
-   *
-   * @return string
-   *   The formatted text (html).
-   */
-  public static function placeholder($text) {
-    $string = '<em class="placeholder">' . static::escape($text) . '</em>';
-    static::$safeStrings[$string]['html'] = TRUE;
-    return $string;
-  }
-
 }

core/modules/editor/editor.module

@@ -78,7 +78,7 @@ function editor_form_filter_admin_overview_alter(&$form, FormStateInterface $for
   $editors = \Drupal::service('plugin.manager.editor')->getDefinitions();
   foreach (Element::children($form['formats']) as $format_id) {
     $editor = editor_load($format_id);
-    $editor_name = ($editor && isset($editors[$editor->getEditor()])) ? $editors[$editor->getEditor()]['label'] : drupal_placeholder('—');
+    $editor_name = ($editor && isset($editors[$editor->getEditor()])) ? $editors[$editor->getEditor()]['label'] : '—';
     $editor_column['editor'] = array('#markup' => $editor_name);
     $position = array_search('name', array_keys($form['formats'][$format_id])) + 1;
     $start = array_splice($form['formats'][$format_id], 0, $position, $editor_column);

core/modules/filter/src/FilterFormatListBuilder.php

@@ -7,7 +7,6 @@
 
 namespace Drupal\filter;
 
-use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Config\Entity\DraggableListBuilder;
 use Drupal\Core\Entity\EntityInterface;
@@ -94,24 +93,28 @@ public function buildHeader() {
   public function buildRow(EntityInterface $entity) {
     // Check whether this is the fallback text format. This format is available
     // to all roles and cannot be disabled via the admin interface.
+    $row['label'] = $this->getLabel($entity);
+    $row['roles'] = [];
     if ($entity->isFallbackFormat()) {
-      $row['label'] = SafeMarkup::placeholder($entity->label());
-
       $fallback_choice = $this->configFactory->get('filter.settings')->get('always_show_fallback_choice');
       if ($fallback_choice) {
-        $roles_markup = SafeMarkup::placeholder($this->t('All roles may use this format'));
+        $roles_markup = $this->t('All roles may use this format');
       }
       else {
-        $roles_markup = SafeMarkup::placeholder($this->t('This format is shown when no other formats are available'));
+        $roles_markup = $this->t('This format is shown when no other formats are available');
       }
+      // Emphasize the fallback role text since it is important to understand
+      // how it works which configuring filter formats. Additionally, it is not
+      // a list of roles unlike the other values in this column.
+      $row['roles']['#prefix'] = '<em>';
+      $row['roles']['#suffix'] = '</em>';
     }
     else {
-      $row['label'] = $this->getLabel($entity);
       $roles = array_map('\Drupal\Component\Utility\SafeMarkup::checkPlain', filter_get_roles_by_format($entity));
       $roles_markup = $roles ? implode(', ', $roles) : $this->t('No roles may use this format');
     }
 
-    $row['roles'] = !empty($this->weightKey) ? array('#markup' => $roles_markup) : $roles_markup;
+    $row['roles']['#markup'] = $roles_markup;
 
     return $row + parent::buildRow($entity);
   }

core/modules/filter/src/FilterPermissions.php

@@ -7,7 +7,6 @@
 
 namespace Drupal\filter;
 
-use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
 use Drupal\Core\Entity\EntityManagerInterface;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
@@ -60,7 +59,11 @@ public function permissions() {
       if ($permission = $format->getPermissionName()) {
         $permissions[$permission] = [
           'title' => $this->t('Use the <a href="@url">@label</a> text format', ['@url' => $format->url(), '@label' => $format->label()]),
-          'description' => SafeMarkup::placeholder($this->t('Warning: This permission may have security implications depending on how the text format is configured.')),
+          'description' => [
+            '#prefix' => '<em>',
+            '#markup' => $this->t('Warning: This permission may have security implications depending on how the text format is configured.'),
+            '#suffix' => '</em>'
+          ],
         ];
       }
     }

core/modules/node/src/Controller/NodeController.php

@@ -7,7 +7,6 @@
 
 namespace Drupal\node\Controller;
 
-use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Component\Utility\Xss;
 use Drupal\Core\Controller\ControllerBase;
 use Drupal\Core\Datetime\DateFormatter;
@@ -206,7 +205,11 @@ public function revisionOverview(NodeInterface $node) {
       if ($vid == $node->getRevisionId()) {
         $row[0]['class'] = ['revision-current'];
         $row[] = [
-          'data' => SafeMarkup::placeholder($this->t('current revision')),
+          'data' => [
+            '#prefix' => '<em>',
+            '#markup' => $this->t('current revision'),
+            '#suffix' => '</em>',
+          ],
           'class' => ['revision-current'],
         ];
       }

core/modules/update/update.report.inc

@@ -228,7 +228,11 @@ function template_preprocess_update_project_status(&$variables) {
       $extra_item = array();
       $extra_item['attributes'] = new Attribute();
       $extra_item['label'] = $value['label'];
-      $extra_item['data'] = drupal_placeholder($value['data']);
+      $extra_item['data'] = [
+        '#prefix' => '<em>',
+        '#markup' => $value['data'],
+        '#suffix' => '</em>'
+      ];
       $variables['extras'][] = $extra_item;
     }
   }

core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php

@@ -7,7 +7,7 @@
 
 namespace Drupal\user\Plugin\Validation\Constraint;
 
-use Drupal\Component\Utility\SafeMarkup;
+use Drupal\Component\Utility\Html;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidatorInterface;
 use Symfony\Component\Validator\ExecutionContextInterface;
@@ -29,6 +29,9 @@ class UserMailRequired extends Constraint implements ConstraintValidatorInterfac
   /**
    * Violation message. Use the same message as FormValidator.
    *
+   * Note that the name argument is not sanitized so that translators only have
+   * one string to translate. The name is sanitized in self::validate().
+   *
    * @var string
    */
   public $message = '!name field is required.';
@@ -70,7 +73,7 @@ public function validate($items, Constraint $constraint) {
     $required = !(!$existing_value && \Drupal::currentUser()->hasPermission('administer users'));
 
     if ($required && (!isset($items) || $items->isEmpty())) {
-      $this->context->addViolation($this->message, array('!name' => SafeMarkup::placeholder($account->getFieldDefinition('mail')->getLabel())));
+      $this->context->addViolation($this->message, ['!name' => Html::escape($account->getFieldDefinition('mail')->getLabel())]);
     }
   }
 

core/modules/user/src/Tests/UserValidationTest.php

@@ -136,7 +136,7 @@ function testValidation() {
     $violations = $user->validate();
     $this->assertEqual(count($violations), 1, 'E-mail addresses may not be removed');
     $this->assertEqual($violations[0]->getPropertyPath(), 'mail');
-    $this->assertEqual($violations[0]->getMessage(), t('!name field is required.', array('!name' => SafeMarkup::placeholder($user->getFieldDefinition('mail')->getLabel()))));
+    $this->assertEqual($violations[0]->getMessage(), t('!name field is required.', array('!name' => $user->getFieldDefinition('mail')->getLabel())));
     $user->set('mail', 'someone@example.com');
 
     $user->set('timezone', $this->randomString(33));

core/tests/Drupal/Tests/Component/Utility/SafeMarkupTest.php

@@ -192,15 +192,6 @@ function providerFormat() {
     return $tests;
   }
 
-  /**
-   * Tests SafeMarkup::placeholder().
-   *
-   * @covers ::placeholder
-   */
-  function testPlaceholder() {
-    $this->assertEquals('<em class="placeholder">Some text</em>', SafeMarkup::placeholder('Some text'));
-  }
-
   /**
    * Tests the interaction between the safe list and XSS filtering.
    *