@zero-24 the images rendered through the media field still have no width and height but easily fixable:
change

editor.value = `${Joomla.selectedFile.url}?width=${Joomla.selectedFile.width}&height=${Joomla.selectedFile.height}`;

Explainer: The value that is stored in the db for each form field media is a url. Urls can have params. We're passing the required data as params, and pick them up in the layout [php: parse_url($url)] and append them to the respected image attributes. FWIW this technic was proposed to solve also the editing of an image (by having a ?version=xxx param) but was never implemented in the new media manager due to the lack of db integration (I think that was planned for the next iteration).

Patched thanks @dgrammatiko

Close #30615?

Close #30615?

Done

@zero-24 it seems that I broke something in the media manager at some point. Please add

                                width: this.item.width ? this.item.width : undefined,
                                height: this.item.height? this.item.height : undefined,

after

Also here's a quick implementation of fulltext image layout:

defined('_JEXEC') or die;

use Joomla\Utilities\ArrayHelper;

$params  = $displayData->params;
$images  = json_decode($displayData->images);
$image   = parse_url($images->image_fulltext);
$src     = $image['path'];
$attr    = [];

parse_str($image['query'], $imageParams);

if (count($imageParams))
{
	if ($imageParams['width'] !== 'undefined')
	{
		$attr['width'] = $imageParams['width'] . 'px';
	}

	if ($imageParams['height'] !== 'undefined')
	{
		$attr['height'] = $imageParams['height'] . 'px';
	}
}
?>
<?php if (!empty($images->image_fulltext)) : ?>
	<?php $imgfloat = empty($images->float_fulltext) ? $params->get('float_fulltext') : $images->float_fulltext; ?>
	<figure class="float-<?php echo htmlspecialchars($imgfloat, ENT_COMPAT, 'UTF-8'); ?> item-image">
		<img loading="lazy" src="<?php echo htmlspecialchars($src, ENT_COMPAT, 'UTF-8'); ?>"
			 alt="<?php echo htmlspecialchars($images->image_fulltext_alt, ENT_COMPAT, 'UTF-8'); ?>"
			 itemprop="image"
				<?php echo ArrayHelper::toString($attr); ?>/>
		<?php if (isset($images->image_fulltext_caption) && $images->image_fulltext_caption !== '') : ?>
			<figcaption class="caption"><?php echo htmlspecialchars($images->image_fulltext_caption, ENT_COMPAT, 'UTF-8'); ?></figcaption>
		<?php endif; ?>
	</figure>
<?php endif;

and a screenshot of the expected result

Thanks @dgrammatiko i have just implemended the suggested changes for the full and intro image layout as well as the mention JS changes in the media manager. Thanks!

?‍♂️

What is wrong @SharkyKZ ?

Everything. Revert previous commits and don't screw around with image URLs please.

Everything. Revert previous commits and don't screw around with image URLs please.

You mean the layout changes right? what is wrong with them? We can also branch this out to a dedicated PR when we need to cosider more things on that.

Everything. Revert previous commits and don't screw around with image URLs please.

I'm curious as well can you elaborate please?

Not that anyone cares, but Media Manager is supposed to work with external image hosts.

Not that anyone cares, but Media Manager is supposed to work with external image hosts.

It does, there's an API for that

FYI:

Since @SharkyKZ raised some questions about the proposal to pass the additional data (width and height) as query params in the image url, I think I should try to debunk those and explain a bit more why this is possibly the only viable way.

It may or may not cause issues depending on image host.

Joomla is appending query parameters in CSS and JS files for almost one decade without anyone (at least as far as I know) complaining that their host cannot handle these properly. I don't see how a Joomla host will be broken on any image URL that has some extra query params.

But J4's media manager opens the door for external storages.

Sure, and the API already cover us here as it's mandatory to return the image width and height in the Adapter.

So the API is fine but what about the actual host?

Well, Dropbox, S3, Cloudinary, etc. (almost every popular service I know of) will handle the extra URL params. Most of them are already using URL params one way or another. But if that's still a concern there's a way out, keep reading

How about name classing?

There are 3 ways out of this:

