Using the template manager try to upload an *.ico image such as a favicon
It should fail
Go to the template manager options and add ico to the list of allowed image formats
Try again to upload an *.ico image using the template manager
File uploaded
File Uploaded success message
AND
File not uploaded error message
This is not about if an ico is an image or if it should be used or any other off topic issue. This is purely about getting two contradictory messages. Any off topic post I will request to be removed by the JBS
Labels |
Added:
No Code Attached Yet
|
@brianteeman the problem is that you're adding a proprietary image format that is not supported by either the HTML specs or the PHP GD library and somehow you expect to somehow magically work. Joomla is just leveraging the existing formats, either in the PHP or in the HTML world, so you can't somehow magically add the .psd
format (which is also proprietary) and expect that both PHP and HTML will do something meaningful.
In short supporting the ico is possible (I just created a PR) but adding the .ico
files as supported images is a totally different case and for the HTML specs won't ever happen (due to proprietary nature). Same goes for the PHP which is just adding the formats that the HTML spec support.
you just cant help yourself from completely ignoring the reported issue and going on a rant.
It’s not a rand. What you are reporting is the same thing as me adding a psd file format in the allowed images and expect Joomla to somehow do something meaningful. In the previous issue I had a link to the HTML specs were they state the supported formats and these are the only ones allowed in that field! So basically your issue is a misunderstanding of what is allowed/valid image format for the specific field
you just dont understand - never mind - you will eventually - and then you will realise that you are not talking about the same thing as me
you just don't understand
Ok, could you elaborate
if you try to do something you should get a success or a fail.
you should not get both
you should not get both
The root problem here (and also in the Media Manager) is that these kind of fields shouldn't allow the typing the formats/extensions, etc rather they should had a pool of possible values and let the user pick the ones they want. This had been raised before: #34634 (comment)
In short both the template manager and the media manager should not allow users to pass non valid extensions, formats in the respected fields because that would cause such inconsistent behaviour
Seriously @brianteeman: I dont get whats your point here (as well as in the other, closed, issue)? Oh, and @dgrammatiko is not trying to go on rant here, both his wording and his thinking process are correct here.
The core problem here can be that you look at this from different angle than @dgrammatiko or me. And this may be valid reason of confusion.
So we both (@dgrammatiko and I) asked you for clarification. As a reply you closed original issue and opened this one. For the sake of what?
@wojtekxtx actually Brian is right here, the field should not accept any extension that Joomla could not handle. I was confused because I thought he was asking for support for the ico extension in the allowed/valid image extensions. The key part is the It should fail
that I missed in both issues!
finally ;)
@brianteeman using this pattern, on the field the front end part could fail on invalid extensions. Of course this needs a field validation also in the PHP
^[gif|jpg|jpeg|png|webp|avif]+(,[gif|jpg|jpeg|png|webp|avif]+)*$
@dgrammatiko if Joomla cannot handle extension, than why not add support to it? Instead of opening multiple issues as well as fruitless comments exchange, maybe its time to add support for ico
files to Joomla?
I still fail to see whats wrong in ico
files. @brianteeman: dare to explain?
I still fail to see whats wrong in
ico
files. @brianteeman: dare to explain?
no. it is not related to the reported problem
maybe its time to add support for ico files to Joomla?
The web is moving away from the ico files for the favicon so personally I wouldn't like to see Joomla adding so late support for them (although I already open a PR for that)
@brianteeman related or not, does not matter. You opened this double issue mess, so I want you to explain.
@dgrammatiko Just looked at this issue. The reason of this error is because we are using Image
class to detect size of the uploaded image https://github.com/joomla/joomla-cms/blob/4.2-dev/administrator/components/com_templates/src/Model/TemplateModel.php#L1461 but our Image
class could not handle that ico image type.
One solution is that instead of using Image
class like that, we can just use getimagesize
function to get size instead.
$info = getimagesize(Path::clean($path . $fileName));
if ($info !== false)
{
$image['height'] = $info[0];
$image['width'] = $info[1];
}
else
{
$app->enqueueMessage('Could not detect image size');
}
Just a bit of info from code reading. Don't ask me to make PR because I am not familiar with com_templates, so could easily miss something.
@joomdonation I had come to the same conclusion (Image
class was breaking for ico) #39861 (comment)
FWIW I had a PR adding support for the .ico
files #39866 though it needs some js, ie a canvas and a custom decoder for displaying the icon (stackoverflow?) but the issue Brian is reporting has to do with non existing validation of the allowed image extensions. You could add PSD, TIFF or whatever other image format and still have the same double messages...
@dgrammatiko I spent sometime to play with com_templates today and understand a bit more how it works. So for this issue, the error happens because after the file uploaded, we redirect user to image editing page to allow them to edit further but the image is not supported by our image API
How about after the file uploaded, if it is one of the image types which can be edited, we redirect them to image editing page? Otherwise, redirect them back to template files management page. So the code on this line https://github.com/joomla/joomla-cms/blob/4.2-dev/administrator/components/com_templates/src/Controller/TemplateController.php#L533 could be changed to:
if (in_array(File::getExt($return), ['gif', 'jpg', 'jpeg', 'png', 'webp', 'avif'])) {
$url = 'index.php?option=com_templates&view=template&id=' . $id . '&file=' . $redirect . '&isMedia=' . $this->input->getInt('isMedia', 0);
} else {
$url = 'index.php?option=com_templates&view=template&id=' . $id;
}
Also, on this block of code https://github.com/joomla/joomla-cms/blob/4.2-dev/administrator/components/com_templates/src/View/Template/HtmlView.php#L189-L195 , we should also catch InvalidArgumentException
so that if users attempt to edit one of the none supported image types, they will see a warning messages instead of seeing an error from uncatch Exception at the moment. Something like:
try {
$this->image = $this->get('Image');
$this->type = 'image';
} catch (\RuntimeException $exception) {
$app->enqueueMessage(Text::_('COM_TEMPLATES_GD_EXTENSION_NOT_AVAILABLE'));
$this->type = 'home';
}
catch (\InvalidArgumentException $exception) {
$app->enqueueMessage('Editing this image type is not supported');
$this->type = 'home';
}
Labels |
Added:
bug
|
Labels |
Added:
PBF
|
@joomdonation that explains the double (and conflicting messages) and looks like a sensible solution
Labels |
Removed:
PBF
|
closing
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-12-03 11:40:00 |
Closed_By | ⇒ | brianteeman |
There's a PR: #39866