agoat / contao-deferredimages-bundle

Contao 4 deferred images library
GNU Lesser General Public License v3.0
1 stars 0 forks source link

4.6.x cannot redeclare class Image #9

Closed reluem closed 5 years ago

reluem commented 6 years ago

Since Contao 4.6 I get the following exception:

Uncaught PHP Exception ErrorException: "Warning: Cannot declare class Image, because the name is already in use" at /xx/xx/xx/xx/vendor/agoat/contao-deferredimages/src/Resources/contao/config/config.php line 17

Any fix for this? Thank you!

agoat commented 6 years ago

There are many changes in contao 4.6, so I am not surprised that an error occurs without adjustments.

I am currently working on another project, so I can't look into it right now. It' s going to take a while.

reluem commented 6 years ago

commenting the class_alias in the config.php works for now. // class_alias('Agoat\DeferredImagesBundle\Contao\VirtualImage', 'Image');

agoat commented 6 years ago

commenting the class_alias in the config.php works for now. // class_alias('Agoat\DeferredImagesBundle\Contao\VirtualImage', 'Image');

Thank you for the workaround.

fritzmg commented 6 years ago

But this will disable its functionality, won't it?

reluem commented 6 years ago

seems to be still working on my system. When clearing tl_image_deferred in the maintenance settings, the files are successfully recreated on page load.

The symfony debug, however, does indeed not show the alialised image class...

fritzmg commented 6 years ago

the files are successfully recreated on page load.

Well, they would be anyway ;). The question is if they are generated in a deferred manner, i.e. on browser request for each resource.

reluem commented 6 years ago

The main advantage of this module is that the image resize/generation is not done during the page load which could lead to a timeout on some systems, right? I have galleries with 50+ images which are generated when accessed, meaning that after the intial page load images are showing up slightly later. => So I would assume that the 'deferred' part is working...

I am a bit confused though since the code assumes deferred images to be in path /assets/images/g (VirtualImage.php#L76), which is not the case for the above mentioned galleries.

So I could certainly be that the module is in fact not working correctly with the commented class_alias.

fritzmg commented 6 years ago

The main advantage of this module is that the image resize/generation is not done during the page load which could lead to a timeout on some systems, right?

Exactly.

I have galleries with 50+ images which are generated when accessed, meaning that after the intial page load images are showing up slightly later.

Yes, that would mean it is working :)

I am a bit confused though since the code assumes deferred images to be in path /assets/images/g (VirtualImage.php#L76), which is not the case for the above mentioned galleries.

The resized images (for the thumbnails for example) are in that folder. This is normal.

reluem commented 6 years ago

so far so good then, still the issue with the folder:

The resized images (for the thumbnails for example) are in that folder. This is normal.

the thumbnails from my galleries are not in the/g/folder (example path: assets/images/0/20171216_141321-405c9dc2.jpg

fritzmg commented 6 years ago

Oh you meant the g folder specifically. This folder does not actually exist, it is not used by Contao. I suppose that is why @agoat used it for the deferred images.

agoat commented 6 years ago

This folder does not actually exist, it is not used by Contao. I suppose that is why @agoat used it for the deferred images.

Exactly. The folder g is not used by Contao itself, but is used from this extension as a virtual folder, which is forwarded to the Deferred Image Controller, who calculates the images, stores them in the 0-f folders and then sends them.

Since Contao integrates more and more Symfony, it may work without the class_alias command now, but that would be a surprise to me. After resizing they are stored like normal images, unless the image folder itself is pruned and they have to be recalculated.

Unfortunately I don't have time to take a closer look for now.

fritzmg commented 6 years ago

Wouldn't the executeResize hook be sufficient instead of replacing the Image class with class_alias?

agoat commented 6 years ago

Wouldn't the executeResize hook be sufficient instead of replacing the Image class with class_alias?

The executeResize hook is depreciated. The preferred (new) method will be to define a custom Image Service in the Symfony settings.

And I had to start much further ahead, because the executeResize hook is called inside the Image class. But I can't say exactly right now.

fritzmg commented 6 years ago

The executeResize hook is depreciated.

So is class replacing ;). Hooks will still work in all Contao 4 versions.

And I had to start much further ahead, because the executeResize hook is called inside the Image class.

Should be sufficient. All you have to do is return the virtual path there.

The preferred (new) method will be to define a custom Image Service in the Symfony settings.

Yeah, that's better anyway.

agoat commented 6 years ago

So is class replacing ;). Hooks will still work in all Contao 4 versions.

