[4.0] Support External Adapters For Media Form Field

[joomdonation]

Pull Request for Issue #26750.

Summary of Changes

This is my quick attempt to solve the issue with external adapters discussed in #26750. The change includes:

• For local adapter, it will just work as how it was before
• For external adapters:

• Store full url of the image into value of the field so that we do not have to ask the adapter to get the image url each time the image is being displayed
• The adapter + the internal path of the image is stored as part of value, too. It is added to the end of the image URL (# + Adapter Name + Internal Path). I decided to use # character instead of & character because Dropbox does not allow adding query string to image url.

• Modify HTMLHelper to remove the adater name + Internal Path from the image before it is displayed

Testing Instructions

1. Download update package for this PR at https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/33724/downloads/44325/Joomla_4.0.0-beta8-dev+pr.33724-Development-Update_Package.zip . Go to System -> Update -> Joomla, upload and update this package.
2. General testing is test and make sure you can select image for your articles and these selected images will still be displayed properly when you view the article from frontend of your site.

Test local adapter:

• Create a article, select an image for that article, make sure you can still select the image. Save the article .
• Access to the article from frontend, make sure the image is being displayed
• Edit that article again. Click on the Select button next to the image and try to select new image. Please make sure that you don't see any error while clicking on that Select button. Also, click on that Select button should show you the folder which the image belong to (basically, the system remember the path of the image, and you do not have to navigate from root to find a new image.

Test External Adapter

It will be good to test media manager with an external adapter like dropbox and make sure it is working OK. You can"

• Download Dropbox Adapter plugin from this link https://github.com/joomla/joomla-cms/files/6526439/plug_filesystem_dpdropbox.zip . Go to System -> Extensions -> Install to install that plugin
• Go to System -> Manage -> Plugins, find that plugin, click on it to edit and enter necessary parameters. You can find the testing parameters from my Dropbox account below:

Client ID : 2p8aoyqpeub1a39
Client Secret: 2p8aoyqpeub1a39
Access Token: jeU8dH-JufAAAAAAAAAAAex3adAkjxBV5e7BqzrOi6T60ZRzwhVQUNJDOnmitGOa

Then try to create article or edit article, select an image from dropbox adapter for that article, save the article. Then view that article from frontend, make sure the image is still being displayed.

[joomdonation]

One thing I should mention here is that for image from external adapter, the system remember the path of the image. Next time when you edit the image, it will remember the folder of that image, so you won't have to navigate from root folder. Maybe we should try to do the same for local adapter.

[HLeithner]

Don't know where to start ;-)

if we are going to use # then we should also change the jooma_image_width to use this. Also please don't do the parsing manually. It should be possible to use uri to parse the image meta information string somthing like this:

$imageUrl = 'images/bla.jpg#joomlaImage://local-0?width=123&height=321&path=/images';

$url = new Uri($image);

$metadata = new Uri($url->getFragment());

local-0:// should be local-name://

joomla_image_width -> width
joomla_image_height -> height

of course it would be possible to explode $imageUrl at #

[dgrammatiko]

joomla_image_width -> width
joomla_image_height -> height

B/C break, just saying

[HLeithner]

joomla_image_width -> width
joomla_image_height -> height

B/C break, just saying

Depends how you are doing it ;-) if you already parse the $imageUrl with JUri you have this parameters and can use them too

[joomdonation]

Thinking more about it, I wonder if this solution will work. If the real link of the external image is expired for some reason, it won't be displayed anymore (so for Dropbox for example, I don't know if the link is there for every or it will be expired at certain time). If it is expired, then this solution won't work.

[richard67]

Anyway the scss linter complains again, this time about unused variables: https://ci.joomla.org/joomla/joomla-cms/43375/1/21 .

[joomdonation]

Thanks @richard67 Please ignore code style issue for now. I just want to see if the PR works or not first before working on further cleanup

[richard67]

@joomdonation Code style errors make the system test not being run, so they should be avoided even in working phase because system tests can show errors.

[richard67]

Code style is fixed now. Currently analysis4x fails in Drone, very likely due to false alarm. Will this PR receive further changes? If so, we should wait with fixing the analysis4x thing. But if the PR is ready let me know, then I will ping the experts for fixing it.

[joomdonation]

In the last commit, I implemented the new change suggested by @HLeithner . The value for the image will now has this sample format images/headers/walden-pond.jpg#joomlaImage://local-0/headers/walden-pond.jpg?width=700&height=180

I tested it myself and it works OK. I think it implemented the changes which we discussed so far OK now

Please note that this PR will only work if the external adapter can implement a method return permanent URL of the image. The Dropbox adapter which I am using for testing can only get temporary link (which will be expired in 4 hours), so it is not really working. I looked at Dropbox API and seems we can get permanent link, so in theory, this PR should work.

[HLeithner]

@joomdonation thank you for creating this pr and doing the changes

