Joomla! Issue Tracker | Joomla! CMS #32326 - [4.0] Fix Media manager for multilingual sites

NPM Resource Changed ?

Closed
14 Feb 2021
Medium
Build: 4.0-dev
# 32326
Diff
dgrammatiko:patch-10

Pending

User tests: Successful: Unsuccessful:

dgrammatiko
6 Feb 2021

Pull Request for Issue #32288 .

Summary of Changes

Corrects the URL for media manager

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

73b19f7 6 Feb 2021

Fix multilingual

dgrammatiko - open - 6 Feb 2021

dgrammatiko - change - 6 Feb 2021

Status

New

⇒

Pending

joomla-cms-bot - change - 6 Feb 2021

Category

JavaScript Repository NPM Change

⇒

JavaScript Administration com_media NPM Change Repository

dd20bcb 6 Feb 2021

Update image.vue

ccfc341 6 Feb 2021

image

infograf768 - comment - 6 Feb 2021

@dgrammatiko
Sorry, but this does not help.
I still get GET http://localhost:8888/newfolder/joomla40/fr/administrator/undefined

infograf768 - comment - 6 Feb 2021

Along my tests on that test site I had added 2 files in the images folder
file_example.mp4
login.svg

Deleting these solves my issue. No more GET

remains to find why...

dgrammatiko - comment - 6 Feb 2021

Deleting these solves my issue. No more GET

Does this happen only on multilingual or on normal sites as well?

infograf768 - comment - 6 Feb 2021

Does this happen only on multilingual or on normal sites as well?

testing now. give me a sec

dgrammatiko - comment - 6 Feb 2021

The media manager is missing a lot of file types:

Updating:

Legal Extensions (File Types): mp4,svg,bmp,csv,doc,gif,ico,jpg,jpeg,odg,odp,ods,odt,pdf,png,ppt,txt,xcf,xls,BMP,CSV,DOC,GIF,ICO,JPG,JPEG,ODG,ODP,ODS,ODT,PDF,PNG,PPT,TXT,XCF,XLS
Legal Image Extensions (File Types): svg,bmp,gif,jpg,png
Legal MIME Types: video/mp4,image/svg+xml,image/jpeg,image/gif,image/png,image/bmp,application/msword,application/excel,application/pdf,application/powerpoint,text/plain,application/x-zip

infograf768 - comment - 6 Feb 2021

I have the issue when monolanguage or multilanguage when media parameters were in my test site

If I leave the default params, the svg and mp4 don't show at all as placeholders in the manager and I do not have the GET

dgrammatiko - comment - 6 Feb 2021

@infograf768 for starters this

joomla-cms/administrator/components/com_media/src/Model/ApiModel.php

Line 543 in f856578

$extension = substr($path, strrpos($path, '.') + 1);

is not the best way to get the extension, there's a PHP native method for this https://www.php.net/manual/en/function.pathinfo.php

Also can you var_dump($e) or get a breakpoint here:

joomla-cms/plugins/filesystem/local/src/Adapter/LocalAdapter.php

Line 367 in f856578

    
           // Ignore the exception - it's an image that we don't know how to parse right now

Sorry, I still can't replicate it

infograf768 - comment - 7 Feb 2021

The path is correctly obtained. Tested by deleting the svg.
I have no problem editing the usual files (jpg, etc.) with this PR or without it.

var_dump($e); gives nothing.

Fedik - comment - 7 Feb 2021

this is bug for SVG preview , it does not have a correct preview url:

Media manager fail to make a preview, somewhere deep in code (client side or both sides)

infograf768 - comment - 7 Feb 2021

and same for mp4 I guess.

Fedik - comment - 7 Feb 2021

mp4 works fine, it display "file-icon".

btw, you should not add mp4 to legal images, because it not an image ?
this may be a reason if you have this bug with mp4

infograf768 - comment - 7 Feb 2021

mp4 works fine, it display "file-icon".

nope

and I have video/mp4 for mp4 in Legal MIME Types

but I understand that J does not like mp4 ;)

Fedik - comment - 7 Feb 2021

yes, it because mp4 not an image.

I think the code need to be smarter, and do not crash even if user add doc,xls,pdf as "legal images". Enough to show a "file icon" or another random icon.

dgrammatiko - comment - 7 Feb 2021

@infograf768 @Fedik I guess someone needs to do the remain tasks from #31233
See my comment there #31233 (comment)

infograf768 - comment - 7 Feb 2021

hmm
we have a specific video vue
administrator/components/com_media/resources/scripts/components/browser/items/video.vue

and mp4 is all over the place in constants

/Applications/MAMP/htdocs/newfolder/joomla40/administrator/components/com_media/resources/scripts/components/browser/items/item.es6.js:21:       const videoExtensions = ['mp4'];
/Applications/MAMP/htdocs/newfolder/joomla40/administrator/components/com_media/resources/scripts/components/browser/items/item.js:20:             let videoExtensions = ['mp4'];
/Applications/MAMP/htdocs/newfolder/joomla40/administrator/components/com_media/resources/scripts/components/browser/items/row.vue:73:       const extensionWithPreview = ['jpg', 'jpeg', 'png', 'gif', 'mp4'];