I know class replacing is bad practice. I can't remember why, but it didn't worked with the hook and returning the virtual path wasn't enough.

I will definitely look at it after the deadline from my customer project, probably November.

agoat commented 5 years ago

The new version 1.4.0 is now compatible with Contao 4.6.

Some extra info: The class_alias is needed (and that's what breaks it in Contao 4.6 - see https://github.com/contao/core-bundle/commit/1218f01f3888c7bc9279ba55c763b61b2a079c1f) because the DC_Folder class is using the Image class in global namespace (https://github.com/contao/core-bundle/blob/master/src/Resources/contao/drivers/DC_Folder.php#L2780).

reluem commented 5 years ago

Thanks for the update!

the image.php class still contains two dump statements, which should probably be removed ;)

agoat commented 5 years ago

Oh, damn it. I will do as soon as I get back to my computer.. I forgot to test it in production mode.

agoat commented 5 years ago

Fixed.

I replaced the 1.4.0 release. Probably you need to remove and (re)require the package.

reluem commented 5 years ago

there's, however, still another problem.

Error when opening/editing a news archive or article. Regular articles, events and FAQ items work just fine.

For news items:

Attempted to load class "Image" from the global namespace. Did you forget a "use" statement for e.g. "Imagine\Gmagick\Image", "Imagine\Imagick\Image", "Imagine\Gd\Image", "Haste\Image\Image", "Contao\ImagineSvg\Image" or "Contao\Image\Image"?

For news archives:

Symfony\Component\Debug\Exception\FatalErrorException: Error: Uncaught ErrorException: Warning: Cannot declare class Image, because the name is already in use in /www/xxx/xxx/xxx/vendor/contao/core-bundle/src/Resources/contao/library/Contao/Image.php:895

agoat commented 5 years ago

I can not reproduce this on a clean Contao 4.6.12 installation.

Maybe it's a problem in conjunction with another extension. What other extensions are installed?

reluem commented 5 years ago

Hi @agoat,

I think it might be codefog/contao-news_categories. The symfony debugger show this image to be the error source:

Class 'Contao\Image' not found "exception" => FatalThrowableError {#324 -originalClassName: "Error"

message: "Class 'Contao\Image' not found"

#code: 0
#file: "…/vendor/codefog/contao-news_categories/src/Widget/NewsCategoriesPickerWidget.php"
#line: 49
#severity: E_ERROR

}

Edit: Actually, this happens every time another extension uses image::, I have the same error with terminal42/contao-rootcontent as well.

Can you reproduce this?

Thank you!

agoat commented 5 years ago

OK, it looks like some extensions are using the \Contao\Image class directly. Contao core is using \Image in global namespace.

agoat commented 5 years ago

Edit: Actually, this happens every time another extension uses image::, I have the same error with terminal42/contao-rootcontent as well.

It depends on what they set in a use statement for the class name Image which will be used when calling Image:: ...

reluem commented 5 years ago

true. so do you think this is something you can fix, or do the other extension developers need to change to the global namespace?

fritzmg commented 5 years ago

The global namespace should not be used.

@agoat shouldn't it be possible to simply use the getImage hook instead?

agoat commented 5 years ago

so do you think this is something you can fix, or do the other extension developers need to change to the global namespace?

I added an extra class alias to route to the overwritten Image class. Think that should do it (untested by now). Can you test it by installing the master branch composer require agoat/contao-deferredimages:dev-master.

reluem commented 5 years ago

hi @agoat, seems to be working fine now. Thanks for the effort!

agoat commented 5 years ago

Perfect. I will make a release so you can install regularly.