From e76a96f48c19e41e7ef0ca81f8a74c19619e3202 Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Tue, 18 Aug 2015 00:29:35 +0100 Subject: [PATCH] Issue #2363423 by dorficus, Manuel Garcia, stefan.r, Cottser, prajaankit, joelpittet, davidhernandez, alexpott, xjm, dawehner, penyaskito: views-view-fields.html.twig gets escaped --- .../Plugin/views/field/FieldPluginBase.php | 6 +- .../views/src/Tests/ViewsEscapingTest.php | 72 ++++++++++++++ .../templates/views-view-fields.html.twig | 38 +++++--- .../templates/views-view-fields.html.twig | 11 +++ .../views_test_theme.info.yml | 5 + core/modules/views/views.theme.inc | 94 ++++++++++--------- 6 files changed, 166 insertions(+), 60 deletions(-) create mode 100644 core/modules/views/src/Tests/ViewsEscapingTest.php create mode 100644 core/modules/views/tests/themes/views_test_theme/templates/views-view-fields.html.twig create mode 100644 core/modules/views/tests/themes/views_test_theme/views_test_theme.info.yml diff --git a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php index 996ef3120d..310997cbb0 100644 --- a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php @@ -242,7 +242,7 @@ public function elementType($none_supported = FALSE, $default_empty = FALSE, $in } } if ($this->options['element_type']) { - return SafeMarkup::checkPlain($this->options['element_type']); + return $this->options['element_type']; } if ($default_empty) { @@ -270,7 +270,7 @@ public function elementLabelType($none_supported = FALSE, $default_empty = FALSE } } if ($this->options['element_label_type']) { - return SafeMarkup::checkPlain($this->options['element_label_type']); + return $this->options['element_label_type']; } if ($default_empty) { @@ -290,7 +290,7 @@ public function elementWrapperType($none_supported = FALSE, $default_empty = FAL } } if ($this->options['element_wrapper_type']) { - return SafeMarkup::checkPlain($this->options['element_wrapper_type']); + return $this->options['element_wrapper_type']; } if ($default_empty) { diff --git a/core/modules/views/src/Tests/ViewsEscapingTest.php b/core/modules/views/src/Tests/ViewsEscapingTest.php new file mode 100644 index 0000000000..18e4acc581 --- /dev/null +++ b/core/modules/views/src/Tests/ViewsEscapingTest.php @@ -0,0 +1,72 @@ +enableViewsTestModule(); + } + + /** + * Tests for incorrectly escaped markup in the views-view-fields.html.twig. + */ + public function testViewsViewFieldsEscaping() { + // Test with system theme using theme function. + $this->drupalGet('test_page_display_200'); + + // Assert that there are no escaped '<'s characters. + $this->assertNoEscaped('<'); + + // Install theme to test with template system. + \Drupal::service('theme_handler')->install(array('views_test_theme')); + + // Make base theme default then test for hook invocations. + $this->config('system.theme') + ->set('default', 'views_test_theme') + ->save(); + $this->assertEqual($this->config('system.theme')->get('default'), 'views_test_theme'); + + $this->drupalGet('test_page_display_200'); + + // Assert that we are using the correct template. + $this->assertText('force', 'The force is strong with this one'); + + // Assert that there are no escaped '<'s characters. + $this->assertNoEscaped('<'); + } + +} diff --git a/core/modules/views/templates/views-view-fields.html.twig b/core/modules/views/templates/views-view-fields.html.twig index dc39897211..c06c1a86ce 100644 --- a/core/modules/views/templates/views-view-fields.html.twig +++ b/core/modules/views/templates/views-view-fields.html.twig @@ -11,13 +11,16 @@ * - class: The safe class ID to use. * - handler: The Views field handler controlling this field. * - inline: Whether or not the field should be inline. - * - inline_html: Either div or span based on the 'inline' flag. - * - wrapper_prefix: A complete wrapper containing the inline_html to use. - * - wrapper_suffix: The closing tag for the wrapper. + * - wrapper_element: An HTML element for a wrapper. + * - wrapper_attributes: List of attributes for wrapper element. * - separator: An optional separator that may appear before a field. * - label: The field's label text. - * - label_html: The full HTML of the label to use including configured - * element type. + * - label_element: An HTML element for a label wrapper. + * - label_attributes: List of attributes for label wrapper. + * - has_label_colon: A boolean indicating whether to display a colon after + * the label. + * - element_type: An HTML element for the field content. + * - element_attributes: List of attributes for HTML element for field content. * - row: The raw result from the query, with all data it fetched. * * @see template_preprocess_views_view_fields() @@ -31,11 +34,24 @@ See http://api.drupal.org/api/function/theme_views_view_fields/8 for details. After copying this file to your theme's folder and customizing it, remove this comment. #} -{% for field in fields %} +{% for field in fields -%} {{ field.separator }} - - {{ field.wrapper_prefix }} - {{ field.label_html }} + {%- if field.wrapper_element -%} + <{{ field.wrapper_element }}{{ field.wrapper_attributes }}> + {%- endif %} + {%- if field.label -%} + {%- if field.label_element -%} + <{{ field.label_element }}{{ field.label_attributes }}>{{ field.label }}{{ field.has_label_colon ? ': ' }} + {%- else -%} + {{ field.label }}{{ field.has_label_colon ? ': ' }} + {%- endif %} + {%- endif %} + {%- if field.element_type -%} + <{{ field.element_type }}{{ field.element_attributes }}>{{ field.content }} + {%- else -%} {{ field.content }} - {{ field.wrapper_suffix }} -{% endfor %} + {%- endif %} + {%- if field.wrapper_element -%} + + {%- endif %} +{%- endfor %} diff --git a/core/modules/views/tests/themes/views_test_theme/templates/views-view-fields.html.twig b/core/modules/views/tests/themes/views_test_theme/templates/views-view-fields.html.twig new file mode 100644 index 0000000000..075f6ecf26 --- /dev/null +++ b/core/modules/views/tests/themes/views_test_theme/templates/views-view-fields.html.twig @@ -0,0 +1,11 @@ +{# +/** + * @file + * Theme override to display all the fields in a views row. + * + * The reason for this template is to override the theme function provided by + * views. + */ +#} +{% include '@views/views-view-fields.html.twig' %} +May the force be with you! diff --git a/core/modules/views/tests/themes/views_test_theme/views_test_theme.info.yml b/core/modules/views/tests/themes/views_test_theme/views_test_theme.info.yml new file mode 100644 index 0000000000..b32f40ab04 --- /dev/null +++ b/core/modules/views/tests/themes/views_test_theme/views_test_theme.info.yml @@ -0,0 +1,5 @@ +name: Views test theme +type: theme +description: Theme for testing Views functionality. +version: VERSION +core: 8.x diff --git a/core/modules/views/views.theme.inc b/core/modules/views/views.theme.inc index 7728d89f33..4853613c70 100644 --- a/core/modules/views/views.theme.inc +++ b/core/modules/views/views.theme.inc @@ -90,6 +90,9 @@ function template_preprocess_views_view_fields(&$variables) { $object = new stdClass(); $object->handler = $view->field[$id]; $object->inline = !empty($variables['options']['inline'][$id]); + // Set up default value of the flag that indicates whether to display a + // colon after the label. + $object->has_label_colon = FALSE; $object->element_type = $object->handler->elementType(TRUE, !$variables['options']['default_field_elements'], $object->inline); if ($object->element_type) { @@ -101,18 +104,7 @@ function template_preprocess_views_view_fields(&$variables) { if ($classes = $object->handler->elementClasses($row->index)) { $attributes['class'][] = $classes; } - $attributes = new Attribute($attributes); - - $pre = '<' . $object->element_type; - $pre .= $attributes; - $field_output = $pre . '>' . $field_output . 'element_type . '>'; - } - - // Protect ourselves somewhat for backward compatibility. This will - // prevent old templates from producing invalid HTML when no element type - // is selected. - if (empty($object->element_type)) { - $object->element_type = 'span'; + $object->element_attributes = new Attribute($attributes); } $object->content = $field_output; @@ -130,16 +122,14 @@ function template_preprocess_views_view_fields(&$variables) { $object->class = Html::cleanCssIdentifier($id); $previous_inline = $object->inline; - $object->inline_html = $object->handler->elementWrapperType(TRUE, TRUE); - if ($object->inline_html === '' && $variables['options']['default_field_elements']) { - $object->inline_html = $object->inline ? 'span' : 'div'; + // Set up field wrapper element. + $object->wrapper_element = $object->handler->elementWrapperType(TRUE, TRUE); + if ($object->wrapper_element === '' && $variables['options']['default_field_elements']) { + $object->wrapper_element = $object->inline ? 'span' : 'div'; } - // Set up the wrapper HTML. - $object->wrapper_prefix = ''; - $object->wrapper_suffix = ''; - - if ($object->inline_html) { + // Set up field wrapper attributes if field wrapper was set. + if ($object->wrapper_element) { $attributes = array(); if ($object->handler->options['element_default_classes']) { $attributes['class'][] = 'views-field'; @@ -149,26 +139,24 @@ function template_preprocess_views_view_fields(&$variables) { if ($classes = $object->handler->elementWrapperClasses($row->index)) { $attributes['class'][] = $classes; } - $attributes = new Attribute($attributes); - - $object->wrapper_prefix = '<' . $object->inline_html; - $object->wrapper_prefix .= $attributes; - $object->wrapper_prefix .= '>'; - $object->wrapper_suffix = 'inline_html . '>'; + $object->wrapper_attributes = new Attribute($attributes); } - // Set up the label for the value and the HTML to make it easier - // on the template. - $object->label = SafeMarkup::checkPlain($view->field[$id]->label()); - $object->label_html = ''; + // Set up field label + $object->label = $view->field[$id]->label(); + + // Set up field label wrapper and its attributes. if ($object->label) { - $object->label_html .= $object->label; + // Add a colon in a label suffix. if ($object->handler->options['element_label_colon']) { - $object->label_html .= ': '; + $object->has_label_colon = TRUE; } - $object->elementLabelType = $object->handler->elementLabelType(TRUE, !$variables['options']['default_field_elements']); - if ($object->elementLabelType) { + // Set up label HTML element. + $object->label_element = $object->handler->elementLabelType(TRUE, !$variables['options']['default_field_elements']); + + // Set up label attributes. + if ($object->label_element) { $attributes = array(); if ($object->handler->options['element_default_classes']) { $attributes['class'][] = 'views-label'; @@ -179,13 +167,7 @@ function template_preprocess_views_view_fields(&$variables) { if ($element_label_class) { $attributes['class'][] = $element_label_class; } - $attributes = new Attribute($attributes); - - $pre = '<' . $object->elementLabelType; - $pre .= $attributes; - $pre .= '>'; - - $object->label_html = $pre . $object->label_html . 'elementLabelType . '>'; + $object->label_attributes = new Attribute($attributes); } } @@ -219,12 +201,32 @@ function theme_views_view_fields($variables) { if (!empty($field->separator)) { $output .= $field->separator; } - - $output .= $field->wrapper_prefix; - $output .= $field->label_html; + $wrapper_element = Html::escape($field->wrapper_element); + if ($wrapper_element) { + $output .= '<' . $wrapper_element . $field->wrapper_attributes . '>'; + if (isset($field->label_element) && !empty($field->label_element)) { + $label_element = Html::escape($field->label_element); + $output .= '<' . $label_element . $field->label_attributes . '>'; + } + $output .= Html::escape($field->label); + if ($field->has_label_colon) { + $output .= ': '; + } + if (isset($label_element)) { + $output .= ''; + } + } + $element_type = Html::escape($field->element_type); + if ($element_type) { + $output .= '<' . $element_type . $field->element_attributes . '>'; + } $output .= $field->content; - - $output .= $field->wrapper_suffix; + if ($element_type) { + $output .= ''; + } + if ($wrapper_element) { + $output .= ''; + } } return $output; -- GitLab