• destructure and re construct of the URL removing the extra URL params (I've tried that but was very rough)
• use unique naming, eg joomla_img_width and joomla_img_height
• both destructure and re construct of the URL and unique naming ?

How about B/C break here?

There is no possible B/C break by adding a couple of params in the URL. That said I want to mention that the new media manager supports more storages other than the localhost. So, if some developers have specific code that treats these URLs as part of the localhost path (eg concatenate with JPATH_ROOT' . $url) that will not work, and this is not something introduced here, it's far deeper (the Adapter part, but these changes make it way more obvious as they also affect the localhost).

Can't we just do some simple PHP check on the file meta?

No, we can't do that for two main reasons:

• It will not work for any other storage other than localhost
• The performance difference is significant here (having to read each image per request to get the dimensions hardly passes and perf consideration)
  (Not to mention that is good to have one point of truth, eg the database)

@zero-24 one more change in the js, please replace

      const appendParam = (url, key, value) => {
        const newKey = encodeURIComponent(key);
        const newValue = encodeURIComponent(value);
        const r = new RegExp(`(&|\\?)${key}=[^\&]*`);
        let s = url;
        const param = `${newKey}=${newValue}`;

        s = s.replace(r, `$1${param}`);

        if (!RegExp.$1 && s.includes('?')) {
          return `${s}&${param}`;
        }

        if (!RegExp.$1 && !s.includes('?')) {
          return `${s}?${param}`;
        }

        return s;
      };

      if (Joomla.selectedFile.url) {
        if (!isElement(editor) && (typeof editor !== 'object')) {
          Joomla.editors.instances[editor].replaceSelection(`<img loading="lazy" src="${Joomla.selectedFile.url}" width="${Joomla.selectedFile.width}" height="${Joomla.selectedFile.height}" alt="" />`);
        } else if (!isElement(editor) && (typeof editor === 'object' && editor.id)) {
          window.parent.Joomla.editors.instances[editor.id].replaceSelection(`<img loading="lazy" src="${Joomla.selectedFile.url}" width="${Joomla.selectedFile.width}" height="${Joomla.selectedFile.height}" alt="" />`);
        } else {
          const val = appendParam(Joomla.selectedFile.url, 'joomla_image_width', Joomla.selectedFile.width);
          editor.value = appendParam(val, 'joomla_image_height', Joomla.selectedFile.height);
          fieldClass.updatePreview();
        }
      }

Basically it will respect any preexisting URL params, previous code was not treating correctly existing URL params.

PS 1. Please mind the change in the names of the URL params

PS 2 . About the restructuring and reconstructing the URL, these can be done as an HTMLHelper and the code is already widely available, eg: https://stackoverflow.com/questions/4354904/php-parse-url-reverse-parsed-url (destructuring is supported by PHP so all it might need is return an object with all the parts or false if it fails)

@zero-24 As we support updates from Beta to later versions (Beta or RC or final) in J4, should the plugin be uninstalled when updating? Maybe you can discuss that directly with George? I'm not sure (yet) how we should handle that.

@zero-24 As we support updates from Beta to later versions (Beta or RC or final) in J4, should the plugin be uninstalled when updating? Maybe you can discuss that directly with George? I'm not sure (yet) how we should handle that.

Yes that is the same thing i had in mind too. I dont want to implement beta to beta SQL or thing to the script PHP. But lucky us nothing should break in the end so a note that on upgrade there might should be some cleanup script executed would be ok? @wilsonge ?

@zero-24 one more change in the js, please replace

@dgrammatiko just to be sure in that JS code we would change to joomla_image_width and joomla_image_heigth so we should do that on all places as well as edit all the other code to make sure they check for the new values right?

@zero-24 Maybe we will be "lucky" and something else breaks update to the next beta, then we don't have to do anything anyway ? But you are right, if we do something it will be script.php, using model, othewise we won't get rid of stuff like assets.

should do that on all places as well as edit all the other code to make sure they check for the new values right

Yup, it's the naming on the layouts, intro and fulltext. Also if you think that snake_case is not the preferred case please change it to whatever is more appropriate (camelCase, pascal, kebab). Finally you might want to rename the names to something else (naming is hard)

Thanks yes i have just added that to my list hopefully i get this cleandup by next week, thanks for all that feedback! :)

@zero-24 can you rollback to only uninstall the plugin here?
I already prepared a PR that has all the needed JS, layout, etc #30784

Fine for me will do that hopefully tomorrow Thabks for your PR!

This PR has now been reset to only remove the plugin and add the is_array check for the HTML Helper. Thanks @dgrammatiko ?

I have tested this item ✅ successfully on 8de2081

@zero-24 Thank you for this one!

I have tested this item ✅ successfully on 8de2081

RTC

@dgrammatiko @Quy We (Tobias and me) had to add an additional test, see section "upgrade test" in the testing instructions. Could you test that, too? The other test and related code has not been changed, so no need to repeat your previous, valid test for the "clean install test" section. Thanks in advance.

I have tested this item ✅ successfully on e735059

Update of a clean J4 was fine, plugin was removed successfully

Update test on MySQLi and PostgreSQL is ok. Now am doing new installation tests for both database types.

I have tested this item ✅ successfully on e735059

• New installation MySQLi: ✅
• New installation PostgreSQL (PDO): ✅
• Update MySQLi: ✅
• Update PostgreSQL (PDO): ✅

  ---

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

Thanks for doing it the proper way.

Thanks