Problem/Motivation

See #2566503: [meta] Replace remaining !placeholder for Non-URL HTML outputs only

modules/image/image.views.inc:      'label' => t('image from !field_name', array('!field_name' => $field_storage->getName())),
modules/image/src/Plugin/Field/FieldType/ImageItem.php:          $form_state->setError($element[$dimension], t('Both a height and width value must be specified in the !name field.', array('!name' => $element['#title'])));

Proposed resolution

Remaining tasks

Agree that removing HTML support makes sense.

User interface changes

None

API changes

Date format strings no longer support adding HTML using the \ escape character.

Data model changes

None

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because at the moment date formats support HTML but it is escaped
Issue priority Major because part of the critical to remove !placeholder
Disruption Disruptive for existing sites that are adding HTML to date formats. If HTML is required in a formatted date then the site should implement a custom field formatter to do this.
CommentFileSizeAuthor
#4 2571945-4.patch1.66 KBswentel

Comments

dawehner created an issue. See original summary.

dawehner’s picture

Issue summary: View changes
dawehner’s picture

Title: Remove !placeholder in image.views.inc » Remove !placeholder in image module
Issue summary: View changes
swentel’s picture

Status: Active » Needs review
StatusFileSize
new1.66 KB
dawehner’s picture

Status: Needs review » Needs work
+++ b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
@@ -383,7 +383,9 @@ public static function validateResolution($element, FormStateInterface $form_sta
+          // here, so it won't be escaped again as it's already marked safe.
+          $form_state->setError($element[$dimension], t('Both a height and width value must be specified in the @name field.', array('@name' => $element['#title'])));
           return;

At that point I would not like us to assume that this is a string but rather call render() which distincts between render methods and pure strings

swentel’s picture

pwolanin’s picture

Since we are converting from ! to @ here, I think supporting render arrays is out of scope. This looks fine to me, except this comment needs work:

+          // We expect the field name placeholder value to be wrapped in t()
+          // here, so it won't be escaped again as it's already marked safe.

You could change to something more like:

+          // We expect the field name placeholder value to be wrapped in a
+          // TranslatableString object, so it won't be escaped again.
dawehner’s picture

Status: Needs work » Reviewed & tested by the community

Discussion with alex

  • alexpott committed 10aa780 on 8.0.x
    Issue #2571945 by swentel: Remove !placeholder in image module
    
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 10aa780 and pushed to 8.0.x. Thanks!

Status: Fixed » Needs work

The last submitted patch, 4: 2571945-4.patch, failed testing.

stefan.r’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.