Fix use of !placeholder for imploding in views.views.inc

will give it a try this night

Thank you @izus, just missing one more set for !display_title.

ehhh I think we got the issue summary wrong here? This is all already being addressed in #2506479: Replace !placeholder with @placeholder for non URLs in t() in Views, except for t() output that is used as an attribute value, we're merely splitting out the comma implode, which is postponed on having a helper for suffixing strings

Let's be clear

Blocked by #2509218: Ensure that SafeString objects can be used in non-HTML contexts

Sorry there was a confusion on IRC about what this is blocked by.

Discussed with @alexpott and we can actually use htmlspecialchars_decode here as a workaround - it should be the only place in core where we do this. The help key is schema-type information that ought to be plain-text only, as it is used both to be plan-text help information as well as to be UI help text.

Here is an additional integration level testing.

+++ b/core/modules/views_ui/src/Tests/HandlerTest.php
@@ -137,6 +139,40 @@ public function testUICRUD() {
+      'label' => 'The giraffe" label <script>alert("the return of the xss")</script>'
...
+    $this->assertText('Appears in: page, article. Also known as: Content: The giraffe" label alert("the return of the xss")');

It is odd that this is Xss filtered and not escaped as I would have expected.

Maybe because htmlspecialchars_decode() undoes quotes but also turns <script> into <script>, which gets filtered out whenever the field help text goes into #markup... @dawehner is that possible? I could look with a debugger?

@alexpott
The reason is the following:

          $form['options']['name'][$key] = array(
            '#type' => 'checkbox',
            '#title' => $this->t('@group: @field', array('@group' => $option['group'], '@field' => $option['title'])),
            '#description' => $option['help'],

... Its using #description which is maybe like #markup

Spoke with multiple people about this. It's a necessary evil for now. This is more an oversight/architecture problem in views that our UI string is defined in the views data.

Okay so we have a problem because of the #description which does not auto-escape. It is admin filtered. This makes the documentation added incorrect and it also makes the behaviour incorrect. If you put em tags in the field labels they should always be escaped but they are not here :(

I really don't like the behaviour of #description - it feels like it should auto-escape by default. If we don't want to fix this - and that would be understand since it would be a big change now - I think we have to create another SafeStringInterface object to help building view data.

I've created #2573743: #description should escape by default to explore making #description auto-escape.

I'm not clear how the child is blocking, where specifically would em be a problem?

I think this shows we need better tools for concatenating strings. It's not the first time we encounter this problem?

FWIW we had another patch earlier in #2506479: Replace !placeholder with @placeholder for non URLs in t() in Views, except for t() output that is used as an attribute value containing an array of markup arrays.

If we had #2554073: Allow #markup to be an array of strings and join them safely we'd be able to use just that and generate the help string without markup

+++ b/core/modules/views_ui/src/Tests/HandlerTest.php
@@ -137,6 +139,40 @@ public function testUICRUD() {
+      'label' => 'The giraffe" label <script>alert("the return of the xss")</script>'
...
+    $this->assertText('Appears in: page, article. Also known as: Content: The giraffe" label alert("the return of the xss")');

In every other context the script tags would be escaped. We're stripping them here because #description is admin filtered. The problem we have is that we need to mix markup and escaping - since a translator might need to add HTML to a translation.

I'm working on this.

This approach preserves the escaping and makes the way labels are escaped consistent with other places in the Views and Field UI. With the previous approach if a field label contained em tags they would be escaped in the Field UI but in this help text the em tags would not have been filtered or escaped.

The only reason this works is because #description is XSS admin filtered. And therefore the fact that the final string is not marked safe does not cause double escaping.

+++ b/core/modules/views/views.views.inc
@@ -550,7 +559,17 @@ function views_field_default_views_data(FieldStorageConfigInterface $field_stora
+        $data[$table_alias][$column_real_name]['help'] .= ' ' . t('Also known as:') . ' ' . $also_known_list;

This bit doesn't have tests does it? Do we need the htmlspecialchars_decode()?

Workign on the discussion we had during drupalcon

There we go. We make it a safe string, as we know it is safe.

I think the patches in #38 and #39 are for a different issue?

I think the above concerns have been addressed, the patch in #37 looks good.

Discussed with @dawehner, @xjm, @laurii, @joelpittet, and @stefan.r. It is a shame that we have to do this but creating UI text at the point of building the schema information makes this very very tricky. Committed 36b936f and pushed to 8.0.x. Thanks!