[meta] Replace remaining !placeholder for Non-URL HTML outputs only

As discussed in #2506445-176: Replace !placeholder with @placeholder in t() and format_string() for non-URLs in tests and #177. Include these changes in this issue:

+++ b/core/modules/config_translation/tests/src/Unit/ConfigEntityMapperTest.php
@@ -70,7 +70,7 @@ protected function setUp() {
-      'title' => '!label language',
+      'title' => '@label language',

+++ b/core/modules/config_translation/tests/src/Unit/ConfigFieldMapperTest.php
@@ -50,7 +50,7 @@ protected function setUp() {
-      'title' => '!label field',
+      'title' => '@label field',

Although this issue is still on hold and taking the risk that my work is in vain, I had the time to work on it.

This time a patch from scratch. Only low hanging !placeholder fruit was picked. The more difficult !placeholders are listed below and may or may not deserve a follow-up issue. In all cases the !placeholder contains HTML in some form.

contextual.module:
t('Hovering over the area ... The icon typically looks like this: !picture', array('!picture' => $sample_picture))

system.admin.inc:

t('Machine name: !machine-name', array('!machine-name' => '<span dir="ltr" class="table-filter-text-source">' . $key . '</span>'))

BreakLockForm.php:

$this->t('By breaking this lock, any unsaved changes made by !user will be lost.', array('!user' => drupal_render($username)));

ViewEditForm.php:

$this->t('This view is being edited by user !user, ... Click here to <a href="@url">break this lock</a>.', $lock_message_substitutions),

AccountForm.php:

$current_pass_description = $this->t('Required if you want to change the %mail or %pass below. !request_new.',

ConfigSync.php:
$this->t('The following items in your active configuration ... !changes', array('!changes' => $change_list_html))

Some !placeholders are suggested by the regex but not replaced by the patch:
Placeholder contains a URL in:

• syslog.module
• tour.module
• book.module

Placeholder used in non-html context:

• syslog.module (placeholders for syslog output)
• MailManagerInterface.php (mail content)
• contact.module (mail content)
• update.module (mail content)

! not used as placeholder

• Datelist.php
• StringFilter.php

+++ b/core/modules/user/user.api.php
@@ -122,7 +122,7 @@ function hook_user_cancel_methods_alter(&$methods) {
diff --git a/core/modules/views/src/Plugin/Derivative/ViewsBlock.php b/core/modules/views/src/Plugin/Derivative/ViewsBlock.php

diff --git a/core/modules/views/src/Plugin/Derivative/ViewsBlock.php b/core/modules/views/src/Plugin/Derivative/ViewsBlock.php
index 3ebfc5f..9b0947c 100644

index 3ebfc5f..9b0947c 100644
--- a/core/modules/views/src/Plugin/Derivative/ViewsBlock.php

--- a/core/modules/views/src/Plugin/Derivative/ViewsBlock.php
+++ b/core/modules/views/src/Plugin/Derivative/ViewsBlock.php

+++ b/core/modules/views/src/Plugin/Derivative/ViewsBlock.php
+++ b/core/modules/views/src/Plugin/Derivative/ViewsBlock.php
@@ -93,10 +93,10 @@ public function getDerivativeDefinitions($base_plugin_definition) {

@@ -93,10 +93,10 @@ public function getDerivativeDefinitions($base_plugin_definition) {
 
           if (empty($desc)) {
             if ($display->display['display_title'] == $display->definition['title']) {
-              $desc = t('!view', array('!view' => $view->label()));
+              $desc = t('@view', array('@view' => $view->label()));
             }
             else {
-              $desc = t('!view: !display', array('!view' => $view->label(), '!display' => $display->display['display_title']));
+              $desc = t('@view: @display', array('@view' => $view->label(), '@display' => $display->display['display_title']));
             }

Remove. Is handled in #2567159: Fix improper usage of t() in ViewsBlock

The blocker is in now.

Thanks @Sutharsan for the starter patch and the information on the unmatched lines.

I'm assigning this one to myself to read through and make recommendations on how we can break up the scope a bit.

Taking some notes while I look through the starter patch on how to split it up. I'll try to summarize afterward.

1. +++ b/core/includes/install.inc
   @@ -558,7 +558,7 @@ function drupal_verify_profile($install_state) {
   -      'description' => t('The following modules are required but were not found. Move them into the appropriate modules subdirectory, such as <em>/modules</em>. Missing modules: !modules', array('!modules' => implode(', ', $modules))),
   +      'description' => t('The following modules are required but were not found. Move them into the appropriate modules subdirectory, such as <em>/modules</em>. Missing modules: @modules', array('@modules' => implode(', ', $modules))),
   

   This one is fine.

2. +++ b/core/lib/Drupal/Core/Config/ConfigImporter.php
   @@ -762,7 +762,7 @@ protected function processConfiguration($collection, $op, $name) {
   -      $this->logError($this->t('Unexpected error during import with operation @op for @name: !message', array('@op' => $op, '@name' => $name, '!message' => $e->getMessage())));
   +      $this->logError($this->t('Unexpected error during import with operation @op for @name: @message', array('@op' => $op, '@name' => $name, '@message' => $e->getMessage())));
   

   This one is probably okay, since the message is probably safe on its own, but we should double-check and maybe look for other similar uses in the patch with error logging.

3. +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php
   @@ -122,10 +122,10 @@ protected function checkEncoding() {
   -        $this->fail(t('The %driver database must use %encoding encoding to work with Drupal. Recreate the database with %encoding encoding. See !link for more details.', array(
   +        $this->fail(t('The %driver database must use %encoding encoding to work with Drupal. Recreate the database with %encoding encoding. See @link for more details.', array(
   ...
   -          '!link' => '<a href="INSTALL.pgsql.txt">INSTALL.pgsql.txt</a>'
   +          '@link' => '<a href="INSTALL.pgsql.txt">INSTALL.pgsql.txt</a>'
   

   This one will cause double escaping as it is. Let's split this one out 'cause it's sort of a weird used of t() and we may want to discuss how to change the translation.

4. +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php
   @@ -172,9 +172,9 @@ function checkBinaryOutput() {
   -            '!query' => "<code>" . $query . "

   ",
   + '@query' => "" . $query . "",
   ...
   - $this->fail(t("The %setting setting is currently set to '%current_value', but needs to be '%needed_value'. Change this by running the following query: !query", $replacements));
   + $this->fail(t("The %setting setting is currently set to '%current_value', but needs to be '%needed_value'. Change this by running the following query: @query", $replacements));

   @@ -225,9 +225,9 @@ public function checkStandardConformingStrings() {
   - '!query' => "" . $query . "",
   + '@query' => "" . $query . "",
   ...
   - $this->fail(t("The %setting setting is currently set to '%current_value', but needs to be '%needed_value'. Change this by running the following query: !query", $replacements));
   + $this->fail(t("The %setting setting is currently set to '%current_value', but needs to be '%needed_value'. Change this by running the following query: @query", $replacements));
   

   These will cause double-escaping. We can fix this by putting the code tag in the string itself; no idea why it's not. Let's put these and any others like them in another issue.

5. +++ b/core/lib/Drupal/Core/Datetime/Element/Datetime.php
   @@ -241,7 +241,7 @@ public static function processDatetime(&$element, FormStateInterface $form_state
   -        'title' => t('Date (e.g. !format)', array('!format' => static::formatExample($date_format))),
   +        'title' => t('Date (e.g. @format)', array('@format' => static::formatExample($date_format))),
   
   @@ -288,7 +288,7 @@ public static function processDatetime(&$element, FormStateInterface $form_state
   -        'title' => t('Time (e.g. !format)', array('!format' => static::formatExample($time_format))),
   +        'title' => t('Time (e.g. @format)', array('@format' => static::formatExample($time_format))),
   
   +++ b/core/lib/Drupal/Core/Extension/module.api.php
   @@ -946,7 +946,7 @@ function hook_requirements($phase) {
   -      $requirements['cron']['value'] = t('Last run !time ago', array('!time' => \Drupal::service('date.formatter')->formatTimeDiffSince($cron_last)));
   +      $requirements['cron']['value'] = t('Last run @time ago', array('@time' => \Drupal::service('date.formatter')->formatTimeDiffSince($cron_last)));
   
   +++ b/core/modules/system/system.install
   @@ -464,7 +464,7 @@ function system_requirements($phase) {
   -    $summary = t('Last run !time ago', array('!time' => \Drupal::service('date.formatter')->formatTimeDiffSince($cron_last)));
   +    $summary = t('Last run @time ago', array('@time' => \Drupal::service('date.formatter')->formatTimeDiffSince($cron_last)));
   
   +++ b/core/modules/tracker/tracker.pages.inc
   @@ -106,8 +106,8 @@ function tracker_page($account = NULL) {
   -          'data' => t('!time ago', array(
   -            '!time' => \Drupal::service('date.formatter')->formatTimeDiffSince($node->last_activity),
   +          'data' => t('@time ago', array(
   +            '@time' => \Drupal::service('date.formatter')->formatTimeDiffSince($node->last_activity),
   

   I think in all three of these the things passed to the @placeholder are translated, and therefore returning safe strings or string objects, so these should be okay, but someone should probably confirm that.

6. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/EntityReferenceAutocompleteWidget.php
   @@ -73,7 +73,7 @@ public function settingsSummary() {
   -    $summary[] = t('Textfield size: !size', array('!size' => $this->getSetting('size')));
   +    $summary[] = t('Textfield size: @size', array('@size' => $this->getSetting('size')));
   
   +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/StringTextfieldWidget.php
   @@ -60,7 +60,7 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
   -    $summary[] = t('Textfield size: !size', array('!size' => $this->getSetting('size')));
   +    $summary[] = t('Textfield size: @size', array('@size' => $this->getSetting('size')));
   
   +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/UriWidget.php
   @@ -60,7 +60,7 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
   -    $summary[] = $this->t('URI field size: !size', array('!size' => $this->getSetting('size')));
   +    $summary[] = $this->t('URI field size: @size', array('@size' => $this->getSetting('size')));
   

   Why would this size ever have had !placeholder in the first place? May need a little quick research.

7. +++ b/core/lib/Drupal/Core/Form/FormValidator.php
   @@ -296,7 +296,7 @@ protected function doValidateForm(&$elements, FormStateInterface &$form_state, $
   -          $form_state->setError($elements, $this->t('!name field is required.', array('!name' => $elements['#title'])));
   +          $form_state->setError($elements, $this->t('@name field is required.', array('@name' => $elements['#title'])));
   

   I think all of these form validation errors with the #title element should get their own issue. (It was a big swath of the tests too.)

8. +++ b/core/lib/Drupal/Core/Form/form.api.php
   @@ -112,8 +112,8 @@ function callback_batch_operation($MULTIPLE_PARAMS, &$context) {
   -    $message = t("!count items were processed.", array(
   -      '!count' => count($results),
   +    $message = t("@count items were processed.", array(
   +      '@count' => count($results),
   

   This one is fine; obviously no reason for it to use !placeholder.

9. +++ b/core/lib/Drupal/Core/Render/Element/Table.php
   @@ -200,7 +200,7 @@ public static function processTable(&$element, FormStateInterface $form_state, &
   -              $title = t('Update !title', array('!title' => $title));
   +              $title = t('Update @title', array('@title' => $title));
   

   This one is a handful; it's sometimes a #title and sometimes a #markup and eek. Own issue maybe.

10. +++ b/core/lib/Drupal/Core/Updater/Updater.php
    @@ -257,7 +257,7 @@ public function update(&$filetransfer, $overrides = array()) {
    -      throw new UpdaterFileTransferException(t('File Transfer failed, reason: !reason', array('!reason' => strtr($e->getMessage(), $e->arguments))));
    +      throw new UpdaterFileTransferException(t('File Transfer failed, reason: @reason', array('@reason' => strtr($e->getMessage(), $e->arguments))));
    
    @@ -295,7 +295,7 @@ public function install(&$filetransfer, $overrides = array()) {
    -      throw new UpdaterFileTransferException(t('File Transfer failed, reason: !reason', array('!reason' => strtr($e->getMessage(), $e->arguments))));
    +      throw new UpdaterFileTransferException(t('File Transfer failed, reason: @reason', array('@reason' => strtr($e->getMessage(), $e->arguments))));
    

    Exception messages should not be translated. Lets' split these and any other translated exception messages in the patch out into their own issue.

11. +++ b/core/modules/aggregator/src/Entity/Feed.php
    @@ -218,7 +218,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
    -      ->setDescription(t("The parent website's description that comes from the !description element in the feed.", array('!description' => '<description>')));
    +      ->setDescription(t("The parent website's description that comes from the @description element in the feed.", array('@description' => '<description>')));
    

    This looks like it was broken in the first place. Single escaping would be the correct thing to do here. So I think this is okay if someone confirms that.

12. +++ b/core/modules/aggregator/src/FeedViewBuilder.php
    @@ -114,7 +114,7 @@ public function buildComponents(array &$build, array $entities, array $displays,
    -          '#title' => t('!title feed', array('!title' => $entity->label())),
    +          '#title' => t('@title feed', array('@title' => $entity->label())),
    

    Entity labels and #title. Tricksy. These probably also want their own issue (separate from the form one).

13. +++ b/core/modules/aggregator/src/Tests/AggregatorTestBase.php
    @@ -66,7 +66,7 @@ protected function setUp() {
    -    $this->assertRaw(t('The feed %name has been added.', array('%name' => $edit['title[0][value]'])), format_string('The feed !name has been added.', array('!name' => $edit['title[0][value]'])));
    +    $this->assertRaw(t('The feed %name has been added.', array('%name' => $edit['title[0][value]'])), format_string('The feed @name has been added.', array('@name' => $edit['title[0][value]'])));
    

    Let's remove all format_string() from this patch and do t() only please.

14. +++ b/core/modules/book/src/BookManager.php
    @@ -357,7 +357,7 @@ protected function addParentSelectFormElements(array $book_link) {
    -        '#description' => $this->t('The parent page in the book. The maximum depth for a book and all child pages is !maxdepth. Some pages in the selected book may not be available as parents if selecting them would exceed this limit.', array('!maxdepth' => static::BOOK_MAX_DEPTH)),
    +        '#description' => $this->t('The parent page in the book. The maximum depth for a book and all child pages is @maxdepth. Some pages in the selected book may not be available as parents if selecting them would exceed this limit.', array('@maxdepth' => static::BOOK_MAX_DEPTH)),
    

    Absolutely no reason for this to have ever been !placeholder, so this one is fine to include in this patch.

15. +++ b/core/modules/comment/comment.module
    @@ -717,8 +717,8 @@ function template_preprocess_comment(&$variables) {
    -    $variables['parent'] = t('In reply to !parent_title by !parent_username',
    -        array('!parent_username' => $variables['parent_author'], '!parent_title' => $variables['parent_title']));
    +    $variables['parent'] = t('In reply to @parent_title by @parent_username',
    +        array('@parent_username' => $variables['parent_author'], '@parent_title' => $variables['parent_title']));
    

    Preprocess function. This one should maybe have its own issue. We had some fun with rdf_preprocess_comment() when removing SafeMarkup::set().

16. +++ b/core/modules/config/src/Form/ConfigSync.php
    @@ -237,7 +237,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
    -          '#value' => $this->t('!collection configuration collection', array('!collection' => $collection)),
    +          '#value' => $this->t('@collection configuration collection', array('@collection' => $collection)),
    

    It took a bit of digging, but I found:

    he collection name. Valid collection names conform to the following regex [a-zA-Z_.]

    (That's from StorageInterface::createCollection().)

    So this one is also fine to convert here since no special characters are possible.

17. +++ b/core/modules/config_translation/config_translation.api.php
    @@ -53,7 +53,7 @@ function hook_config_translation_info(&$info) {
    -          'title' => t('!label field'),
    +          'title' => t('@label field'),
    
    +++ b/core/modules/config_translation/src/ConfigEntityMapper.php
    @@ -158,7 +158,7 @@ public function getTitle() {
    -    return $this->t($this->pluginDefinition['title'], array('!label' => $this->entity->label(), '!entity_type' => $entity_type_info->getLowercaseLabel()));
    +    return $this->t($this->pluginDefinition['title'], array('@label' => $this->entity->label(), '@entity_type' => $entity_type_info->getLowercaseLabel()));
    
    +++ b/core/modules/config_translation/tests/src/Unit/ConfigEntityMapperTest.php
    @@ -70,7 +70,7 @@ protected function setUp() {
    -      'title' => '!label language',
    +      'title' => '@label language',
    
    +++ b/core/modules/config_translation/tests/src/Unit/ConfigFieldMapperTest.php
    @@ -50,7 +50,7 @@ protected function setUp() {
    -      'title' => '!label field',
    +      'title' => '@label field',
    

    This one is more entity labels, so maybe with that other issue? Also a line of docs. We should evaluate the whole hook example.

18. +++ b/core/modules/config_translation/src/FormElement/FormElementBase.php
    @@ -100,9 +100,9 @@ protected function getSourceElement(LanguageInterface $source_language, $source_
    -      '#title' => $this->t('!label <span class="visually-hidden">(!source_language)</span>', array(
    -        '!label' => $this->t($this->definition->getLabel()),
    -        '!source_language' => $source_language->getName(),
    +      '#title' => $this->t('@label <span class="visually-hidden">(@source_language)</span>', array(
    +        '@label' => $this->t($this->definition->getLabel()),
    +        '@source_language' => $source_language->getName(),
    
    @@ -162,9 +162,9 @@ protected function getSourceElement(LanguageInterface $source_language, $source_
    -      '#title' => $this->t('!label <span class="visually-hidden">(!source_language)</span>', array(
    -        '!label' => $this->t($this->definition['label']),
    -        '!source_language' => $translation_language->getName(),
    +      '#title' => $this->t('@label <span class="visually-hidden">(@source_language)</span>', array(
    +        '@label' => $this->t($this->definition['label']),
    +        '@source_language' => $translation_language->getName(),
    

    More #title and entity labels. Also this one looks like it's mixing templating with translator context. Maybe its own separate issue for that reason, though it'd need whatever info from the main entity label issue.

19. +++ b/core/modules/config_translation/tests/src/Unit/ConfigEntityMapperTest.php
    @@ -70,7 +70,7 @@ protected function setUp() {
    -      'title' => '!label language',
    +      'title' => '@label language',
    
    +++ b/core/modules/config_translation/tests/src/Unit/ConfigFieldMapperTest.php
    @@ -50,7 +50,7 @@ protected function setUp() {
    -      'title' => '!label field',
    +      'title' => '@label field',
    

    More entity labels. You get the idea.

20. +++ b/core/modules/contact/src/MailHandler.php
    @@ -91,7 +91,7 @@ public function sendMailMessages(MessageInterface $message, AccountInterface $se
    -      $sender_cloned->name = $this->t('!name (not verified)', array('!name' => $message->getSenderName()));
    +      $sender_cloned->name = $this->t('@name (not verified)', array('@name' => $message->getSenderName()));
    

    This is in the MailHandler so a non-HTML use I think, so should not be included.

21. +++ b/core/modules/file/src/Element/ManagedFile.php
    @@ -372,19 +372,19 @@ public static function validateManagedFile(&$element, FormStateInterface $form_s
    -              $form_state->setError($element, t('The file used in the !name field may not be referenced.', ['!name' => $element['#title']]));
    +              $form_state->setError($element, t('The file used in the @name field may not be referenced.', ['@name' => $element['#title']]));
    

    More #title, used in another string this time.

22. +++ b/core/modules/migrate/src/MigrateExecutable.php
    @@ -159,8 +159,8 @@ public function __construct(MigrationInterface $migration, MigrateMessageInterfa
    -            $this->message->display($this->t('Invalid PHP memory_limit !limit, setting to unlimited.',
    -              array('!limit' => $limit)));
    +            $this->message->display($this->t('Invalid PHP memory_limit @limit, setting to unlimited.',
    +              array('@limit' => $limit)));
    
    @@ -462,10 +462,10 @@ protected function memoryExceeded() {
    -        $this->t('Memory usage is !usage (!pct% of limit !limit), reclaiming memory.',
    -          array('!pct' => round($pct_memory*100),
    -                '!usage' => $this->formatSize($usage),
    -                '!limit' => $this->formatSize($this->memoryLimit))),
    +        $this->t('Memory usage is @usage (@pct% of limit @limit), reclaiming memory.',
    +          array('@pct' => round($pct_memory*100),
    +                '@usage' => $this->formatSize($usage),
    +                '@limit' => $this->formatSize($this->memoryLimit))),
    
    @@ -473,19 +473,19 @@ protected function memoryExceeded() {
    -          $this->t('Memory usage is now !usage (!pct% of limit !limit), not enough reclaimed, starting new batch',
    -            array('!pct' => round($pct_memory*100),
    -                  '!usage' => $this->formatSize($usage),
    -                  '!limit' => $this->formatSize($this->memoryLimit))),
    +          $this->t('Memory usage is now @usage (@pct% of limit @limit), not enough reclaimed, starting new batch',
    +            array('@pct' => round($pct_memory*100),
    +                  '@usage' => $this->formatSize($usage),
    +                  '@limit' => $this->formatSize($this->memoryLimit))),
    ...
    -          $this->t('Memory usage is now !usage (!pct% of limit !limit), reclaimed enough, continuing',
    -            array('!pct' => round($pct_memory*100),
    -                  '!usage' => $this->formatSize($usage),
    -                  '!limit' => $this->formatSize($this->memoryLimit))),
    +          $this->t('Memory usage is now @usage (@pct% of limit @limit), reclaimed enough, continuing',
    +            array('@pct' => round($pct_memory*100),
    +                  '@usage' => $this->formatSize($usage),
    +                  '@limit' => $this->formatSize($this->memoryLimit))),
    

    format_size() returns translatable strings which will in the safe list or safe string objects later on, so these are fine. Edit: @alexpott said something this morning about this being not as simple as it looks, so these should probably get their own issue.

23. +++ b/core/modules/migrate/src/MigrateExecutable.php
    @@ -237,7 +237,7 @@ public function import() {
    -        $this->t('Migration failed with source plugin exception: !e', array('!e' => $e->getMessage())), 'error');
    +        $this->t('Migration failed with source plugin exception: @e', array('@e' => $e->getMessage())), 'error');
    
    @@ -318,8 +318,8 @@ public function import() {
    -          $this->t('Migration failed with source plugin exception: !e',
    -            array('!e' => $e->getMessage())), 'error');
    +          $this->t('Migration failed with source plugin exception: @e',
    +            array('@e' => $e->getMessage())), 'error');
    

    These are more putting of error messages into other strings; can probably go with that one from the first hunk.

24. +++ b/core/modules/search/search.module
    @@ -776,7 +776,7 @@ function search_excerpt($keys, $text, $langcode = NULL) {
    -  $ellipses = explode('!excerpt', t('… !excerpt … !excerpt …'));
    +  $ellipses = explode('@excerpt', t('… @excerpt … @excerpt …'));
    

    Search excerpts are a whole complicated thing. Let's give this one its own issue for proper manual testing and maybe some fixer-upper.

25. +++ b/core/modules/system/src/Controller/SystemController.php
    @@ -231,8 +231,8 @@ public function themesPage() {
    -            'alt' => $this->t('Screenshot for !theme theme', array('!theme' => $theme->info['name'])),
    -            'title' => $this->t('Screenshot for !theme theme', array('!theme' => $theme->info['name'])),
    +            'alt' => $this->t('Screenshot for @theme theme', array('@theme' => $theme->info['name'])),
    +            'title' => $this->t('Screenshot for @theme theme', array('@theme' => $theme->info['name'])),
    

    More opportunity for extension authors to inject XSS by putting XSS in their extension names. ;) Yahuh. All these SystemController theme name ones should be fine I think but it could be its own patch.

26. +++ b/core/modules/system/src/Form/ModulesListForm.php
    @@ -353,8 +353,8 @@ protected function buildRow(array $modules, Extension $module, $distribution) {
    -      $reasons[] = $this->t('This version is not compatible with Drupal !core_version and should be replaced.', array(
    -        '!core_version' => \Drupal::CORE_COMPATIBILITY,
    +      $reasons[] = $this->t('This version is not compatible with Drupal @core_version and should be replaced.', array(
    +        '@core_version' => \Drupal::CORE_COMPATIBILITY,
    

    This one is fine.

27. +++ b/core/modules/system/src/Form/ModulesListForm.php
    @@ -362,9 +362,9 @@ protected function buildRow(array $modules, Extension $module, $distribution) {
    -      $reasons[] = $this->t('This module requires PHP version @php_required and is incompatible with PHP version !php_version.', array(
    +      $reasons[] = $this->t('This module requires PHP version @php_required and is incompatible with PHP version @php_version.', array(
    ...
    -        '!php_version' => phpversion(),
    +        '@php_version' => phpversion(),
    
    +++ b/core/modules/system/system.admin.inc
    @@ -412,7 +412,7 @@ function template_preprocess_system_themes_page(&$variables) {
    -        $current_theme['incompatible'] = t('This theme requires PHP version @php_required and is incompatible with PHP version !php_version.', array('@php_required' => $theme->info['php'], '!php_version' => phpversion()));
    +        $current_theme['incompatible'] = t('This theme requires PHP version @php_required and is incompatible with PHP version @php_version.', array('@php_required' => $theme->info['php'], '@php_version' => phpversion()));
    

    For these, I think maybe the version requirements can contain angle brackets, e.g. > 5.5.2 or whatever? So single escaping is probably correct, but we should test that there is no double escaping. Could be its own little issue.

28. +++ b/core/modules/system/system.admin.inc
    @@ -218,13 +218,13 @@ function theme_system_modules_details($variables) {
    -      $description .= '<div class="admin-requirements">' . t('Version: !module-version', array('!module-version' => drupal_render($module['version']))) . '</div>';
    +      $description .= '<div class="admin-requirements">' . t('Version: @module-version', array('@module-version' => drupal_render($module['version']))) . '</div>';
         }
         if ($requires) {
    -      $description .= '<div class="admin-requirements">' . t('Requires: !module-list', array('!module-list' => implode(', ', $module['#requires']))) . '</div>';
    +      $description .= '<div class="admin-requirements">' . t('Requires: @module-list', array('@module-list' => implode(', ', $module['#requires']))) . '</div>';
         }
         if ($required_by) {
    -      $description .= '<div class="admin-requirements">' . t('Required by: !module-list', array('!module-list' => implode(', ', $module['#required_by']))) . '</div>';
    +      $description .= '<div class="admin-requirements">' . t('Required by: @module-list', array('@module-list' => implode(', ', $module['#required_by']))) . '</div>';
         }
    

    Yowza at all these. Let's give them their own issue.

29. +++ b/core/modules/text/src/Plugin/Field/FieldWidget/TextareaWithSummaryWidget.php
    @@ -57,7 +57,7 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
    -    $summary[] = t('Number of summary rows: !rows', array('!rows' => $this->getSetting('summary_rows')));
    +    $summary[] = t('Number of summary rows: @rows', array('@rows' => $this->getSetting('summary_rows')));
    

    This also looks like an integer.

30. +++ b/core/modules/tour/src/TourViewBuilder.php
    @@ -44,7 +44,7 @@ public function viewMultiple(array $entities = array(), $view_mode = 'full', $la
    -              '#children' => t('!tour_item of !total', array('!tour_item' => $index + 1, '!total' => $count)),
    +              '#children' => t('@tour_item of @total', array('@tour_item' => $index + 1, '@total' => $count)),
    

    Integers; no reason to use !placeholder. So these are fine to include in the patch.

31. +++ b/core/modules/user/user.api.php
    @@ -122,7 +122,7 @@ function hook_user_cancel_methods_alter(&$methods) {
    -    $name = t('User !uid', array('!uid' => $account->id()));
    +    $name = t('User @uid', array('@uid' => $account->id()));
    

    This'd normally be a serial ID unless someone is implementing a weird storage, and this is just hook docs, so this change is fine.

32. +++ b/core/modules/views/src/Plugin/Derivative/ViewsBlock.php
    @@ -93,10 +93,10 @@ public function getDerivativeDefinitions($base_plugin_definition) {
    -              $desc = t('!view', array('!view' => $view->label()));
    +              $desc = t('@view', array('@view' => $view->label()));
                 }
                 else {
    -              $desc = t('!view: !display', array('!view' => $view->label(), '!display' => $display->display['display_title']));
    +              $desc = t('@view: @display', array('@view' => $view->label(), '@display' => $display->display['display_title']));
    

    This hunk can go since it got fixed elsewhere, as mentioned above.

33. +++ b/core/modules/views/src/Plugin/views/HandlerBase.php
    @@ -185,7 +185,7 @@ public function adminLabel($short = FALSE) {
    -    return $this->t('!group: !title', array('!group' => $this->definition['group'], '!title' => $title));
    +    return $this->t('@group: @title', array('@group' => $this->definition['group'], '@title' => $title));
    

    Oh Views. Why so many !placeholder. This is why I started with Views first back a few months ago. ;) I'm tempted to say leave Views out of the intial patch entirely, actually, so we can take a more general look at everything that's going on there. We could just resurrect the original views issue thta I closed as a dupe for that.

Phew!

Okay, so let's start by getting some split off issues for:

1. core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php (those two seem okay to combine in one patch).
2. $e->getMessage() inside another string.
3. Date stuff.
4. #title stuff.
5. Entity labels.
6. template_preprocess_comment()
7. Theme name stuff in SystemController
8. theme_system_modules_details()
9. search_excerpt()
10. ...And then resurrecting the Views issue, but postponing it: #2506479: Replace !placeholder with @placeholder for non URLs in t() in Views, except for t() output that is used as an attribute value

Someone else can file those 9 issues in #10. :) Postponing this one until some of the targeted fixes get in to reduce reroll hell.

Updating summary with the scoping recommendations.

(Fixing the grep command a bit more.)

#2506427-63: [meta] !placeholder causes strings to be escaped and makes the sanitization API harder to understand lists the 3 behaviors of ! that 8.0 won't be released with. While there's still some debate on that issue on how to proceed for URL usages and non-HTML usages, there is no debate that we must fix all of core non-url HTML outputs to work correctly with @. Therefore, raising this to Critical.

Also, unpostponing to make clear that there's an actionable next step, which is to file the child issues and start rolling patches for them, per the issue summary.

.

When working through the child issues, where it's non-onerous, we should:

• Document existing test coverage or add a test if it's missing
• Manually test in most cases that are not trivially obvious, with a screenshot, depending on fix
• If the code is just kinda wrong, create an @todo/followup so that developers can actually find the right way to do it rather than being confused by bad patterns

Separating out a section for issues still to file.

Looking at #5, we could also file issues for ConfigSync (non-best-practice of t()), the system system.admin.inc machine name/markup thing (that's an interesting use of directional markup), AccountForm (another non-best-practice use of t()) and contextual.module (that one looks like it needs some specific focus, because weird).

BreakLockForm should be equivalent for ! or @ since it's a drupal_render() for the username right there inline (...we seem to early render usernames a lot).

There we go. Some of them certainly have not the best title / wrong component but well, feel free to improve as you go.

@justAChris
This is super helpful, especially once we provide an update path for them, which be should better do, I guess?

We probably need a postponed critical to create some kind of update functions for all of them, I think.

I'm actually convinced that we need an update path for all the strings, which should be critical. In general it is a good idea to list the strings in a non critical meta whatever issue.

Outside of Views and the test coverage for !placeholder itself, we still have this usage that should get an issue filed when grepping:

core/modules/contact/src/Tests/ContactPersonalTest.php:    $this->assertEqual($mail['subject'], t('[!site-name] !subject', $variables), 'Subject is in sent message.');

These are the strings that potx module found in current HEAD (general.pot.txt). Also adding an error file of strings that potx excluded mostly because t() contains a variable and not a string.

Added a new issue, but I think its not critical #2575599: Remove !placeholder in ContactPersonalTest and Drupal\migrate\Plugin\migrate\id_map\Sql

Thanks, looking through that list and found one more, trivial case:

#: /www/drupal8/docroot/core/modules/migrate/src/Plugin/migrate/id_map/Sql.php:509
msgid "Could not save to map table due to NULL value for key field !field"
msgstr ""

This might be a valid test:

#: /www/drupal8/docroot/core/modules/system/src/Tests/Common/XssUnitTest.php:43
msgid "Verbatim text: !value"
msgstr ""

The only other is syslog (non HTML output) and ViewsEditForm which is being fixed in one of the remaining issues.

Looks like we're almost there!

This is a grepped list. I'm trying to address it in #2571695: Remove !placeholder and unsafe string return from SafeMarkup::format().

Please, stick this time.

We should be done here.

Discussed with @plach, @berdir, and @dawehner. We aren't quite entirely done. There is:

• One usage in Migrate's Sql.php on line 509
• The usage in #2575599: Remove !placeholder in ContactPersonalTest and Drupal\migrate\Plugin\migrate\id_map\Sql that got rolled into #2571695: Remove !placeholder and unsafe string return from SafeMarkup::format() atm to make tests pass there.
• We've agreed informally that removing the placeholder in JS is not critical, but AFAIK this isn't discussed anywhere at all in the issue queue. So we should file a major plan for that. One related issue I know of is #2567727: LocaleJavascriptTranslationTest needs test coverage for @placeholders in JavaScript.

However #2506427: [meta] !placeholder causes strings to be escaped and makes the sanitization API harder to understand is still open, so I will just move those outstanding tasks over there.