I have tested this item ✅ successfully on 9a36d9b

I have tested this item ✅ successfully on 9a36d9b

I have tested this item ✅ successfully on 9a36d9b

I have tested this item ✅ successfully on dff217a

I have tested this item ✅ successfully on dff217a

@hans2103 can you test again, please?

I have tested this item ✅ successfully on dff217a

I have tested this item ✅ successfully on dff217a

I have tested this item ✅ successfully on 8a2e00b

I have tested this item ✅ successfully on 8a2e00b

I have tested this item ✅ successfully on 8a2e00b

Nice job!

I am sorry but unless I misunderstand something this creates a very confusing UI in the admin and broken output in the front end and I dont see how any of you can have tested this successfully

I have tested this item 🔴 unsuccessfully on 8a2e00b

@sergeytolkachyov I have removed the successful test result you posted as it is assumed that the author would not submit something that didnt work and only tests from other people are valid

When testing this PR you need to actually test what the field produces and not the limited tests in the original post. In summary you might be able to select a document or a video but this field will NOT be able to display the document or video

When testing this PR you need to actually test what the field produces and not the limited tests in the original post. In summary you might be able to select a document or a video but this field will NOT be able to display the document or video

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45013.

I think all the successful tests were related to checking the very possibility of choosing a file type other than the image. in most cases, the standard layouts are redefined and in the frontend, the field is only required to provide the actual path to the file. and the desired layout is already determined by the developer in a specific override. nevertheless, I agree that the output of pdf or mp3 in the image tag will be incorrect. after all, audio and video can be output as a player, and pdf as <object>, but you can't display docx like <object> anymore. Such a variety of formats will complicate the logic of choosing a layout. would it be enough to output a file (not an image) as a download link?

For files you are correct it will just have to be a link

@brianteeman by default the field name attribute is imagefile, but if we are choosing a documents it will not an imagefile by logic. But I think I will not change a field name because it will be a bc breakes.

@gug2 @hans2103 @viocassel @web-eau-net @brianteeman check it please. Testing instructions has been updated.

I have tested this item ✅ successfully on 02796b9

still testing but looks good

I have tested this item ✅ successfully on 02796b9

👍

gets a bit confused if you select images + another type

php warnings

Create a field for documents and in an article select a pdf and save
The following php warning
[27-Feb-2025 14:24:45 UTC] PHP Warning: mime_content_type(images/RL%20application%202017.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

Same for video
Feb-2025 14:24:08 UTC] PHP Warning: mime_content_type(images/sampledata/output.mp4): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

From an accessibility perspective the default text for every link is "download"
This can result in multiple links on the same page with the exact same details - which is an accessibility failure

An option might be to do it the way we do a read more link. eg Download < Filename >

Tagging @chmst for their a11y input

@viocassel how are you testing this?

php warnings

Create a field for documents and in an article select a pdf and save The following php warning [27-Feb-2025 14:24:45 UTC] PHP Warning: mime_content_type(images/RL%20application%202017.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

I can't repeat the problem. Perhaps this is a MediaHelper and location issue on Windows.

Same for video Feb-2025 14:24:08 UTC] PHP Warning: mime_content_type(images/sampledata/output.mp4): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

gets a bit confused if you select images + another type

Yes, I agree. Showon comes to mind here, but there is no substring search so that you can show the rest of the fields based on the value of the main one.

for video and audio I would expect the output to be a player not a download.

see

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/video
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/audio

for video and audio I would expect the output to be a player not a download.

see

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/video https://developer.mozilla.org/en-US/docs/Web/HTML/Element/audio

To do this, I will need to add new layouts. However, even for pdf, you will want to show them as an <object> instead of a download link. Also, each file type may require its own additional data and settings specific to that particular file type. For videos, for example, the poster attribute or specifying a file in a different format (mp4, webm etc.). This can greatly complicate the logic of choosing the output layout. I assumed that at this point, the very fact that you can choose something other than pictures is already a good thing. And the rest can be solved using override.

What doesn't it like?

If a user selects an image and it displays an image then when they select a video they would expect it to display a video etc overrides are for customising the output with a layout is fine but it should work out of the box without an override.

If a user selects an image and it displays an image then when they select a video they would expect it to display a video etc overrides are for customising the output with a layout is fine but it should work out of the box without an override.

So we need a isVideo, isDocument and isAudio methods for MediaHelper... And check mime types too.
Because in plugins/fields/media/tmpl/media.php I got a field params (with all file types configured for it) and field value. But I don't get the exact file type in the layout. Therefore, I need to define it, and then generate the necessary options for the render.

Alternatively, I can realize this check in the plugins/fields/media/tmpl/media.php for now, and then add the isAudio, isDocument and isVideo methods to MediaHelper. After adding these methods, it will be possible to simplify the verification in the plugin layout.

If a user selects an image and it displays an image then when they select a video they would expect it to display a video etc overrides are for customising the output with a layout is fine but it should work out of the box without an override.

I added this. Check it please.

Testing instructions updated

@brianteeman @hans2103 @QuyTon

@gug2 @web-eau-net @hans2103 @viocassel can you test again?

I have tested this item ✅ successfully on 887919c

still get php warning on non-image media

example

Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/rl.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99
[03-Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/sampledata/output.mp4): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

still get php warning on non-image media

example

Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/rl.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99
[03-Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/sampledata/output.mp4): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

I can't reproduce this issue both on Windows local server and remote Linux server. This is my result for pdf file.


This is for mp3

I came across with this issue only one time when the filename has contained a space. But this is not a problem of this PR, it is a MediaHelper problem that needs to be solved separately.