/Applications/MAMP/htdocs/newfolder/joomla40/libraries/vendor/guzzlehttp/psr7/src/MimeType.php:81:             'mp4' => 'video/mp4',

dgrammatiko - comment - 7 Feb 2021

and mp4 is all over the place in constants

The guzzlehttp is a composer dependency so this is something the project can't directly change. Also I have no clue if this is even used here

About the administrator/components/com_media/resources/scripts/components/browser/items/video.vue file this was exactly what I was referring in #31233 (comment) The hard coded parts there are really bad

EDIT: One more thing here, the local adapter, as far as I understand the code, is only returning info for images:

joomla-cms/plugins/filesystem/local/src/Adapter/LocalAdapter.php

Lines 325 to 372 in f856578

    
           private function getPathInformation(string $path): \stdClass 
        
           { 
        
           	// Prepare the path 
        
           	$path = Path::clean($path, '/'); 
        
           	// The boolean if it is a dir 
        
           	$isDir = is_dir($path); 
        
           	$createDate   = $this->getDate(filectime($path)); 
        
           	$modifiedDate = $this->getDate(filemtime($path)); 
        
           	// Set the values 
        
           	$obj            = new \stdClass; 
        
           	$obj->type      = $isDir ? 'dir' : 'file'; 
        
           	$obj->name      = $this->getFileName($path); 
        
           	$obj->path      = str_replace($this->rootPath, '', $path); 
        
           	$obj->extension = !$isDir ? File::getExt($obj->name) : ''; 
        
           	$obj->size      = !$isDir ? filesize($path) : ''; 
        
           	$obj->mime_type = MediaHelper::getMimeType($path, MediaHelper::isImage($obj->name)); 
        
           	$obj->width     = 0; 
        
           	$obj->height    = 0; 
        
           	// Dates 
        
           	$obj->create_date             = $createDate->format('c', true); 
        
           	$obj->create_date_formatted   = HTMLHelper::_('date', $createDate, Text::_('DATE_FORMAT_LC5')); 
        
           	$obj->modified_date           = $modifiedDate->format('c', true); 
        
           	$obj->modified_date_formatted = HTMLHelper::_('date', $modifiedDate, Text::_('DATE_FORMAT_LC5')); 
        
           	if (MediaHelper::isImage($obj->name)) 
        
           	{ 
        
           		// Get the image properties 
        
           		try 
        
           		{ 
        
           			$props       = Image::getImageFileProperties($path); 
        
           			$obj->width  = $props->width; 
        
           			$obj->height = $props->height; 
        
           			// Todo : Change this path to an actual thumbnail path 
        
           			$obj->thumb_path = $this->getUrl($obj->path); 
        
           		} 
        
           		catch (UnparsableImageException $e) 
        
           		{ 
        
           			// Ignore the exception - it's an image that we don't know how to parse right now 
        
           		} 
        
           	} 
        
           	return $obj; 
        
           }

This can't be right

Fedik - comment - 7 Feb 2021

There need to hardcode "previewable" file types, it should not depend from User config.
So User can enter anything in to "Images", and system only do preview only for "previewable".

dgrammatiko - comment - 7 Feb 2021

So User can enter anything in to "Images", and system only do preview only for "reviewable".

I'm not sure that I agree here. Eg I'm the admin and I disable .webp I would expect the system to respect my choice and use the placeholder icon for any .webp image

richard67 - comment - 7 Feb 2021

So User can enter anything in to "Images", and system only do preview only for "reviewable".

I'm not sure that I agree here. Eg I'm the admin and I disable .webp I would expect the system to respect my choice and use the placeholder icon for any .webp image

So it needs to check both arrays, the allowed images and the previewable images, and only if it is in both it shall be previewed.

Or check the intersect of the 2 arrays only one time.

dgrammatiko - comment - 7 Feb 2021

Or check the intersect of the 2 arrays only one time.

I'll go for this one

richard67 - comment - 7 Feb 2021

Who of you two will make the PR? ?

Fedik - comment - 7 Feb 2021

I'm not sure that I agree here. Eg I'm the admin and I disable .webp I would expect the system to respect my choice and use the placeholder icon for any .webp image

You cannot show more than browser able to show.

Add .webp and .avif to list of previewable, but they need an extra care in non supported browsers,

Or check the intersect of the 2 arrays only one time.

It does not make sense, again you cannot show more than browser able to show.

richard67 - comment - 7 Feb 2021

@Fedik It's not about to show more than the browser can do, it's about showing less because an admin wants to exclude a certain type of images from the list of allowed.

dgrammatiko - comment - 7 Feb 2021

It does not make sense, again you cannot show more than browser able to show.

@Fedik it's not about the browser's capabilities, it's about respecting the admin's options. If for whatever reason they don't want to support .gif files these files should not appear in the media manager ?

richard67 - comment - 7 Feb 2021

@Fedik That means we like your idea of the hard-coded pre-defined list, we only like to check the allowed images maintained in backend in addition, so if an image is not in both lists, we don't show a preview.

Fedik - comment - 7 Feb 2021

