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.

This was an unrelated issue and solved elswhere

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?

@QuyTon can I do anything for this PR? I forgot make a separate branch for PR and then I can't do another one till this PR will be merged ((

@QuyTon @rdeutz @brianteeman @laoneo Joomla 6.0.0 beta1 is coming in 2 days. It means a feature freeze for J6. This PR was succesfully tested and marked as update requested - so it should not be tested again. Is there anything else required for this PR? It has been opened in february...

the people to tag are the release leads @softforge @Bodge-IT

the people to tag are the release leads @softforge @Bodge-IT

Thank you!

Hi @sergeytolkachyov, this only hit our radar on Sunday and we were locked by then. We had opinion this still needed resolutions.

Hi @sergeytolkachyov, this only hit our radar on Sunday and we were locked by then. We had opinion this still needed resolutions.

Can I do anything for this pr or it will wait for j6.1?

Hi @sergeytolkachyov, this only hit our radar on Sunday and we were locked by then. We had opinion this still needed resolutions.

Can I do anything for this pr or it will wait for j6.1?
We are looking into it and will have a decision tomorrow, but appreciate the work you have done to date, thank you

We are looking into it and will have a decision tomorrow, but appreciate the work you have done to date, thank you

If you have any questions, feel free to ask me. I myself see several potential ways to improve consistency, but this will generally require discussion with other developers before implementation.

Dear @sergeytolkachyov, first, thank you for all your work and patience so far.
We had a look through the code and see there are still some comments to be resolved, and then testing as there have been commits since the last tests.
Beta 1 has passed, so we cannot include it, but we would like to work with you over the next few weeks to get any comments satisfied and then test it in PBF if it's ready for that or straight after, and to help you get it into 6.1 early so you are not waiting 6 months for a decision but it could be one of the first things ready for the 6.1 managers to merge

This pull request has been automatically rebased to 6.1-dev.

Beta 1 has passed, so we cannot include it, but we would like to work with you over the next few weeks to get any comments satisfied and then test it in PBF if it's ready for that or straight after, and to help you get it into 6.1 early so you are not waiting 6 months for a decision but it could be one of the first things ready for the 6.1 managers to merge

6.0.0 has been released. I'm waiting for feedback

6.0.0 has been released. I'm waiting for feedback

We will be happy to take a look and give you feedback.

6.0.0 has been released. I'm waiting for feedback

We will be happy to take a look and give you feedback.

Hello, just a reminder.

6.0.0 has been released. I'm waiting for feedback

We will be happy to take a look and give you feedback.

Hello. What can I do for this PR and when?

it's not perfect that you see all fields for all types but ok for now

if you selected a documents and images types together then for images you need this fields, but not need for documents.

So I think it would be better to rename a imagefile field input to file but it is a b/c break.

it's not perfect that you see all fields for all types but ok for now

if you selected a documents and images types together then for images you need this fields, but not need for documents.

So I think it would be better to rename a imagefile field input to file but it is a b/c break.

wouldn't change it to imagefile now could be done in a x.0 release but not in a minor.

I have tested this item ✅ successfully on 887919c

I have tested this item ✅ successfully on 887919c

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

Thank you a lot, but we don't need any user tests for now for this PR.

@sergeytolkachyov thanks for the revert, could you fix the code style?
Since we use heredoc you can indent the full block so it doesn't start left instead under the variable name. But I can life with the existing indent. Just fix the cs please

@sergeytolkachyov thanks for the revert, could you fix the code style? Since we use heredoc you can indent the full block so it doesn't start left instead under the variable name. But I can life with the existing indent. Just fix the cs please

That is why I dislike heredoc syntax)) One more space and all are crashed)))

I'll fix it

@sergeytolkachyov can you fix it please? I would like to get some tests and merge this before alpha3

@sergeytolkachyov can you fix it please? I would like to get some tests and merge this before alpha3

Done. Check it, please.

I still have all the same problems I reported earlier #45013 (comment)

Video Recorded Today

@sergeytolkachyov can you address the things brian mentioned? about the "image" label for all types and img-tag even for not images? and the placeholder text? I don't care about the additional fields in the "all" types variant, where you might add an image description for a audio file, we can't solve this easily so I would accept this for now. But the other things should be addressed before merging.

thanks

img-tag even for not images

I've already written before that I can't reproduce this problem for myself. I have tried to apply PR on several Joomla installations and this problem is not reproduced anywhere. It was at the beginning of the work on the code, but I remember that I eliminated it.

about the "image" label for all types

I think it's possible to add field name construction by media type here. But this will require returning to using SimpleXMLElement, as it is more convenient to work with it within the framework of this task.

When you don't work on a task for a long time, the context and why a particular solution was applied are forgotten. I've been touching this code myself for almost six months now...

Why do we even allow File Types to be a multiple selection?

Surely it would be easier for users AND for development if there were multiple fields (images, documents, audio and vidoe) instead of trying to make a one size fits all solution which requires multiple compromises to make it all work in one field

Why do we even allow File Types to be a multiple selection?

Surely it would be easier for users AND for development if there were multiple fields (images, documents, audio and vidoe) instead of trying to make a one size fits all solution which requires multiple compromises to make it all work in one field

Because we have one media type field plugin, which historically allowed us to specify only one type - images. Therefore, it is logical to add new types to it. If you create separate plugins and field classes for Mediaaudio, mediadocument, mediavideo, then it would be logical to rename the existing plugin to mediaimage or something like that. But this is a backward compatibility bummer. I can't guess which way is easier: one mega-field or 4 separated ones.

Whichever way you look at it this pull request is not suitable (yet) for merging. For me the easiest solution is to create new field plugins for the additional types as that will easily let you resolve the roadblocker problems that currently exist

Whichever way you look at it this pull request is not suitable (yet) for merging. For me the easiest solution is to create new field plugins for the additional types as that will easily let you resolve the roadblocker problems that currently exist

I think I'll return SimpleXMLElement as the basis for forming XML (for correctly inserting the language constant into the default value) and add the construction of language constants so that all selected types are specified in the field name: image, image, document, video, audio, image, video, audio and so on (@HLeithner, I need your comment too).

The next step may be to check the selected file type by extension in the browser. And if it is an image, then you can show additional fields for the image. If it is a document, show a field for the text of the download link, similar to showon. After that, the only problem will be (in my opinion) the name of the imagefile field, which may not correspond to the audio file or video. This can be solved later when moving to the next major version of Joomla by deprecating the imagefile name and adding the file property. The data will be duplicated in both fields for a while.

It's been almost a year and I don't want to lose the work I've done. Making 3 separate additional plugins is more work at this stage, which I won't have time to do before the alpha 3 version. Thus, the adoption of this PR will be postponed to autumn 2026. This, to put it mildly, is not motivating at all.

@sergeytolkachyov its not motivating for testers either when there feedback is ignored for so long