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.

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 #

joomla_image_width -> width
joomla_image_height -> height

B/C break, just saying

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

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.

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

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

@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.

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.

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.

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

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

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:

Or what do you think @HLeithner ?

@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.

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

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?

But I guess it is some kind of string manipulation?

yeah, just explode(':', $path, 2), look the link in my previous comment

okay, maybe it can be as @HLeithner suggested

Actually, I'm find with any format :). We just need to make decision about the format we want to use :)

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

We are looking into it and report back here please do not merge this as long as rips fails.

the schema follows more your suggestion @Fedik it would allow us (if needed at some point) to create a php stream wrapper.

And it's a standard format which we have a parse in every language ;-) That's the reason I suggested the change to this.

images/headers/walden-pond.jpg#local-0:/headers/walden-pond.jpg?width=700&height=180

this can't be parse by JURI because we don't have a ://

images/headers/walden-pond.jpg?joomla_file_uri=local-0:/headers/walden-pond.jpg&width=700&height=180

This breaks the image if some may not filter the url right for what ever reason. It's easier to explode on # because then on ? because you may have get parameters for the image...

Think about this I wonder if removing the # fragment we may break svg images? Is this possible?

Think about this I wonder if removing the # fragment we may break svg images? Is this possible?

Valid

Edit: Actually this is invalid as the media manager is not having any means to return a specific node of an SVG element, we can only return only the SVG file.

so using only the last # would be better and validate the schema

all good then

it would allow us (if needed at some point) to create a php stream wrapper.

stream wrapper would be much nice,
some time in future :)

all good then

it would allow us (if needed at some point) to create a php stream wrapper.

stream wrapper would be much nice,
some time in future :)

maybe it would make sense to check the mediamanager code if change the complete schema todo format? don't know how much it's used inside mediamanager...

So I take a look at our MediaField code https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Form/Field/MediaField.php#L253-L267 and see that currently, we always check local directory. That mean for a Media field type, it is not possible to use a folder/relative path for external adapter. Any idea how we should handle that?

Maybe allow syntax such as directory="dpdropbox-0:/photos" ?

So I take a look at our MediaField code https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Form/Field/MediaField.php#L253-L267 and see that currently, we always check local directory. That mean for a Media field type, it is not possible to use a folder/relative path for external adapter. Any idea how we should handle that?

Maybe allow syntax such as directory="dpdropbox-0:/photos" ?

Code looks wrong :(

Not sure what joomla-media-field needs, if it require the adapter and the path separated then it would make sense to provide it in 2 attributes. In case it only pass it to the mediamanger we can keep only a single string

Not sure what joomla-media-field needs, if it require the adapter and the path separated then it would make sense to provide it in 2 attributes. In case it only pass it to the mediamanger we can keep only a single string

I'm unsure, too. Maybe we should support a new attribute adapter and keep directory attribute, there. adapter should default to local for backward compatible purpose. @dgrammatiko Do you have any idea about this?

maybe it would make sense to check the mediamanager code if change the complete schema todo format?

Most of the code in ApiController, and ApiModel, as far as I can see.
Simplified version is:

list($adapter, $sourcePath) = explode(':', $path, 2);
$apiModel->getAdapter($adapter)->doStuff($sourcePath); // ->copy(), ->move(), etc

Someone tried to implement kind of stream wrapper in own way :)
It looks limited to use with com_media.