it's about respecting the admin's options

I am not talking about what is diplaied in the Media browser, admin can show any ? there.

I am about render the preview.
If Admin choose to show png,jpg,webp,pdf,foobar then in Media browser should be visible ALL this files, but render preview only for previewable png,jpg,webp, rest should stay as Icon.

richard67 - comment - 7 Feb 2021

I am about render the preview.
If Admin choose to show png,jpg,webp,pdf,foobar then in Media browser should be visible ALL this files, but render preview only for previewable png,jpg,webp, rest should stay as Icon.

@Fedik Then I think we want the same thing, we have just chosen different words to describe it ?

Fedik - comment - 7 Feb 2021

we have just chosen different words to describe it

Can be ?

Here is pseudo code:

loop Image in Images:
  if can preview Image:
    show preview
  else:
    show icon

dgrammatiko - comment - 7 Feb 2021

Here is pseudo code:

Well, in that case, we need to pass from PHP a list with all the files that can have a preview. The allowed list should be used only to upload/manipulate/select a file. A simple JSON file with all the valid extensions for all file types could be passed with addScriptOptions so we don;t have to hard code things...

richard67 - comment - 7 Feb 2021

loop Image in Images:
  if can preview Image:
    show preview
  else:
    show icon

Maybe we can do a

>   if can preview Image:
>     show preview
>   else if image shows a cat:
>     show preview in any case because it's soo cute
>   else:
>     show icon

dgrammatiko - comment - 7 Feb 2021

show preview in any case because it's soo cute

This is debatable. Dog people might suggest their favourite pet ?

dgrammatiko - comment - 14 Feb 2021

I'm closing this as it's obviously not solving the problem here

dgrammatiko - change - 14 Feb 2021

Status	Pending	⇒	Closed
Closed_Date	0000-00-00 00:00:00	⇒	2021-02-14 10:05:47
Closed_By		⇒	dgrammatiko

dgrammatiko - close - 14 Feb 2021

richard67 - comment - 14 Feb 2021

So shall we re-open issue #32288 ? Or open a new one because #32288 is not exactly the same issue? I‘m not really sure.

dgrammatiko - comment - 14 Feb 2021

Maybe open an umbrella issue for all media manager issues (eg https://github.com/joomla/joomla-cms/projects)

richard67 - comment - 14 Feb 2021

There is one, but it's closed: https://github.com/joomla/joomla-cms/projects/9 ;-)

richard67 - comment - 14 Feb 2021

Anyway, creating projects is beyond my privileges. Will check what to do.

Magnytu2 - comment - 14 Feb 2021

Hello, sorry for this intervention. But the Joomshaper team had already found problems in the media manager (from memory). They had for that, realized a new media manager. Maybe we need to get closer to them to save time? It's a simple proposition.
https://youtu.be/Ks8tuxrmv24

richard67 - comment - 14 Feb 2021

Not on me to decide such things.

dgrammatiko - comment - 14 Feb 2021

@Magnytu2 the repo is still there: https://github.com/joomla-projects/j4adminui so someone (not me) could diff and merge things

Magnytu2 - comment - 14 Feb 2021

@dgrammatiko There was another version, much easier to install. I am a fan of Joomla! and yet it is bigger mistake than I regret. Since the time everything would already be corrected.

Add a Comment

Older
Newer

Joomla! Issue Tracker - CMS

[#32326] - [4.0] Fix Media manager for multilingual sites

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

Add a Comment

	private function getPathInformation(string $path): \stdClass
	{
	// Prepare the path
	$path = Path::clean($path, '/');

	// The boolean if it is a dir
	$isDir = is_dir($path);

	$createDate = $this->getDate(filectime($path));
	$modifiedDate = $this->getDate(filemtime($path));

	// Set the values
	$obj = new \stdClass;
	$obj->type = $isDir ? 'dir' : 'file';
	$obj->name = $this->getFileName($path);
	$obj->path = str_replace($this->rootPath, '', $path);
	$obj->extension = !$isDir ? File::getExt($obj->name) : '';
	$obj->size = !$isDir ? filesize($path) : '';
	$obj->mime_type = MediaHelper::getMimeType($path, MediaHelper::isImage($obj->name));
	$obj->width = 0;
	$obj->height = 0;

	// Dates
	$obj->create_date = $createDate->format('c', true);
	$obj->create_date_formatted = HTMLHelper::_('date', $createDate, Text::_('DATE_FORMAT_LC5'));
	$obj->modified_date = $modifiedDate->format('c', true);
	$obj->modified_date_formatted = HTMLHelper::_('date', $modifiedDate, Text::_('DATE_FORMAT_LC5'));

	if (MediaHelper::isImage($obj->name))
	{
	// Get the image properties
	try
	{
	$props = Image::getImageFileProperties($path);
	$obj->width = $props->width;
	$obj->height = $props->height;

	// Todo : Change this path to an actual thumbnail path
	$obj->thumb_path = $this->getUrl($obj->path);
	}
	catch (UnparsableImageException $e)
	{
	// Ignore the exception - it's an image that we don't know how to parse right now
	}
	}

	return $obj;
	}