[HLeithner]

${media.path.replace(':', '')}

That doesn't look right, the path (after the adapter /) should be url encoded. please don't remove parts of the string

[joomdonation]

Actually, it's needed so that the value has format which we discussed. media.path contains adapter name, :, and then path to the file, so I need to remove : from there to make it works.

Sample data of media.path: local-0:/sampledata/cassiopeia/nasa1-400.jpg

[HLeithner]

ok that makes sense at least for the first parameter should be removed, a second : shouldnt be a problem (after the /)

[HLeithner]

@dgrammatiko shouldn't we explode this into proper html attributes for joomla-field-media? So we don't need to parse the url in joomla-field-media?

[dgrammatiko]

Yup, but it's not about the parsing part. We use it in the iframe URL:

so the value needs to be available to JS from an attribute, eg <joomla-field-media class="field-media-wrapper" adapter="local-0". This will also ensure that the field opens the media manager in the right path, (what the other issue
was all about)

[joomdonation]

@dgrammatiko So any change needed here? From my test, it opens the right path when I click on Select button, so it is working well for me

[dgrammatiko]

@HLeithner if it works fine for @joomdonation I think we're fine.

[dgrammatiko]

Meh there's something off here, so if there is a value the adapter will be extracted from the URL if ($value && strpos($value, '#') !== false) else it falls back to local-0. But the whole issue was about passing the adapter to the field (or I'm totally confused?)

[joomdonation]

The original issue is that the adapter is always hardcoded to local-0. That mean if you select a file from Dropbox for example, next time, if you click on the Select button to edit the file again, it will use local adapter, so the path will be wrong....

With this change, we store and use valid adapter, also, remember the path of the file so that when you click on Select button, it will redirect you to original folder of that file.

[dgrammatiko]

The original issue is that the adapter is always hardcoded to local-0

Not exactly, take for example tinyMCE and the drag and drop path:

joomla-cms/plugins/editors/tinymce/forms/setoptions.xml

Line 66 in 5e08d61

type="uploaddirs"

ATM it takes the value for the path (and the adapter which at the moment is only local) from

joomla-cms/plugins/editors/tinymce/src/Field/UploaddirsField.php

Line 48 in 5e08d61

$this->directory = ComponentHelper::getParams('com_media')->get('image_path');

So the whole idea (at least from my point of view) is to have the ability to inject/set a particular adapter for the field. Also, banners could very likely outsource the images to a CDN (dropbox, s3, whatever) and in that case, an adapter field in the media field could set the correct starting point when media manager will be opened (instead of the current case that users need to start from local-0)

[joomdonation]

@dgrammatiko If I understand your idea correctly, it could be addressed by @Fedik suggestion #33724 (comment)

[joomdonation]

I'm unsure why drone failed for this PR https://ci.joomla.org/joomla/joomla-cms/43404/1/27

[rdeutz]

@zero-24 @SniperSister can you have a look at rips, thanks

[Fedik]

Thanks for the PR.

One thing I should mention here is that for image from external adapter, the system remember the path of the image. Next time when you edit the image, it will remember the folder of that image, so you won't have to navigate from root folder.

I have wrote about it also #26750 (comment)
In theory, if we store the source URI correctly then we can write some script that check/validate paths.

The value for the image will now has this sample format images/headers/walden-pond.jpg#joomlaImage://local-0/headers/walden-pond.jpg?width=700&height=180

To me @HLeithner path #33724 (comment) a bit to much ;)
I would try to keep the original URI as it is local-0:/headers/walden-pond.jpg

Maybe more simple:
images/headers/walden-pond.jpg#local-0:/headers/walden-pond.jpg?width=700&height=180
or
images/headers/walden-pond.jpg?joomla_file_uri=local-0:/headers/walden-pond.jpg&width=700&height=180

Unfortunately local-0: is not really a schema, but some fancy prefix :)

Check how it parsed in the media controller:

joomla-cms/administrator/components/com_media/src/Controller/ApiController.php

Lines 375 to 404 in 3a5984d

	private function getAdapter()
	{
	$parts = explode(':', $this->input->getString('path', ''), 2);
	if (count($parts) < 1)
	{
	return null;
	}
	return $parts[0];
	}
	/**
	* Get the Path.
	*
	* @return string
	*
	* @since 4.0.0
	*/
	private function getPath()
	{
	$parts = explode(':', $this->input->getString('path', ''), 2);
	if (count($parts) < 2)
	{
	return null;
	}
	return $parts[1];
	}

Or what do you think @HLeithner ?

[joomdonation]

@Fedik The format suggested from @HLeithner is joomlaImage://local-0/headers/walden-pond.jpg?width=700&height=180 (of course, it is just a sample)

The purpose is that it allows us to use our Uri class to parser the data and get the correct information we want. So local-0 is host, headers/walden-pond.jpg is path, width=700&height=180 is query string. It will allow us to parse and get the data easier.