still get php warning on non-image media

example

Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/rl.pdf): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99
[03-Mar-2025 12:37:12 UTC] PHP Warning:  mime_content_type(images/sampledata/output.mp4): Failed to open stream: No such file or directory in D:\repos\j51\libraries\src\Helper\MediaHelper.php on line 99

Can you reproduce this issue on other Joomla installation?

@QuyTon is there a ready-made preset of the Joomla code style for Php Storm? I use PSR-12 for formatting code, but at the same time, the alignment of the code does not occur as (as far as I understand) it is required in the Joomla core. This results in a lot of unnecessary commits.

@brianteeman

All new feature have to go into 6.0, rebased the PR, sorry

All new feature have to go into 6.0, rebased the PR, sorry

https://joomlacommunity.cloud.mattermost.com/main/pl/ja16arwf7jrrxkqfmxs8dqo6dw

Is there any generally accepted approach in this regard? In the semver yes, this PR should get into 6.0. However, I was informed in the chat that there was a chance to get into 5.3 (see screenshot). I decided to abandon a third-party plugin on my projects that implements this functionality and add it to the core. And I tried to make it just because I was informed that it was possible to add it to 5.3, which will be released soon (April). I'm not sorry for the work done, and I'm glad that after successful testing, I hope it will be included in the core. But now I have to wait until the fall and use a third-party extension... And also make sure that maybe some other PR can do about the same thing or make changes that may affect the already tested code.

not sure what you are asking me for. I tested this and reported a bug and there have been no changes by you since then so nothing will have changed

not sure what you are asking me for. I tested this and reported a bug and there have been no changes by you since then so nothing will have changed

I can't reproduce this issue both on Windows local server and remote Linux server. I have tested 4 different Joomla installations. Here #45013 (comment) I show how the MediaHelper method works correctly for me. I asked 2 people to test it carefully and they also did not have such a problem. Therefore, I ask you to check the work on another Joomla installation.

I also note that this problem occurs in the MediaHelper::getMimeType method and not directly in the submitted code. And it seems to me that a separate PR should be created to solve it so that everything is not mixed up in the current one.

I will not be testing it on another server. I use one server for all my joomla tests and development. It's up to maintainers to decide if my test is good or not

Can anyone else reproduce this bug?

Regarding the issue from Brian. Looks like you use a relative path. Also keep in mind that there exist none local filesystem plugins.

This needs to be tested with a 3rd party extension like DPMedia or so.

This needs to be tested with a 3rd party extension like DPMedia or so.

As I remember DP Media make copy of remote file to local filesystem. I'll try to use it

This needs to be tested with a 3rd party extension like DPMedia or so.

The MediaHelper::getMimeTypes() method does require an absolute path to the file and therefore there is a problem with remote files, but I cannot ignore this method because of the SVG files...

DPMedia makes only a copy to the local filesystem when the service doesn't hat public http based urls for images, like the FTP adapter. The Google implementation does link directly to the file on google drive. This is what the url attribute does then contain.

DPMedia makes only a copy to the local filesystem when the service doesn't hat public http based urls for images, like the FTP adapter. The Google implementation does link directly to the file on google drive. This is what the url attribute does then contain.

I try to detect external file url this way (Uri::isInternal() can't detect simple local path to file like files/file.pdf)

$fileUrl = MediaHelper::getCleanMediaFieldValue($field->value['imagefile']);

// detect local file path
$isLocalFile = false;
if(empty((new Uri($fileUrl))->getHost()))
{
    $fileUrl = JPATH_SITE.DIRECTORY_SEPARATOR.$fileUrl;
    $isLocalFile = true;
}

if($isLocalFile && !\is_file($fileUrl))
{
    return;
}
// ....

Then check a Mime type

if (MediaHelper::isImage($fileUrl) || ($isLocalFile && MediaHelper::getMimeType($fileUrl) === 'image/svg+xml')) {
    $options = [
        'src' => $field->value['imagefile'],
        'alt' => empty($field->value['alt_text']) && empty($field->value['alt_empty']) ? false : $field->value['alt_text'],
    ];

// ...

If we receive an external file, then we can determine whether it is an image, since the definition is simply based on the file extension. But we can't check its Mime type. Therefore, I have to set the logical $isLocalFile flag. This way we will be able to specify local SVG files in the field, but we will not be able to specify remote ones.
Perhaps the best way would be to simply add the svg extension to the list of images in the MediaHelper::isImage() method, but I was guided by the code in the Joomla core, where the MIME type is additionally checked for SVG files.
There is an example

If this way is OK - I'll commit it

Please fix phpcs: https://ci.joomla.org/joomla/joomla-cms/84154/1/7

@hans2103 @web-eau-net @gug2 @viocassel can you test this again?

I have tested this item ✅ successfully on 887919c

I have tested this item ✅ successfully on 887919c

Successful test

From an accessibility perspective the default text for every link is "download" This can result in multiple links on the same page with the exact same details - which is an accessibility failure

An option might be to do it the way we do a read more link. eg Download < Filename >

Tagging @chmst for their a11y input

This accessibility failure must be resolved

retesting this PR and I find that with this PR applied I am unable to upload an image into the root images folder. No error messages :(/ I can still upload to subdirectories

As soon as I revert this PR I can once again use media manager correctly.

From an accessibility perspective the default text for every link is "download" This can result in multiple links on the same page with the exact same details - which is an accessibility failure
An option might be to do it the way we do a read more link. eg Download < Filename >
Tagging @chmst for their a11y input

This accessibility failure must be resolved

Local pdf:

Remote pdf:

Frontend

@rdeutz can I do anything else for this PR?