diff --git a/src/ImageManipulation.php b/src/ImageManipulation.php
index d07027e5..a2827fc2 100644
--- a/src/ImageManipulation.php
+++ b/src/ImageManipulation.php
@@ -1145,8 +1145,8 @@ protected function getDefaultAttributes(): array
$attributes = [];
if ($this->getIsImage()) {
$attributes = [
- 'width' => $this->getWidth(),
- 'height' => $this->getHeight(),
+ 'width' => $this->getWidth() ?: null,
+ 'height' => $this->getHeight() ?: null,
'alt' => $this->getTitle(),
'src' => $this->getURL(false)
];
@@ -1171,11 +1171,19 @@ public function getAttributes()
$attributes = array_merge($defaultAttributes, $this->attributes);
- // We need to suppress the `loading="eager"` attribute after we merge the default attributes
- if (isset($attributes['loading']) && $attributes['loading'] === 'eager') {
- unset($attributes['loading']);
+ if (isset($attributes['loading'])) {
+ // Check if the dimensions for the image have been set
+ $dimensionsUnset = (
+ empty($attributes['width']) ||
+ empty($attributes['height'])
+ );
+
+ // We need to suppress the `loading="eager"` attribute after we merge the default attributes
+ // or if dimensions are not set
+ if ($attributes['loading'] === 'eager' || $dimensionsUnset) {
+ unset($attributes['loading']);
+ }
}
-
$this->extend('updateAttributes', $attributes);
return $attributes;
diff --git a/src/Shortcodes/ImageShortcodeProvider.php b/src/Shortcodes/ImageShortcodeProvider.php
index 11ce70b3..0830cfcd 100644
--- a/src/Shortcodes/ImageShortcodeProvider.php
+++ b/src/Shortcodes/ImageShortcodeProvider.php
@@ -7,6 +7,7 @@
use SilverStripe\Assets\Image;
use SilverStripe\Core\Flushable;
use SilverStripe\Core\Injector\Injector;
+use SilverStripe\Dev\Deprecation;
use SilverStripe\View\Parsers\ShortcodeHandler;
use SilverStripe\View\Parsers\ShortcodeParser;
@@ -73,26 +74,43 @@ public static function handle_shortcode($args, $content, $parser, $shortcode, $e
return null; // There were no suitable matches at all.
}
+ // Grant access to file if necessary
+ if (static::getGrant($record)) {
+ $record->grantFile();
+ }
+
// Check if a resize is required
+ $manipulatedRecord = $record;
$width = null;
$height = null;
- $grant = static::getGrant($record);
- $src = $record->getURL($grant);
if ($record instanceof Image) {
$width = isset($args['width']) ? (int) $args['width'] : null;
$height = isset($args['height']) ? (int) $args['height'] : null;
+
+ // Resize the image if custom dimensions are provided
$hasCustomDimensions = ($width && $height);
if ($hasCustomDimensions && (($width != $record->getWidth()) || ($height != $record->getHeight()))) {
- $resized = $record->ResizedImage($width, $height);
+ $resized = $manipulatedRecord->ResizedImage($width, $height);
// Make sure that the resized image actually returns an image
if ($resized) {
- $src = $resized->getURL($grant);
+ $manipulatedRecord = $resized;
}
}
+
+ // If only one of width or height is provided, explicitly unset the other
+ if ($width && !$height) {
+ $args['height'] = false;
+ } elseif (!$width && $height) {
+ $args['width'] = false;
+ }
}
- // Determine whether loading="lazy" is set
- $args = ImageShortcodeProvider::updateLoadingValue($args, $width, $height);
+ // Set lazy loading attribute
+ if (!empty($args['loading'])) {
+ $loading = strtolower($args['loading']);
+ unset($args['loading']);
+ $manipulatedRecord = $manipulatedRecord->LazyLoad($loading !== 'eager');
+ }
// Build the HTML tag
$attrs = array_merge(
@@ -101,7 +119,7 @@ public static function handle_shortcode($args, $content, $parser, $shortcode, $e
// Use all other shortcode arguments
$args,
// But enforce some values
- ['id' => '', 'src' => $src]
+ ['id' => '', 'src' => '']
);
// If file was not found then use the Title value from static::find_error_record() for the alt attr
@@ -111,11 +129,14 @@ public static function handle_shortcode($args, $content, $parser, $shortcode, $e
// Clean out any empty attributes (aside from alt) and anything not whitelisted
$whitelist = static::config()->get('attribute_whitelist');
- $attrs = array_filter($attrs ?? [], function ($v, $k) use ($whitelist) {
- return in_array($k, $whitelist) && (strlen(trim($v ?? '')) || $k === 'alt');
- }, ARRAY_FILTER_USE_BOTH);
+ foreach ($attrs as $key => $value) {
+ if (in_array($key, $whitelist) && (strlen(trim($value ?? '')) || in_array($key, ['alt', 'width', 'height']))) {
+ $manipulatedRecord = $manipulatedRecord->setAttribute($key, html_entity_decode($value));
+ }
+ }
- $markup = ImageShortcodeProvider::createImageTag($attrs);
+ // We're calling renderWith() with an explicit template in case someone wants to use a custom template
+ $markup = $manipulatedRecord->renderWith(self::class . '_Image');
// cache it for future reference
if ($fileFound) {
@@ -131,9 +152,12 @@ public static function handle_shortcode($args, $content, $parser, $shortcode, $e
/**
* Construct and return HTML image tag.
+ *
+ * @deprecated 2.3.0
*/
public static function createImageTag(array $attributes) : string
{
+ Deprecation::notice('2.3.0', 'Will be removed without equivalent functionality to replace it.');
$preparedAttributes = '';
foreach ($attributes as $attributeKey => $attributeValue) {
if (strlen($attributeValue ?? '') > 0 || $attributeKey === 'alt') {
@@ -209,29 +233,4 @@ protected static function find_error_record($errorCode)
'Title' => _t(__CLASS__ . '.IMAGENOTFOUND', 'Image not found'),
]);
}
-
- /**
- * Updated the loading attribute which is used to either lazy-load or eager-load images
- * Eager-load is the default browser behaviour so when eager loading is specified, the
- * loading attribute is omitted
- *
- * @param array $args
- * @param int|null $width
- * @param int|null $height
- * @return array
- */
- private static function updateLoadingValue(array $args, ?int $width, ?int $height): array
- {
- if (!Image::getLazyLoadingEnabled()) {
- return $args;
- }
- if (isset($args['loading']) && $args['loading'] == 'eager') {
- // per image override - unset the loading attribute unset to eager load (default browser behaviour)
- unset($args['loading']);
- } elseif ($width && $height) {
- // width and height must be present to prevent content shifting
- $args['loading'] = 'lazy';
- }
- return $args;
- }
}
diff --git a/templates/DBFile_download.ss b/templates/DBFile_download.ss
index 05197af8..7eacf0c7 100644
--- a/templates/DBFile_download.ss
+++ b/templates/DBFile_download.ss
@@ -1 +1 @@
-download="$Basename.ATT"<% else %>download<% end_if %>>$Title
+<% include SilverStripe\Assets\Storage\DBFile %>
diff --git a/templates/DBFile_image.ss b/templates/DBFile_image.ss
index c6a20a2a..5c46dddf 100644
--- a/templates/DBFile_image.ss
+++ b/templates/DBFile_image.ss
@@ -1 +1 @@
-![]()
+<% include SilverStripe\Assets\Storage\DBFile_Image %>
\ No newline at end of file
diff --git a/templates/SilverStripe/Assets/Shortcodes/ImageShortcodeProvider_Image.ss b/templates/SilverStripe/Assets/Shortcodes/ImageShortcodeProvider_Image.ss
new file mode 100644
index 00000000..8d25a333
--- /dev/null
+++ b/templates/SilverStripe/Assets/Shortcodes/ImageShortcodeProvider_Image.ss
@@ -0,0 +1 @@
+<% include SilverStripe\Assets\Storage\DBFile_Image %>
diff --git a/templates/SilverStripe/Assets/Storage/DBFile.ss b/templates/SilverStripe/Assets/Storage/DBFile.ss
new file mode 100644
index 00000000..05197af8
--- /dev/null
+++ b/templates/SilverStripe/Assets/Storage/DBFile.ss
@@ -0,0 +1 @@
+download="$Basename.ATT"<% else %>download<% end_if %>>$Title
diff --git a/templates/SilverStripe/Assets/Storage/DBFile_Image.ss b/templates/SilverStripe/Assets/Storage/DBFile_Image.ss
new file mode 100644
index 00000000..c6a20a2a
--- /dev/null
+++ b/templates/SilverStripe/Assets/Storage/DBFile_Image.ss
@@ -0,0 +1 @@
+![]()
diff --git a/tests/php/Shortcodes/FileLinkTrackingTest.php b/tests/php/Shortcodes/FileLinkTrackingTest.php
index b65a7def..3e81bf71 100644
--- a/tests/php/Shortcodes/FileLinkTrackingTest.php
+++ b/tests/php/Shortcodes/FileLinkTrackingTest.php
@@ -78,7 +78,7 @@ public function testFileRenameUpdatesDraftAndPublishedPages()
// Live and stage pages both have link to public file
$this->assertStringContainsString(
- '
dbObject('Content')->forTemplate()
);
$this->assertStringContainsString(
@@ -91,7 +91,7 @@ public function testFileRenameUpdatesDraftAndPublishedPages()
/** @var EditableObject $pageLive */
$pageLive = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- 'dbObject('Content')->forTemplate()
);
$this->assertStringContainsString(
@@ -120,14 +120,14 @@ public function testFileRenameUpdatesDraftAndPublishedPages()
// the mocked test location disappears for secure files.
$page = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- '
dbObject('Content')->forTemplate()
);
Versioned::withVersionedMode(function () use ($page) {
Versioned::set_stage(Versioned::LIVE);
$pageLive = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- 'dbObject('Content')->forTemplate()
);
});
@@ -137,14 +137,14 @@ public function testFileRenameUpdatesDraftAndPublishedPages()
$image1->publishRecursive();
$page = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- '
dbObject('Content')->forTemplate()
);
Versioned::withVersionedMode(function () use ($page) {
Versioned::set_stage(Versioned::LIVE);
$pageLive = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- 'dbObject('Content')->forTemplate()
);
});
@@ -153,14 +153,14 @@ public function testFileRenameUpdatesDraftAndPublishedPages()
$page->publishRecursive();
$page = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- 'dbObject('Content')->forTemplate()
);
Versioned::withVersionedMode(function () use ($page) {
Versioned::set_stage(Versioned::LIVE);
$pageLive = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- 'dbObject('Content')->forTemplate()
);
});
@@ -194,7 +194,7 @@ public function testTwoFileRenamesInARowWork()
Versioned::set_stage(Versioned::LIVE);
$livePage = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- 'dbObject('Content')->forTemplate()
);
});
@@ -213,7 +213,7 @@ public function testTwoFileRenamesInARowWork()
// Confirm that the correct image is shown in both the draft and live site
$page = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- '
dbObject('Content')->forTemplate()
);
@@ -223,7 +223,7 @@ public function testTwoFileRenamesInARowWork()
Versioned::set_stage(Versioned::LIVE);
$pageLive = EditableObject::get()->byID($page->ID);
$this->assertStringContainsString(
- 'dbObject('Content')->forTemplate()
);
});
diff --git a/tests/php/Shortcodes/ImageShortcodeProviderTest.php b/tests/php/Shortcodes/ImageShortcodeProviderTest.php
index b3109a1a..35e6102e 100644
--- a/tests/php/Shortcodes/ImageShortcodeProviderTest.php
+++ b/tests/php/Shortcodes/ImageShortcodeProviderTest.php
@@ -15,6 +15,7 @@
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Security\InheritedPermissions;
use SilverStripe\Security\Member;
+use DOMDocument;
class ImageShortcodeProviderTest extends SapphireTest
{
@@ -46,33 +47,46 @@ protected function tearDown(): void
public function testShortcodeHandlerDoesNotFallBackToFileProperties()
{
$image = $this->objFromFixture(Image::class, 'imageWithTitle');
- $parser = new ShortcodeParser();
- $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
-
- $this->assertEquals(
- sprintf(
- '
',
- $image->Link()
- ),
- $parser->parse(sprintf('[image id=%d]', $image->ID))
+ $this->assertImageTag(
+ [
+ 'src' => $image->Link(),
+ 'alt' => '',
+ 'width' => '300',
+ 'height' => '300',
+ 'title' => false,
+ 'id' => false,
+ ],
+ sprintf('[image id=%d]', $image->ID),
+ 'Shortcode handler does not fall back to file properties to generate alt attribute'
);
}
public function testShortcodeHandlerUsesShortcodeProperties()
{
$image = $this->objFromFixture(Image::class, 'imageWithTitle');
- $parser = new ShortcodeParser();
- $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
+ $this->assertImageTag(
+ [
+ 'src' => $image->Link(),
+ 'alt' => 'Alt content',
+ 'title' => 'Title content',
+ ],
+ sprintf('[image id="%d" alt="Alt content" title="Title content"]', $image->ID),
+ 'Shortcode handler uses properties from shortcode'
+ );
+ }
- $this->assertEquals(
- sprintf(
- '',
- $image->Link()
- ),
- $parser->parse(sprintf(
- '[image id="%d" alt="Alt content" title="Title content"]',
- $image->ID
- ))
+ public function testShortcodeHandlerHandlesEntities()
+ {
+ $image = $this->objFromFixture(Image::class, 'imageWithTitle');
+ // Because we are using a DOMDocument to read the values, we need to compare to non-escaped value
+ $this->assertImageTag(
+ [
+ 'src' => $image->Link(),
+ 'alt' => 'Alt & content',
+ 'title' => '" Hockey > Rugby "',
+ ],
+ sprintf('[image id="%d" alt="Alt & content" title="" Hockey > Rugby ""]', $image->ID),
+ 'Shortcode handler can handle HTML entities'
);
}
@@ -101,10 +115,12 @@ public function testShorcodeRegenrator()
$image->ID,
$image->Link()
),
- $parser->parse(sprintf(
- '[image id="%d" alt="" title=""]',
- $image->ID
- )),
+ $parser->parse(
+ sprintf(
+ '[image id="%d" alt="" title=""]',
+ $image->ID
+ )
+ ),
"Shortcode regeneration properly handles empty attributes"
);
}
@@ -112,54 +128,65 @@ public function testShorcodeRegenrator()
public function testShortcodeHandlerAddsDefaultAttributes()
{
$image = $this->objFromFixture(Image::class, 'imageWithoutTitle');
- $parser = new ShortcodeParser();
- $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
-
- $this->assertEquals(
- sprintf(
- '',
- $image->Link()
- ),
- $parser->parse(sprintf(
- '[image id="%d"]',
- $image->ID
- ))
+ $this->assertImageTag(
+ [
+ 'src' => $image->Link(),
+ 'alt' => '',
+ 'width' => '300',
+ 'height' => '300',
+ 'title' => false,
+ 'id' => false,
+ ],
+ sprintf('[image id=%d]', $image->ID),
+ 'Shortcode handler adds default attributes'
);
}
public function testShortcodeHandlerDoesNotResampleToNonIntegerImagesSizes()
{
$image = $this->objFromFixture(Image::class, 'imageWithoutTitle');
- $parser = new ShortcodeParser();
- $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
-
- $this->assertEquals(
- sprintf(
- '',
- $image->Link()
- ),
- $parser->parse(sprintf(
- '[image id="%d" alt="" width="50%%" height="auto"]',
- $image->ID
- ))
+ $this->assertImageTag(
+ [
+ 'src' => $image->Link(),
+ 'alt' => '',
+ 'width' => '50%',
+ 'title' => false,
+ 'id' => false,
+ ],
+ sprintf('[image id="%d" alt="" width="50%%"]', $image->ID),
+ 'Shortcode handler does resample to non-integer image sizes'
);
}
public function testShortcodeHandlerFailsGracefully()
{
- $parser = new ShortcodeParser();
- $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
-
$nonExistentImageID = File::get()->max('ID') + 1;
- $expected = '![Image not found]()
';
- $shortcodes = [
+
+ $this->assertImageTag(
+ [
+ 'src' => false,
+ 'alt' => 'Image not found',
+ 'width' => false,
+ 'height' => false,
+ 'title' => false,
+ 'id' => false,
+ ],
'[image id="' . $nonExistentImageID . '"]',
+ 'Shortcode handler fails gracefully when image does not exist'
+ );
+
+ $this->assertImageTag(
+ [
+ 'src' => false,
+ 'alt' => 'Image not found',
+ 'width' => false,
+ 'height' => false,
+ 'title' => false,
+ 'id' => false,
+ ],
'[image id="' . $nonExistentImageID . '" alt="my-alt-attr"]',
- ];
- foreach ($shortcodes as $shortcode) {
- $actual = $parser->parse($shortcode);
- $this->assertEquals($expected, $actual);
- }
+ 'Shortcode handler fails gracefully when image does not exist asd overrides alt attribute'
+ );
}
public function testMissingImageDoesNotCache()
@@ -189,39 +216,119 @@ public function testMissingImageDoesNotCache()
public function testLazyLoading()
{
- $parser = new ShortcodeParser();
- $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
+ $image = $this->objFromFixture(Image::class, 'imageWithoutTitle');
+ $id = $image->ID;
+
+ // regular shortcode
+ $this->assertImageTag(
+ [
+ 'src' => $image->ResizedImage(300, 200)->Link(),
+ 'alt' => '',
+ 'width' => '300',
+ 'height' => '200',
+ 'title' => false,
+ 'id' => false,
+ 'loading' => 'lazy',
+ ],
+ '[image id="' . $id . '" width="300" height="200"]',
+ 'Lazy loading is enabled by default'
+ );
- $id = $this->objFromFixture(Image::class, 'imageWithTitle')->ID;
// regular shortcode
- $shortcode = '[image id="' . $id . '" width="300" height="200"]';
- $this->assertStringContainsString('loading="lazy"', $parser->parse($shortcode));
+ $this->assertImageTag(
+ [
+ 'src' => $image->Link(),
+ 'alt' => '',
+ 'width' => '300',
+ 'height' => '300',
+ 'title' => false,
+ 'id' => false,
+ 'loading' => 'lazy',
+ ],
+ '[image id="' . $id . '"]',
+ 'Lazy loading still works if width and height are not provided. Dimensions are read from the file instead.'
+ );
// missing width
- $shortcode = '[image id="' . $id . '" height="200"]';
- $this->assertStringNotContainsString('loading="lazy"', $parser->parse($shortcode));
+ $this->assertImageTag(
+ [
+ 'src' => $image->Link(),
+ 'alt' => '',
+ 'height' => '200',
+ 'title' => false,
+ 'id' => false,
+ 'loading' => false,
+ ],
+ '[image id="' . $id . '" height="200"]',
+ 'If the width of the image can not be determined, lazy loading is not applied'
+ );
// missing height
- $shortcode = '[image id="' . $id . '" width="300"]';
- $this->assertStringNotContainsString('loading="lazy"', $parser->parse($shortcode));
+ $this->assertImageTag(
+ [
+ 'src' => $image->Link(),
+ 'alt' => '',
+ 'width' => '300',
+ 'title' => false,
+ 'id' => false,
+ 'loading' => false,
+ ],
+ '[image id="' . $id . '" width="300"]',
+ 'If the height of the image can not be determined, lazy loading is not applied'
+ );
// loading="eager"
- $shortcode = '[image id="' . $id . '" width="300" height="200" loading="eager"]';
- $this->assertStringNotContainsString('loading="lazy"', $parser->parse($shortcode));
+ $this->assertImageTag(
+ [
+ 'src' => $image->ResizedImage(300, 200)->Link(),
+ 'alt' => '',
+ 'height' => '200',
+ 'width' => '300',
+ 'title' => false,
+ 'id' => false,
+ 'loading' => false,
+ ],
+ '[image id="' . $id . '" width="300" height="200" loading="eager"]',
+ 'Shortcode can force eager loading'
+ );
// loading="nonsense"
- $shortcode = '[image id="' . $id . '" width="300" height="200" loading="nonsense"]';
- $this->assertStringContainsString('loading="lazy"', $parser->parse($shortcode));
+ $this->assertImageTag(
+ [
+ 'src' => $image->ResizedImage(300, 200)->Link(),
+ 'alt' => '',
+ 'height' => '200',
+ 'width' => '300',
+ 'title' => false,
+ 'id' => false,
+ 'loading' => 'lazy',
+ ],
+ '[image id="' . $id . '" width="300" height="200" loading="nonsense"]',
+ 'Invalid loading value in short code are ignored and the image is lazy loaded'
+ );
// globally disabled
- Config::withConfig(function () use ($id, $parser) {
- Config::modify()->set(Image::class, 'lazy_loading_enabled', false);
- // clear-provider-cache is so that we don't get a cached result from the 'regular shortcode'
- // assertion earlier in this function from ImageShortCodeProvider::handle_shortcode()
- $shortcode = '[image id="' . $id . '" width="300" height="200" clear-provider-cache="1"]';
- $this->assertStringNotContainsString('loading="lazy"', $parser->parse($shortcode));
- });
+ Config::withConfig(
+ function () use ($id, $image) {
+ Config::modify()->set(Image::class, 'lazy_loading_enabled', false);
+ // clear-provider-cache is so that we don't get a cached result from the 'regular shortcode'
+ // assertion earlier in this function from ImageShortCodeProvider::handle_shortcode()
+ $this->assertImageTag(
+ [
+ 'src' => $image->ResizedImage(300, 200)->Link(),
+ 'alt' => '',
+ 'height' => '200',
+ 'width' => '300',
+ 'title' => false,
+ 'id' => false,
+ 'loading' => false,
+ ],
+ '[image id="' . $id . '" width="300" height="200" clear-provider-cache="1"]',
+ 'If lazy loading is disabled globally the image is not lazy loaded'
+ );
+ }
+ );
}
public function testRegenerateShortcode()
@@ -245,10 +352,13 @@ public function testRegenerateShortcode()
'class' => 'leftAlone ss-htmleditorfield-file image',
];
$shortHash = substr($image->getHash(), 0, 10);
- $expected = implode(' ', [
+ $expected = implode(
+ ' ',
+ [
'[image id="' . $image->ID . '" src="/assets/folder/' . $shortHash . '/test-image.png" width="550"',
'height="366" class="leftAlone ss-htmleditorfield-file image"]'
- ]);
+ ]
+ );
$parsedFileID = new ParsedFileID($image->getFilename(), $image->getHash());
$html = ImageShortcodeProvider::regenerate_shortcode($args, '', '', 'image');
$this->assertSame($expected, $html);
@@ -271,77 +381,93 @@ public function testRegenerateShortcode()
public function testEmptyAttributesAreRemoved()
{
$image = $this->objFromFixture(Image::class, 'imageWithTitle');
- $parser = new ShortcodeParser();
- $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
- // Note that alt is allowed to be empty
- $this->assertEquals(
- sprintf(
- '',
- $image->Link()
- ),
- $parser->parse(sprintf('[image id=%d alt="" class=""]', $image->ID))
+ $this->assertImageTag(
+ [
+ 'src' => $image->Link(),
+ 'alt' => '',
+ 'width' => '300',
+ 'height' => '300',
+ 'title' => false,
+ 'id' => false,
+ 'loading' => 'lazy',
+ 'class' => false,
+ ],
+ sprintf('[image id=%d alt="" class=""]', $image->ID),
+ 'Image shortcode does not render empty attributes'
);
}
public function testOnlyWhitelistedAttributesAllowed()
{
$image = $this->objFromFixture(Image::class, 'imageWithoutTitle');
- $parser = new ShortcodeParser();
- $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
$whitelist = ImageShortcodeProvider::config()->get('attribute_whitelist');
- $attributes = '';
+ $attributeString = '';
+ $attributes = [];
foreach ($whitelist as $attrName) {
if ($attrName === 'src') {
continue;
}
- $attributes .= $attrName . '="' . $attrName . '" ';
+ $attributeString .= $attrName . '="' . $attrName . '" ';
+ $attributes[$attrName] = $attrName;
}
- $this->assertEquals(
- sprintf(
- '',
- $image->Link(),
- trim($attributes)
+ $this->assertImageTag(
+ array_merge(
+ $attributes,
+ [
+ 'src' => $image->Link(),
+ 'id' => false,
+ 'loading' => 'lazy',
+ 'style' => false,
+ 'data-some-value' => false
+ ]
),
- $parser->parse(sprintf(
+ sprintf(
'[image id="%d" %s style="width:100px" data-some-value="my-data"]',
$image->ID,
- trim($attributes)
- ))
+ trim($attributeString)
+ ),
+ 'Image shortcode does not render attributes not in whitelist'
);
}
public function testWhiteIsConfigurable()
{
$image = $this->objFromFixture(Image::class, 'imageWithoutTitle');
- $parser = new ShortcodeParser();
- $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
$whitelist = ImageShortcodeProvider::config()->get('attribute_whitelist');
- $attributes = '';
+ $attributeString = '';
+ $attributes = [];
foreach ($whitelist as $attrName) {
if ($attrName === 'src') {
continue;
}
- $attributes .= $attrName . '="' . $attrName . '" ';
+ $attributeString .= $attrName . '="' . $attrName . '" ';
+ $attributes[$attrName] = $attrName;
}
// Allow new whitelisted attribute
Config::modify()->merge(ImageShortcodeProvider::class, 'attribute_whitelist', ['data-some-value']);
- $this->assertEquals(
- sprintf(
- '',
- $image->Link(),
- trim($attributes) . ' data-some-value="my-data"'
+ $this->assertImageTag(
+ array_merge(
+ $attributes,
+ [
+ 'src' => $image->Link(),
+ 'id' => false,
+ 'loading' => 'lazy',
+ 'style' => false,
+ 'data-some-value' => 'my-data'
+ ]
),
- $parser->parse(sprintf(
+ sprintf(
'[image id="%d" %s style="width:100px" data-some-value="my-data"]',
$image->ID,
- trim($attributes)
- ))
+ trim($attributeString)
+ ),
+ 'Image shortcode does not render attributes not in whitelist'
);
}
@@ -380,4 +506,39 @@ public function testCreateImageTag(string $expected, array $attributes)
{
$this->assertEquals($expected, ImageShortcodeProvider::createImageTag($attributes));
}
+
+ /**
+ * This method will assert that the $tag will contain an image with specific attributes and values
+ *
+ * @param array $attrs Key pair values of attributes and values.
+ * If the value is FALSE we assume that it is not present
+ */
+ private function assertImageTag(array $attrs, string $shortcode, $message = ""): void
+ {
+ $parser = new ShortcodeParser();
+ $parser->register('image', [ImageShortcodeProvider::class, 'handle_shortcode']);
+ $tag = $parser->parse($shortcode);
+
+ $doc = new DOMDocument();
+ $doc->loadHTML($tag);
+ $node = $doc->getElementsByTagName('img')->item(0);
+ $nodeAttrs = $node->attributes;
+
+ foreach ($attrs as $key => $value) {
+ $nodeAttr = $nodeAttrs->getNamedItem($key);
+ if ($value === false) {
+ if ($nodeAttr !== null) {
+ $this->fail("$message\nImage should not contain attribute '$key'\n$tag");
+ }
+ } else {
+ if ($nodeAttr === null) {
+ $this->fail("$message\nImage should contain attribute '$key'\n$tag");
+ }
+ if ($nodeAttr->nodeValue !== $value) {
+ $this->fail("$message\nImage attribute '$key' should have value '$value'\n$tag");
+ }
+ }
+ $this->assertTrue(true, 'Suppress warning about not having any assertion');
+ }
+ }
}