silverstripe / silverstripe-assets

Silverstripe Assets component
BSD 3-Clause "New" or "Revised" License
9 stars 66 forks source link

Unable to access LazyLoad setting from extensions applied to Image #513

Closed kinglozzer closed 1 year ago

kinglozzer commented 2 years ago

Example:

<?php

namespace App\Extensions\Assets;

use SilverStripe\Core\Extension;

class ImageExtension extends Extension
{
    public function Test()
    {
        var_dump($this->getOwner()->IsLazyLoaded());
    }
}

Calling $Image.LazyLoad(false).Test in a template will always output true.

This seems to be because the ImageManipulation trait is applied to both File (Image extends File) and DBFile. Calling .LazyLoad(false) calls the following code:

https://github.com/silverstripe/silverstripe-assets/blob/ce9464b6282c34e37324987ea2dd74cae89c50ca/src/ImageManipulation.php#L1101-L1111

This then returns the a DBFile instance with the attribute set, but the attribute is never set on the Image object itself. Setting the attributes on the Image object via $file->getFailover()->initAttributes() fixes this, but goes against the documentation for ImageManipulation::setAttribute() which states the method is supposed to be immutable

PRs

GuySartorelli commented 2 years ago

I'm just trying to wrap my head around the implications of this - do you have a more realistic example of where this would cause a problem?

kinglozzer commented 2 years ago

I'm just trying to wrap my head around the implications of this - do you have a more realistic example of where this would cause a problem?

So my use case is this module, the lazyload setting currently isn’t copied through so I can’t enable/disable loading=lazy for specific images, only globally. Any extension that needs to know if an image is lazy loaded or not would be affected though

GuySartorelli commented 1 year ago

Fixed in 1.12.1