.../Plugin/CKEditorPlugin/DrupalImageCaption.php | 7 +- core/modules/editor/src/Form/EditorImageDialog.php | 6 +- core/modules/filter/filter.module | 2 +- .../filter/src/Plugin/Filter/FilterCaption.php | 84 ++++-------- core/modules/filter/src/Tests/FilterUnitTest.php | 144 +++++++++++++-------- .../filter/templates/filter-caption.html.twig | 5 +- .../config/install/filter.format.basic_html.yml | 6 + .../config/install/filter.format.full_html.yml | 6 + 8 files changed, 140 insertions(+), 120 deletions(-) diff --git a/core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalImageCaption.php b/core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalImageCaption.php index 5c2fffa..6feb593 100644 --- a/core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalImageCaption.php +++ b/core/modules/ckeditor/src/Plugin/CKEditorPlugin/DrupalImageCaption.php @@ -73,9 +73,10 @@ function isEnabled(Editor $editor) { } // Automatically enable this plugin if the text format associated with this - // text editor uses the filter_caption filter and the DrupalImage button is - // enabled. - if ($editor->getFilterFormat()->filters('filter_caption')->status) { + // text editor uses the filter_align or filter_caption filter and the + // DrupalImage button is enabled. + $format = $editor->getFilterFormat(); + if ($format->filters('filter_align')->status || $format->filters('filter_caption')->status) { $enabled = FALSE; $settings = $editor->getSettings(); foreach ($settings['toolbar']['rows'] as $row) { diff --git a/core/modules/editor/src/Form/EditorImageDialog.php b/core/modules/editor/src/Form/EditorImageDialog.php index 893b5a4..b918bf8 100644 --- a/core/modules/editor/src/Form/EditorImageDialog.php +++ b/core/modules/editor/src/Form/EditorImageDialog.php @@ -132,9 +132,9 @@ public function buildForm(array $form, array &$form_state, FilterFormat $filter_ '#parents' => array('attributes', 'height'), ); - // When Drupal core's filter_caption is being used, the text editor may + // When Drupal core's filter_align is being used, the text editor may // offer the ability to change the alignment. - if (isset($image_element['data-align'])) { + if (isset($image_element['data-align']) && $filter_format->filters('filter_align')->status) { $form['align'] = array( '#title' => $this->t('Align'), '#type' => 'radios', @@ -153,7 +153,7 @@ public function buildForm(array $form, array &$form_state, FilterFormat $filter_ // When Drupal core's filter_caption is being used, the text editor may // offer the ability to in-place edit the image's caption: show a toggle. - if (isset($image_element['hasCaption'])) { + if (isset($image_element['hasCaption']) && $filter_format->filters('filter_caption')->status) { $form['caption'] = array( '#title' => $this->t('Caption'), '#type' => 'checkbox', diff --git a/core/modules/filter/filter.module b/core/modules/filter/filter.module index 8eb3373..ea14830 100644 --- a/core/modules/filter/filter.module +++ b/core/modules/filter/filter.module @@ -80,7 +80,7 @@ function filter_theme() { 'node' => NULL, 'tag' => NULL, 'caption' => NULL, - 'align' => NULL, + 'classes' => NULL, ), 'template' => 'filter-caption', ) diff --git a/core/modules/filter/src/Plugin/Filter/FilterCaption.php b/core/modules/filter/src/Plugin/Filter/FilterCaption.php index bccaf02..475cdbb 100644 --- a/core/modules/filter/src/Plugin/Filter/FilterCaption.php +++ b/core/modules/filter/src/Plugin/Filter/FilterCaption.php @@ -15,12 +15,14 @@ use Drupal\filter\Plugin\FilterBase; /** - * Provides a filter to display image captions and align images. + * Provides a filter to caption elements. + * + * When used in combination with the filter_align filter, this must run last. * * @Filter( * id = "filter_caption", - * title = @Translation("Display image captions and align images"), - * description = @Translation("Uses data-caption and data-align attributes on <img> tags to caption and align images."), + * title = @Translation("Caption images"), + * description = @Translation("Uses a data-caption attribute on <img> tags to caption images."), * type = Drupal\filter\Plugin\FilterInterface::TYPE_TRANSFORM_REVERSIBLE * ) */ @@ -32,60 +34,35 @@ class FilterCaption extends FilterBase { public function process($text, $langcode) { $result = new FilterProcessResult($text); - if (stristr($text, 'data-caption') !== FALSE || stristr($text, 'data-align') !== FALSE) { - $caption_found = FALSE; + if (stristr($text, 'data-caption') !== FALSE) { $dom = Html::load($text); $xpath = new \DOMXPath($dom); - foreach ($xpath->query('//*[@data-caption or @data-align]') as $node) { - $caption = NULL; - $align = NULL; + foreach ($xpath->query('//*[@data-caption]') as $node) { + // Read the data-caption attribute's value, then delete it. + $caption = String::checkPlain($node->getAttribute('data-caption')); + $node->removeAttribute('data-caption'); - // Retrieve, then remove the data-caption and data-align attributes. - if ($node->hasAttribute('data-caption')) { - $caption = String::checkPlain($node->getAttribute('data-caption')); - $node->removeAttribute('data-caption'); - // Sanitize caption: decode HTML encoding, limit allowed HTML tags; - // only allow inline tags that are allowed by default, plus
. - $caption = String::decodeEntities($caption); - $caption = Xss::filter($caption, array('a', 'em', 'strong', 'cite', 'code', 'br')); - // The caption must be non-empty. - if (Unicode::strlen($caption) === 0) { - $caption = NULL; - } - } - if ($node->hasAttribute('data-align')) { - $align = $node->getAttribute('data-align'); - $node->removeAttribute('data-align'); - // Only allow 3 values: 'left', 'center' and 'right'. - if (!in_array($align, array('left', 'center', 'right'))) { - $align = NULL; - } - } + // Sanitize caption: decode HTML encoding, limit allowed HTML tags; only + // allow inline tags that are allowed by default, plus
. + $caption = String::decodeEntities($caption); + $caption = Xss::filter($caption, array('a', 'em', 'strong', 'cite', 'code', 'br')); - // Don't transform the HTML if there isn't a caption after validation. - if ($caption === NULL) { - // If there is a valid alignment, then transform the data-align - // attribute to a corresponding alignment class. - if ($align !== NULL) { - $classes = $node->getAttribute('class'); - $classes = (strlen($classes) > 0) ? explode(' ', $classes) : array(); - $classes[] = 'align-' . $align; - $node->setAttribute('class', implode(' ', $classes)); - } + // The caption must be non-empty. + if (Unicode::strlen($caption) === 0) { continue; } - else { - $caption_found = TRUE; - } - // Given the updated node, caption and alignment: re-render it with a - // caption. + // Given the updated node and caption: re-render it with a caption, but + // bubble up the value of the class attribute of the captioned element, + // this allows it to collaborate with e.g. the filter_align filter. + $classes = $node->getAttribute('class'); + $node->removeAttribute('class'); $filter_caption = array( '#theme' => 'filter_caption', '#node' => $node->C14N(), '#tag' => $node->tagName, '#caption' => $caption, - '#align' => $align, + '#classes' => $classes, ); $altered_html = drupal_render($filter_caption); @@ -102,15 +79,12 @@ public function process($text, $langcode) { $node->parentNode->replaceChild($updated_node, $node); } - $result->setProcessedText(Html::serialize($dom)); - - if ($caption_found) { - $result->addAssets(array( + $result->setProcessedText(Html::serialize($dom)) + ->addAssets(array( 'library' => array( 'filter/caption', ), )); - } } return $result; @@ -122,15 +96,11 @@ public function process($text, $langcode) { public function tips($long = FALSE) { if ($long) { return $this->t(' -