[Fedik]

So local-0 is host,

I thought the same before, but the problem that it not really a host ;)
Look how media manager parse it in php backend.

Sorry I mixed host and schema

[joomdonation]

Look how media manager parse it in php backend

I haven't looked at it yet. But I guess it is some kind of string manipulation?

[richard67]

Could be a problem of that plugin. If on Linux only, it's maybe related to file names with wrong case so some class is not found, but that's just a guess. But @joomdonation can't reproduce it on Windows, so maybe not a bad guess?

[richard67]

What confuses me is that the plugin ships with an own guzzlehttp but the call stack ends in the one shipped by the CMS.

[chmst]

I have tested the external adapter on win10, in xampp (error-reporting: maximum).
The plugin is installed without error.
Added the data as given in testing instructions.
Selecting an image from the dropbox works. But the image then has dimension 0/0 and therefor is not visible.

[joomdonation]

So @chmst tested on Window and it is working well, too. Guess there is a problem with Dropbox adapter on Linux which I don't have time to debug yet. It is of course, not related to this PR

There is a small issue remaining and not related to this PR, too. When insert image from Dropbox using CMS Content -> Image, for external adapter, there is no width and height returned, and in that case, it is invisible in the editor. It is related to how we handle lazy load in the code, so I think I can ask @dgrammatiko to look at it on a separate PR.

[joomdonation]

To be more clear, I think it is related to this line of code https://github.com/joomla/joomla-cms/blob/4.0-dev/build/media_source/system/js/fields/joomla-image-select.w-c.es6.js#L148-L150 . Maybe we should check and make sure there is width and height returned before adding lazy load. But that can be addressed in a different PR

So unless someone want to do a final code review, I think this PR can be set to RTC (It has been tested by @dgrammatiko and @richard67 for Local adapter, tested by myself and @chmst for dropbox adapter).

[dgrammatiko]

@joomdonation try this:

if (attribs.getAttribute('is-lazy') === 'true' && Joomla.selectedMediaFile.width > 0 && Joomla.selectedMediaFile.height > 0) {
  isLazy = ` loading="lazy" width="${Joomla.selectedMediaFile.width}" height="${Joomla.selectedMediaFile.height}"`; 
}

FWIW the adapter SHOULD return an object with predefined properties (width, height, url...) so if the dropbox is not returning those is kinda broken

[joomdonation]

@dgrammatiko Will do that in a separate PR. For some reasons, RIPS does not like this PR, so I will stop making un-necessary change to this PR :).

[joomdonation]

For dropbox, maybe it could not detect with and height of the image somehow. Or developer did not put much effort into that plugin. They created the plugin to test the adapter concept only :)

[dgrammatiko]

I'm leaving this here:

const getImageSize = (url) => {
  return Promise((resolve, reject) => {
    const img = new Image();
    img.src = url;
    img.onload = function() {
      resolve({ width: img.width, height: img.height });
    };
    img.onerror = function() {
      reject('Image failed to load');
    };
  });
}

// Can be used like:
const { width, height } = await getImageSize('https://...');

[richard67]

Regarding the dropbox plugin I think the PHP error is related to or is the one described in this issue: https://github.com/Digital-Peak-Incubator/plg_filesystem_dpdropbox/issues/2 .

If I rename the "guzzlehttp" folder in the "vendor" folder of that plugin to something else, the PHP error is gone, and I see "Dropbox" and "Local" filesystems in the media manager.

For articles I can select and then change selections using the "Dropbox" file system.

[richard67]

I have tested this item ✅ successfully on cfa5bd2

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33724.}

[richard67]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33724.}

[dgrammatiko]

@chmst could you test this pr but with the script from #34177 ?

@joomdonation is there anything more that needs to be changed in the selector script?

[joomdonation]

@dgrammatiko The only thing I want to change is if the width and height is zero, it should not be added to lazy loading. So basically, I think all I want is the code you propose at #33724 (comment)

Just for your information, I could not say that I fully understand media api. However, I remember I see from adapter interface that adapter should return width and height of the image, so I'm unsure if detect width and height using js like you did in #34177 is needed. I think adapter will have to return that information using server side code. We just got the problem here is because the dropbox adapter is not a complete plugin, as I said, it was created quickly for testing purpose. So maybe we don't have to handle that special case for now.

[dgrammatiko]

I remember I see from adapter interface that adapter should return width and height of the image

We all know by now that any Joomla API is totally cannibalized from some devs thus the code to sniff the size in case the adapter is not respecting the API. Anyways, the code is ~10locs so I guess it doesn't change anything (it will only run if width or height is 0 which should be very unlikely if devs are coding correctly their adapters)

[joomdonation]

it will only run if width or height is 0 which should be very unlikely if devs are coding correctly their adapters

If so, I guess it's fine/