diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d75bb02..f2e50bc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -16,8 +16,6 @@ jobs: php-versions: ['8.0', '8.1'] neos-versions: ['8.3'] include: - - php-versions: '7.3' - neos-versions: '7.3' - php-versions: '7.4' neos-versions: '7.3' - php-versions: '8.0' diff --git a/Classes/Domain/AbstractImageSource.php b/Classes/Domain/AbstractImageSource.php index 68c753a..5eb6757 100644 --- a/Classes/Domain/AbstractImageSource.php +++ b/Classes/Domain/AbstractImageSource.php @@ -7,7 +7,6 @@ use Neos\Eel\ProtectedContextAwareInterface; use Neos\Flow\Annotations as Flow; use Neos\Flow\Log\Utility\LogEnvironment; -use Neos\Utility\Arrays; use Psr\Log\LoggerInterface; use Sitegeist\Kaleidoscope\EelHelpers\ImageSourceHelperInterface; @@ -218,7 +217,7 @@ public function withVariantPreset(string $presetIdentifier, string $presetVarian } /** - * Render sourceset Attribute for various media descriptors. + * Render sourceset Attribute non-scalable media. * * @param mixed $mediaDescriptors * @@ -226,31 +225,6 @@ public function withVariantPreset(string $presetIdentifier, string $presetVarian */ public function srcset($mediaDescriptors): string { - if ($this instanceof ScalableImageSourceInterface) { - $srcsetArray = []; - - if (is_array($mediaDescriptors) || $mediaDescriptors instanceof \Traversable) { - $descriptors = $mediaDescriptors; - } else { - $descriptors = Arrays::trimExplode(',', (string) $mediaDescriptors); - } - - foreach ($descriptors as $descriptor) { - if (preg_match('/^(?[0-9]+)w$/u', $descriptor, $matches)) { - $width = (int) $matches['width']; - $scaleFactor = $width / $this->width(); - $scaled = $this->scale($scaleFactor); - $srcsetArray[] = $scaled->src() . ' ' . $width . 'w'; - } elseif (preg_match('/^(?[0-9\\.]+)x$/u', $descriptor, $matches)) { - $factor = (float) $matches['factor']; - $scaled = $this->scale($factor); - $srcsetArray[] = $scaled->src() . ' ' . $factor . 'x'; - } - } - - return implode(', ', $srcsetArray); - } - return $this->src(); } diff --git a/Classes/Domain/AbstractScalableImageSource.php b/Classes/Domain/AbstractScalableImageSource.php index 60deaea..fa63e85 100644 --- a/Classes/Domain/AbstractScalableImageSource.php +++ b/Classes/Domain/AbstractScalableImageSource.php @@ -7,12 +7,14 @@ use Imagine\Image\Box; use Imagine\Image\ImagineInterface; use Neos\Flow\Annotations as Flow; +use Neos\Flow\Log\Utility\LogEnvironment; use Neos\Media\Domain\Model\Adjustment\CropImageAdjustment; use Neos\Media\Domain\Model\Adjustment\ImageAdjustmentInterface; use Neos\Media\Domain\Model\Adjustment\ResizeImageAdjustment; use Neos\Media\Domain\ValueObject\Configuration\Adjustment; use Neos\Media\Domain\ValueObject\Configuration\VariantPreset; use Neos\Utility\ObjectAccess; +use Neos\Utility\Arrays; use Sitegeist\Kaleidoscope\EelHelpers\ScalableImageSourceHelperInterface; abstract class AbstractScalableImageSource extends AbstractImageSource implements ScalableImageSourceInterface, ScalableImageSourceHelperInterface @@ -38,9 +40,9 @@ abstract class AbstractScalableImageSource extends AbstractImageSource implement * @param int|null $targetWidth * @param bool $preserveAspect * - * @return ImageSourceInterface + * @return ScalableImageSourceInterface */ - public function withWidth(int $targetWidth = null, bool $preserveAspect = false): ImageSourceInterface + public function withWidth(int $targetWidth = null, bool $preserveAspect = false): ScalableImageSourceInterface { $newSource = clone $this; $newSource->targetWidth = $targetWidth; @@ -60,9 +62,9 @@ public function withWidth(int $targetWidth = null, bool $preserveAspect = false) * @param int|null $targetHeight * @param bool $preserveAspect * - * @return ImageSourceInterface + * @return ScalableImageSourceInterface */ - public function withHeight(int $targetHeight = null, bool $preserveAspect = false): ImageSourceInterface + public function withHeight(int $targetHeight = null, bool $preserveAspect = false): ScalableImageSourceInterface { $newSource = clone $this; $newSource->targetHeight = $targetHeight; @@ -78,12 +80,28 @@ public function withHeight(int $targetHeight = null, bool $preserveAspect = fals return $newSource; } + /** + * @param int $targetWidth + * @param int $targetHeight + * + * @return ScalableImageSourceInterface + */ + public function withDimensions(int $targetWidth, int $targetHeight): ScalableImageSourceInterface + { + $newSource = clone $this; + $newSource->targetWidth = $targetWidth; + $newSource->targetHeight = $targetHeight; + + return $newSource; + } + + /** * @param float $factor * - * @return ImageSourceInterface + * @return ScalableImageSourceInterface */ - public function scale(float $factor): ImageSourceInterface + public function scale(float $factor): ScalableImageSourceInterface { $scaledHelper = clone $this; @@ -213,4 +231,71 @@ protected function createAdjustment(Adjustment $adjustmentConfiguration): ImageA return $adjustment; } + + /** + * Render srcset Attribute for various media descriptors. + * + * If upscaling is not allowed and the width is greater than the base width, + * use the base width. + * + * @param $mediaDescriptors + * + * @return string + */ + public function srcset($mediaDescriptors): string + { + $srcsetArray = []; + + if (is_array($mediaDescriptors) || $mediaDescriptors instanceof \Traversable) { + $descriptors = $mediaDescriptors; + } else { + $descriptors = Arrays::trimExplode(',', (string)$mediaDescriptors); + } + + $srcsetType = null; + $maxScaleFactor = min($this->baseWidth / $this->width(), $this->baseHeight / $this->height()); + + foreach ($descriptors as $descriptor) { + $hasDescriptor = preg_match('/^(?[0-9]+)w$|^(?[0-9\\.]+)x$/u', $descriptor, $matches, PREG_UNMATCHED_AS_NULL); + + if (!$hasDescriptor) { + $this->logger->warning(sprintf('Invalid media descriptor "%s". Missing type "x" or "w"', $descriptor), LogEnvironment::fromMethodName(__METHOD__)); + continue; + } + + if (!$srcsetType) { + $srcsetType = isset($matches['width']) ? 'width' : 'factor'; + } elseif (($srcsetType === 'width' && isset($matches['factor'])) || ($srcsetType === 'factor' && isset($matches['width']))) { + $this->logger->warning(sprintf('Mixed media descriptors are not valid: [%s]', implode(', ', is_array($descriptors) ? $descriptors : iterator_to_array($descriptors))), LogEnvironment::fromMethodName(__METHOD__)); + continue; + } + + if ($srcsetType === 'width') { + $width = (int)$matches['width']; + $scaleFactor = $width / $this->width(); + if (!$this->supportsUpscaling() && ($width / $this->baseWidth > 1)) { + $srcsetArray[] = $this->src() . ' ' . $this->baseWidth . 'w'; + } else { + $scaled = $this->scale($scaleFactor); + $srcsetArray[] = $scaled->src() . ' ' . $width . 'w'; + } + } elseif ($srcsetType === 'factor') { + $factor = (float)$matches['factor']; + if ( + !$this->supportsUpscaling() && ( + ($this->targetHeight && ($maxScaleFactor < $factor)) || + ($this->targetWidth && ($maxScaleFactor < $factor)) + ) + ) { + $scaled = $this->scale($maxScaleFactor); + $srcsetArray[] = $scaled->src() . ' ' . $maxScaleFactor . 'x'; + } else { + $scaled = $this->scale($factor); + $srcsetArray[] = $scaled->src() . ' ' . $factor . 'x'; + } + } + } + + return implode(', ', array_unique($srcsetArray)); + } } diff --git a/Classes/Domain/AssetImageSource.php b/Classes/Domain/AssetImageSource.php index 0015039..0765109 100644 --- a/Classes/Domain/AssetImageSource.php +++ b/Classes/Domain/AssetImageSource.php @@ -69,6 +69,11 @@ public function __construct(ImageInterface $asset, ?string $title = null, ?strin $this->baseHeight = $this->asset->getHeight(); } + public function supportsUpscaling(): bool + { + return false; + } + /** * Use the variant generated from the given variant preset in this image source. * @@ -128,7 +133,7 @@ public function src(): string $async = $this->request ? $this->async : false; $allowCropping = true; - $allowUpScaling = false; + $allowUpScaling = $this->supportsUpscaling(); $thumbnailConfiguration = new ThumbnailConfiguration( $width, $width, @@ -168,7 +173,7 @@ public function dataSrc(): string $async = false; $allowCropping = true; - $allowUpScaling = false; + $allowUpScaling = $this->supportsUpscaling(); $thumbnailConfiguration = new ThumbnailConfiguration( $width, $width, diff --git a/Classes/Domain/DummyImageSource.php b/Classes/Domain/DummyImageSource.php index b19da61..70b46ad 100644 --- a/Classes/Domain/DummyImageSource.php +++ b/Classes/Domain/DummyImageSource.php @@ -35,6 +35,11 @@ class DummyImageSource extends AbstractScalableImageSource */ protected $baseUri; + /** + * @var bool + */ + private $allowUpScaling; + /** * @param string $baseUri * @param string|null $title @@ -45,7 +50,7 @@ class DummyImageSource extends AbstractScalableImageSource * @param string|null $foregroundColor * @param string|null $text */ - public function __construct(string $baseUri, ?string $title = null, ?string $alt = null, ?int $baseWidth = null, ?int $baseHeight = null, ?string $backgroundColor = null, ?string $foregroundColor = null, ?string $text = null) + public function __construct(string $baseUri, ?string $title = null, ?string $alt = null, ?int $baseWidth = null, ?int $baseHeight = null, ?string $backgroundColor = null, ?string $foregroundColor = null, ?string $text = null, bool $allowUpScaling = true) { parent::__construct($title, $alt); $this->baseUri = $baseUri; @@ -54,6 +59,12 @@ public function __construct(string $baseUri, ?string $title = null, ?string $alt $this->backgroundColor = $backgroundColor ?? '999'; $this->foregroundColor = $foregroundColor ?? 'fff'; $this->text = $text ?? ''; + $this->allowUpScaling = $allowUpScaling; + } + + public function supportsUpscaling(): bool + { + return $this->allowUpScaling; } /** diff --git a/Classes/Domain/ScalableImageSourceInterface.php b/Classes/Domain/ScalableImageSourceInterface.php index 9883ded..5e72624 100644 --- a/Classes/Domain/ScalableImageSourceInterface.php +++ b/Classes/Domain/ScalableImageSourceInterface.php @@ -6,5 +6,6 @@ interface ScalableImageSourceInterface extends ImageSourceInterface { + public function supportsUpscaling(): bool; public function scale(float $factor): ImageSourceInterface; } diff --git a/Tests/Unit/Domain/AbstractScalableImageSourceTest.php b/Tests/Unit/Domain/AbstractScalableImageSourceTest.php index 4f3ec86..742db19 100644 --- a/Tests/Unit/Domain/AbstractScalableImageSourceTest.php +++ b/Tests/Unit/Domain/AbstractScalableImageSourceTest.php @@ -8,12 +8,20 @@ class AbstractScalableImageSourceTest extends BaseTestCase { + protected $logger = null; + + protected function setUp(): void + { + parent::setUp(); + $this->logger = $this->createMock(\Psr\Log\LoggerInterface::class); + } + /** * @test */ public function aspectRatioIsHonored() { - $dummy = new DummyImageSource('https://example.com', 'Test', 'Test', 400, 400, '999', 'fff', 'Test'); + $dummy = $this->getDummyImageSource(400, 400); $copy = $dummy->withWidth(200, true); $this->assertEquals(200, $copy->height()); } @@ -23,7 +31,7 @@ public function aspectRatioIsHonored() */ public function srcsetIsGenerated() { - $dummy = new DummyImageSource('https://example.com', 'Test', 'Test', 400, 400, '999', 'fff', 'Test'); + $dummy = $this->getDummyImageSource(400, 400); $this->assertEquals( 'https://example.com?w=200&h=200&bg=999&fg=fff&t=Test 200w, https://example.com?w=400&h=400&bg=999&fg=fff&t=Test 400w', $dummy->srcset('200w, 400w') @@ -35,7 +43,7 @@ public function srcsetIsGenerated() */ public function srcsetWithWidthAdheresToDefinition() { - $dummy = new DummyImageSource('https://example.com', 'Test', 'Test', 400, 400, '999', 'fff', 'Test'); + $dummy = $this->getDummyImageSource(400, 400, true); $this->assertEquals( 'https://example.com?w=200&h=200&bg=999&fg=fff&t=Test 200w, https://example.com?w=400&h=400&bg=999&fg=fff&t=Test 400w, https://example.com?w=600&h=600&bg=999&fg=fff&t=Test 600w', $dummy->srcset('200w, 400w, 600w') @@ -45,23 +53,22 @@ public function srcsetWithWidthAdheresToDefinition() /** * If the actual image is smaller than the requested size, then the image should be returned in its original size. * @test - * @todo */ - #public function srcsetWithWidthShouldOutputOnlyAvailableSources() - #{ - # $dummy = new DummyImageSource('https://example.com', 'Test', 'Test', 500, 500, '999', 'fff', 'Test'); - # $this->assertEquals( - # 'https://example.com?w=200&h=200&bg=999&fg=fff&t=Test 200w, https://example.com?w=400&h=400&bg=999&fg=fff&t=Test 400w, https://example.com?w=500&h=500&bg=999&fg=fff&t=Test 500w', - # $dummy->srcset('200w, 400w, 600w') - # ); - #} + public function srcsetWithWidthShouldOutputOnlyAvailableSources() + { + $dummy = $this->getDummyImageSource(500, 500); + $this->assertEquals( + 'https://example.com?w=200&h=200&bg=999&fg=fff&t=Test 200w, https://example.com?w=400&h=400&bg=999&fg=fff&t=Test 400w, https://example.com?w=500&h=500&bg=999&fg=fff&t=Test 500w', + $dummy->srcset('200w, 400w, 600w') + ); + } /** * @test */ public function srcsetWithRatioAdheresToDefinition() { - $dummy = new DummyImageSource('https://example.com', 'Test', 'Test', 400, 200, '999', 'fff', 'Test'); + $dummy = $this->getDummyImageSource(400, 200); $copy = $dummy->withHeight(50, true); $this->assertEquals( 'https://example.com?w=100&h=50&bg=999&fg=fff&t=Test 1x, https://example.com?w=200&h=100&bg=999&fg=fff&t=Test 2x, https://example.com?w=300&h=150&bg=999&fg=fff&t=Test 3x', @@ -72,15 +79,71 @@ public function srcsetWithRatioAdheresToDefinition() /** * If the actual image is smaller than the requested size, then the image should be returned in its original size. * @test - * @todo */ - #public function srcsetWithRatioShouldOutputOnlyAvailableSources() - #{ - # $dummy = new DummyImageSource('https://example.com', 'Test', 'Test', 30, 12, '999', 'fff', 'Test'); - # $copy = $dummy->withWidth(20, true); - # $this->assertEquals( - # 'https://example.com?w=20&h=8&bg=999&fg=fff&t=Test 1x, https://example.com?w=30&h=12&bg=999&fg=fff&t=Test 1.5x', - # $copy->srcset('1x, 2x') - # ); - #} + public function srcsetWithRatioShouldOutputOnlyAvailableSources() + { + $dummy = $this->getDummyImageSource(30, 12); + $copy = $dummy->withWidth(20, true); + $this->assertEquals( + 'https://example.com?w=20&h=8&bg=999&fg=fff&t=Test 1x, https://example.com?w=30&h=12&bg=999&fg=fff&t=Test 1.5x', + $copy->srcset('1x, 2x') + ); + } + + /** + * Log a warning if the descriptors are mixed between width and factor + * @test + */ + public function srcsetShouldWarnIfMixedDescriptors() + { + $dummy = $this->getDummyImageSource(650, 320); + $this->logger->expects($this->once())->method('warning')->with($this->equalTo('Mixed media descriptors are not valid: [1x, 100w]')); + + $dummy->srcset('1x, 100w'); + } + + /** + * Skip srcset descriptor if it does not match the first matched descriptor + * @test + */ + public function srcsetShouldSkipMixedDescriptors() + { + $dummy = $this->getDummyImageSource(500, 300); + $this->assertEquals( + 'https://example.com?w=200&h=120&bg=999&fg=fff&t=Test 200w, https://example.com?w=440&h=264&bg=999&fg=fff&t=Test 440w', + $dummy->srcset('200w, 1x, 440w') + ); + } + + /** + * Log a warning if the descriptor is invalid + * @test + */ + public function srcsetShouldWarnIfMissingDescriptor() + { + $dummy = $this->getDummyImageSource(30, 12); + $this->logger->expects($this->once())->method('warning')->with($this->equalTo('Invalid media descriptor "1a". Missing type "x" or "w"')); + + $dummy->srcset('1a, 10w'); + } + + /** + * Should skip srcset descriptor if either width w or factor x is missing + * @test + */ + public function srcsetShouldSkipMissingDescriptors() + { + $dummy = $this->getDummyImageSource(200, 400); + $this->assertEquals( + 'https://example.com?w=100&h=200&bg=999&fg=fff&t=Test 100w, https://example.com?w=200&h=400&bg=999&fg=fff&t=Test 200w', + $dummy->srcset('100w, 150, 200w') + ); + } + + protected function getDummyImageSource($width, $height, $allowUpScaling = false) + { + $dummy = new DummyImageSource('https://example.com', 'Test', 'Test', $width, $height, '999', 'fff', 'Test', $allowUpScaling); + $this->inject($dummy, 'logger', $this->logger); + return $dummy; + } } \ No newline at end of file