You can add image captions and align images left, right or centered. Examples:

- '); +

You can add caption images. Example: <img src="" data-caption="This is a caption" />.

'); } else { - return $this->t('You can caption (data-caption="Text") and align images (data-align="center"), but also video, blockquotes, and so on.'); + return $this->t('You can caption images (data-caption="Text"), but also video, blockquotes, and so on.'); } } + } diff --git a/core/modules/filter/src/Tests/FilterUnitTest.php b/core/modules/filter/src/Tests/FilterUnitTest.php index b15f0a0..e67a76e 100644 --- a/core/modules/filter/src/Tests/FilterUnitTest.php +++ b/core/modules/filter/src/Tests/FilterUnitTest.php @@ -46,6 +46,61 @@ protected function setUp() { } /** + * Tests the align filter. + */ + function testAlignFilter() { + $filter = $this->filters['filter_align']; + + $test = function($input) use ($filter) { + return $filter->process($input, 'und'); + }; + + // No data-align attribute. + $input = ''; + $expected = $input; + $this->assertIdentical($expected, $test($input)->getProcessedText()); + + // Data-align attribute: all 3 allowed values. + $input = ''; + $expected = ''; + $this->assertIdentical($expected, $test($input)->getProcessedText()); + $input = ''; + $expected = ''; + $this->assertIdentical($expected, $test($input)->getProcessedText()); + $input = ''; + $expected = ''; + $this->assertIdentical($expected, $test($input)->getProcessedText()); + + // Data-align attribute: a disallowed value. + $input = ''; + $expected = ''; + $this->assertIdentical($expected, $test($input)->getProcessedText()); + + // Empty data-align attribute. + $input = ''; + $expected = ''; + $this->assertIdentical($expected, $test($input)->getProcessedText()); + + // Ensure the filter also works with uncommon yet valid attribute quoting. + $input = ''; + $expected = ''; + $output = $test($input); + $this->assertIdentical($expected, $output->getProcessedText()); + + // Security test: attempt to inject an additional class. + $input = ''; + $expected = ''; + $output = $test($input); + $this->assertIdentical($expected, $output->getProcessedText()); + + // Security test: attempt an XSS. + $input = ''; + $expected = ''; + $output = $test($input); + $this->assertIdentical($expected, $output->getProcessedText()); + } + + /** * Tests the caption filter. */ function testCaptionFilter() { @@ -61,12 +116,12 @@ function testCaptionFilter() { ), ); - // No data-caption nor data-align attributes. + // No data-caption attribute. $input = ''; $expected = $input; $this->assertIdentical($expected, $test($input)->getProcessedText()); - // Only data-caption attribute. + // Data-caption attribute. $input = ''; $expected = '
Loquacious llama!
'; $output = $test($input); @@ -108,26 +163,42 @@ function testCaptionFilter() { $this->assertIdentical($expected, $output->getProcessedText()); $this->assertIdentical($attached_library, $output->getAssets()); - // Only data-align attribute: all 3 allowed values. - $input = ''; - $expected = ''; - $this->assertIdentical($expected, $test($input)->getProcessedText()); - $input = ''; - $expected = ''; - $this->assertIdentical($expected, $test($input)->getProcessedText()); - $input = ''; - $expected = ''; - $this->assertIdentical($expected, $test($input)->getProcessedText()); + // Ensure the filter also works with uncommon yet valid attribute quoting. + $input = ''; + $expected = '
Loquacious llama!
'; + $output = $test($input); + $this->assertIdentical($expected, $output->getProcessedText()); + $this->assertIdentical($attached_library, $output->getAssets()); - // Only data-align attribute: a disallowed value. - $input = ''; - $expected = ''; - $this->assertIdentical($expected, $test($input)->getProcessedText()); + // Finally, ensure that this also works on any other tag. + $input = '