Maybe allow syntax such as directory="dpdropbox-0:/photos" ?
Code looks wrong :(

It is what media manager expects, however media field do not know it.
In theory can add one more IF with if(strpos($this->value, ':'))
before if ($this->value && is_file(JPATH_ROOT . '/' . $this->value))

My 2c here, I would add an adapter field in the XML, type string and in the logic I would change the else part from $mediaPath = 'local-0:/' . $folder; to something like:

if ($adapter) {
  // The adapter was set either in the XML or programmatically
  $mediaPath = $adapter . ':/' . $folder;
} else {
 // No adapter is set so we will get the default from com_media
 Boot('com_media')->getDefaultAdapter(); // This is pseudocode...
}

My 2c here, I would add an adapter field in the XML, type string and in the logic I would change the else part from $mediaPath = 'local-0:/' . $folder; to something like:

if ($adapter) {
  // The adapter was set either in the XML or programmatically
  $mediaPath = $adapter . ':/' . $folder;
} else {
 // No adapter is set so we will get the default from com_media
 Boot('com_media')->getDefaultAdapter(); // This is pseudocode...
}

in which XML do you mean?

in which XML do you mean?

The one that defines the field in a form, eg as in the original issue:

no idea why there is a directory attribute in the form field, maybe as default start value?
The adapter (and the path to the image) can never be stored in the XML because it can be changed if while selecting the image. That's the point why I want to have it as parameter attached to the image url

It's right that directory is the starting point of which media manager loaded when you click on the file. The adapter could be a new attribute to the field, but it should default to local adapter for backward compatible purpose.

no idea why there is a directory attribute in the form field, maybe as default start value?

It serves a purpose: to open the media manager in the correct folder.

My idea is to have in the options on the com_banners a field that sets the adapter and another for the initial path. Then programmatically (this is the important part here) pass these values to the field. Hard coding these values in a dynamic and flexible system will never work (eg delete all the adapters and the install let's say cloudinary as the only adapter and the system will just fail badly)

It's right that directory is the starting point of which media manager loaded when you click on the file. The adapter could be a new attribute to the field, but it should default to local adapter for backward compatible purpose.

Ok the directory is only use full if you didn't select and image and if you use the local adapter (a default that supports directory structure), if you don't have the local adapter the directory is likely useless. Not really a good solution in my opinion but ok.

no idea why there is a directory attribute in the form field, maybe as default start value?

It serves a purpose: to open the media manager in the correct folder.

My idea is to have in the options on the com_banners a field that sets the adapter and another for the initial path. Then programmatically (this is the important part here) pass these values to the field. Hard coding these values in a dynamic and flexible system will never work (eg delete all the adapters and the install let's say cloudinary as the only adapter and the system will just fail badly)

Actually that's not true, if the you open the mediamanager with 'joomlaImage://dropboxdp/banner/image4711.jpg' and the banner directory doesn't exists the mediamanager starts in the root of dropboxdp, if the adapter dropboxdp doesn't exists it will use the 'default' adapter (we really need a solution for the default adapter). No reason to fail just fall back to the default.

No reason to fail just fall back to the default.

Yes, fallbacks are fine but it's not ONLY for the com_media's default adapter, it's also about setting a preferred adapter per component/form...

No reason to fail just fall back to the default.

Yes, fallbacks are fine but it's not ONLY for the com_media's default adapter, it's also about setting a preferred adapter per component/form...

This couldn't be static too but isn't the main problem at the moment. If we have a getDefaultAdapter() function it can be extended by context like com_contact.contact for example. But defining the adapter in the xml doesn't makes any sense, also the directory could/should be context based.

But defining the adapter in the xml doesn't makes any sense

Oh, I think you misunderstood me, if there is a way to expose the property adapter of the field without having an attribute then do that. For me would be way easier to handle things programmatically by setting the attribute but that's just me...

Boot('com_media')->getDefaultAdapter(); // This is pseudocode...

If we have a getDefaultAdapter() function it can be extended by context like com_contact.contact for example.

I think I have wrote it before, it maybe better if it comes from com_media parameters.

The "context" does not need, it should be kind of global for com_media. Example: User choose to store all files in dropbox by default, etc.

But defining the adapter in the xml doesn't makes any sense, also the directory could/should be context based.

That right, does not need atribute.
The directory will depend from "default adapter" (from com_media parameters)

But defining the adapter in the xml doesn't makes any sense

Oh, I think you misunderstood me, if there is a way to expose the property adapter of the field without having an attribute then do that. For me would be way easier to handle things programmatically by setting the attribute but that's just me...

^^ hopefully we now talk about the same ;-)

Boot('com_media')->getDefaultAdapter(); // This is pseudocode...

If we have a getDefaultAdapter() function it can be extended by context like com_contact.contact for example.

I think I have wrote it before, it maybe better if it comes from com_media parameters.

The "context" does not need, it should be kind of global for com_media. Example: User choose to store all files in dropbox by default, etc.

The feature would be that the user can select which component has which starting directory, that's nothing for 4.0. But could be usefull if all your com_contact images are on google and all com_banner images are on dropbox. and Com_content images are provided by nextcloud.

But yes the first step would be to make it configurable in com_media configuration.

User choose to store all files in dropbox by default, etc.

So EVERY components/form should use JUST ONE adapter. That's doesn't sound quite flexible to me. Just give me a way to pass the adapter either through an XML attribute or another way BUT PLEASE don't choose for me...

I think I agree with @dgrammatiko . For my own component for example, I want to get images from a specific folder which is created during installation. For that, I would want to use local adapter, and set directory to the folder I want. I don't think I want to depend on the adapter from media component configuration.

It's not hurt to introduce support for adapter attribute in the xml field definition. The default value for adapter, if not set, could be get from default adapter config option of media component.

Or if not, maybe improve the current code so that someone can set directory to use the adapter they want. So directory="local-0:/images/banners".......

User choose to store all files in dropbox by default, etc.

So EVERY components/form should use JUST ONE adapter. That's doesn't sound quite flexible to me. Just give me a way to pass the adapter either through an XML attribute or another way BUT PLEASE don't choose for me...

I don't choose for you but you shouldn't choose for the user ;-) in the end the use should have the possibility to select the default adapter/path for your component. That can't be in the xml because you may don't have a local-0 and the user want's another adapter or default directory then you. But that's something not for now. For this the joomlaImage://adapter/path?width= format handle this. So your component can set the default in one way maybe by xml attribute but that can't be changed by the user.

I think I agree with @dgrammatiko . For my own component for example, I want to get images from a specific folder which is created during installation. For that, I would want to use local adapter, and set directory to the folder I want. I don't think I want to depend on the adapter from media component configuration.

It's not hurt to introduce support for adapter attribute in the xml field definition. The default value for adapter, if not set, could be get from default adapter config option of media component.

I think a component can't provide a useful default adapter except it provides it's own adapter, in all other cases it would be local-0 which should be changed to something like local-images or local-default

I think a component can't provide a useful default adapter except it provides it's own adapter, in all other cases it would be local-0 which should be changed to something like local-images or local-default

We support directory attribute for the field, so without adapter support, the directory won't be reliable. For example, in my own component, I set directory="com_mycom". The purpose is when users click on the button to select an image for the item, images/com_mycom will be starting folder. Image that someone selects Dropbox as default adapter from media component, this won't work because the folder does not exist.

I think a component can't provide a useful default adapter except it provides it's own adapter, in all other cases it would be local-0 which should be changed to something like local-images or local-default

We support directory attribute for the field, so without adapter support, the directory won't be reliable. For example, in my own component, I set directory="com_mycom". The purpose is when users click on the button to select an image for the item, images/com_mycom will be starting folder. Image that someone selects Dropbox as default adapter from media component, this won't work because the folder does not exist.

it would only work if you have an adapter local-0 but you component doesn't know (at development time) if there is an adapter local-0. So I mean you can add it but it's useless if in joomla 4.1 we change the default adapter from local-0 to local-default.

Hmm. Long discussion and maybe could not come to end :D. So with the current state, could you let me know what change is needed so that I can find sometime to finish this PR?

Hmm. Long discussion and maybe could not come to end :D. So with the current state, could you let me know what change is needed so that I can find sometime to finish this PR?

Sadly I lost track of the PR it self... I check it later again.

In case someone wants to try, you can install this dropbox adapter and use my test dropbox account for seeing how it works (note that the current dropbox adapter can only get temp url for the image, so the url will be invalid after 4 hours)

plg_filesystem_dpdropbox-master.zip

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

That can't be in the xml because you may don't have a local-0 and the user want's another adapter or default directory then you. But that's something not for now

I think the problem here is that misinterpret what I proposed. I said DEFINE an XML attribute for the field but I didn't say prepopulate it or hardcode a value. Is there another way to pass properties to the field apart from the defined ones (as XML attributes)?

@dgrammatiko With last commit, you can set set the adapter you want to use for your media field via directory attribute of the field. For example:

directory="dpdropbox-0:/photos"

Or

directory="local-0:/sampledata/categories"

I think it should address your concern and make the field flexible enough for usage.

There are still some limitation with the current code which I do not see an easy way to address:

• For image value stored in old format (for example, uploaded before this change or migrated from Joomla 3), we have to hardcode adapter to local-0
• For field (XML) which as directory attribute set to certain value and that value doesn't contain adapter information, we have to hardcode adapter to local-0, too

I think that's necessary for backward compatible purpose and maybe it's OK to accept this limitation. Other than that, with the current state, the PR is fine, I think.

@HLeithner Could you please have a discussion with @bembelimen to see if the approach which we are using on this PR is OK?

Drone/RIPS is still failed for this PR for unknow reasons :(.

@zero-24 @SniperSister Could you please help looking to see why RIPS is fail here? Would love to get this release blocker PR finished.

@joomdonation the Rips problem is sovled

Thanks @rdeutz. Need some more clean up to make it ready to merge. Will wait for @dgrammatiko PR #34069 ready and I will finish this one. If it goes well, we can have it finished during this weekend.

images/headers/walden-pond.jpg?joomla_file_uri=local-0:/headers/walden-pond.jpg&width=700&height=180

I'm a little bit worried about the plain width and height URL params. Are we sure that these will not clash with some external adapter's returned path?

@dgrammatiko Exactly what's the problem you are worry about? For external adapter, width and height might not be available and in this case, width and height will be removed by cleanImageURL method. Will that be OK ?

I don't know where we use that data for, so I'm unsure if that would work. But it should be the same with how it works before for local adapter.

I don't know where we use that data for,

This is for the loading=lazy tag on the images. The possible problem here is that we append URL params in the local-0:/headers/walden-pond.jpg which if it already has these params it will result to double entries. I have no idea if there are services out there that use directly height and width as URL params but if there are this would be problematic

Dont kn ow if it helps but cloudinary uses dynamic urls like this
https://res.cloudinary.com/demo/image/upload/w_170,h_153,c_scale/turtles.jpg

w_170 = width 170px
h_153 = height 153px

Dont kn ow if it helps but cloudinary uses dynamic urls like this
https://res.cloudinary.com/demo/image/upload/w_170,h_153,c_scale/turtles.jpg

So my concern is that if there's a service out there with a returning URL like https://res.cloudinary.com/demo/image/upload/turtles.jpg?with=100&height=100 there will be a clash. Cloudinary is not using URL params but we can't rule out that these params are unique. That was the reason I had prefixed them with joomla_image_ (probably overkill too many characters...) but maybe I'm just overcautious here

Also the dropbox plugin seems broken?

@dgrammatiko @brianteeman I don't really understand how we are using with and height to handle lazy load yet, so I could not answer the question about how width and height causes any issue if it is part of the returned image URL

@dgrammatiko In the last commit, I decided to not hardcode the adapter to local-images anymore. The system will try to use the top level folder from the part and validate and make sure it is configured in the adapter before using it in the adapter name. That's too avoid un-friendly error message displayed if someone, folder some reasons, choose to not use images folder in local adapter for their new Joomla installation. This makes the field a bit more flexible.

Attached is the dropbox plugin which I'm using on my local installation for testing in case you want to play with it

plug_filesystem_dpdropbox.zip

images/headers/walden-pond.jpg?joomla_file_uri=local-0:/headers/walden-pond.jpg&width=700&height=180

I'm a little bit worried about the plain width and height URL params. Are we sure that these will not clash with some external adapter's returned path?

we don't user this uri pattern we use the # pattern so even this is used as image url the with parameter is only a fragment

So I think it works OK now (although I don't like much, still many string manipulation). @dgrammatiko @Fedik @HLeithner Do you want to do a code review before we call for end users testing ?

I have tested this item ✅ successfully on cfa5bd2

I have tested only the local adapter

@SniperSister @zero-24 The RIPS problem is back here for this PR, it seems.

@laoneo can you test this pr with your plugin if it still works as expected?

@HLeithner I updated testing instructions with the modified dropbox plugin. The old plugin from @laoneo is not compatible anymore (we changed from numeric index to adapter name index as discussed - a PR from @dgrammatiko was merged)

I've tested the local adapter with success.

But the dropbox thing doesn't work for me.

As soon as the plugin is installed and enabled with the provided access information and I go to the media manager or some media field, I get a PHP error Call to undefined method GuzzleHttp\Utils::chooseHandler().

Call stack:

#  Function                                                                         Location
1  ()                                                                               JROOT/libraries/vendor/guzzlehttp/guzzle/src/functions.php:61
2  GuzzleHttp\choose_handler()                                                      JROOT/plugins/filesystem/dpdropbox/vendor/guzzlehttp/guzzle/src/HandlerStack.php:42
3  GuzzleHttp\HandlerStack::create()                                                JROOT/plugins/filesystem/dpdropbox/vendor/guzzlehttp/guzzle/src/Client.php:65
4  GuzzleHttp\Client->__construct()                                                 JROOT/plugins/filesystem/dpdropbox/vendor/srmklive/flysystem-dropbox-v2/src/Client/DropboxClient.php:105
5  Srmklive\Dropbox\Client\DropboxClient->setClient()                               JROOT/plugins/filesystem/dpdropbox/vendor/srmklive/flysystem-dropbox-v2/src/Client/DropboxClient.php:85
6  Srmklive\Dropbox\Client\DropboxClient->__construct()                             JROOT/plugins/filesystem/dpdropbox/src/Adapter/DPDropboxAdapter.php:96
7  DigitalPeak\Plugin\Filesystem\Dpdropbox\Adapter\DPDropboxAdapter->getClient()    JROOT/plugins/filesystem/dpdropbox/src/Adapter/DPDropboxAdapter.php:80
8  DigitalPeak\Plugin\Filesystem\Dpdropbox\Adapter\DPDropboxAdapter->__construct()  JROOT/plugins/filesystem/dpdropbox/dpdropbox.php:145
9  PlgFileSystemDpdropbox->getAdapters()                                            JROOT/administrator/components/com_media/src/Model/MediaModel.php:52
10 Joomla\Component\Media\Administrator\Model\MediaModel->getProviders()            JROOT/libraries/src/MVC/View/AbstractView.php:146
11 Joomla\CMS\MVC\View\AbstractView->get()                                          JROOT/administrator/components/com_media/src/View/Media/HtmlView.php:64
12 Joomla\Component\Media\Administrator\View\Media\HtmlView->display()              JROOT/libraries/src/MVC/Controller/BaseController.php:692
13 Joomla\CMS\MVC\Controller\BaseController->display()                              JROOT/libraries/src/MVC/Controller/BaseController.php:730
14 Joomla\CMS\MVC\Controller\BaseController->execute()                              JROOT/libraries/src/Dispatcher/ComponentDispatcher.php:146
15 Joomla\CMS\Dispatcher\ComponentDispatcher->dispatch()                            JROOT/libraries/src/Component/ComponentHelper.php:389
16 Joomla\CMS\Component\ComponentHelper::renderComponent()                          JROOT/libraries/src/Application/AdministratorApplication.php:143
17 Joomla\CMS\Application\AdministratorApplication->dispatch()                      JROOT/libraries/src/Application/AdministratorApplication.php:186
18 Joomla\CMS\Application\AdministratorApplication->doExecute()                     JROOT/libraries/src/Application/CMSApplication.php:278
19 Joomla\CMS\Application\CMSApplication->execute()                                 JROOT/administrator/includes/app.php:63
20 require_once()                                                                   JROOT/administrator/index.php:32 

Am testing on Linux.

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?

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

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.

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.

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).

@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

@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 :).

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 :)

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://...');

Regarding the dropbox plugin I think the PHP error is related to or is the one described in this issue: Digital-Peak-Incubator/plg_filesystem_dpdropbox#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.

I have tested this item ✅ successfully on cfa5bd2

RTC

@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?

@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.

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)

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/