No Code Attached Yet
avatar anibalsanchez
anibalsanchez
21 Oct 2021

Steps to reproduce the issue

The media field used to select images is now encoding image metadata into the field. For instance, when you select an image, it produces this value:

images/banners/osmbanner1.png#joomlaImage://local-images/banners/osmbanner1.png?width=468&height=60

Since the image field now encodes image metadata, it can't be directly used as before:

The image field is NOT a relative URL, and it is NOT a valid relative filename anymore.

Expected result

The image field should store the relative path to the image as before.

Actual result

The image field now stores encoded information.

System information (as much as possible)

Joomla 4.0.3

Additional comments

Since the decision to encode images paths into the field is already made by design, I don't expect a change and this issue more to let people be aware of the incompatibilities to the extensions integrating the Media field type.

According to the current Joomla code, it looks like the official way to decode the URL is in this way, and each developer has to decode the field on the spot:

// File: libraries/src/Form/Field/MediaField.php, line 267

// Value in new format such as images/headers/blue-flower.jpg#joomlaImage://local-images/headers/blue-flower.jpg?width=700&height=180
if ($this->value && strpos($this->value, '#') !== false)
{
	$uri     = new Uri(explode('#', $this->value)[1]);
	$adapter = $uri->getHost();
	$path    = $uri->getPath();

	// Remove filename from stored path to get the path to the folder which file is stored
	$pos = strrpos($path, '/');

	if ($pos !== false)
	{
		$path = substr($path, 0, $pos);
	}

	if ($path === '')
	{
		$path = '/';
	}

	$this->folder = $adapter . ':' . $path;
}
elseif ($this->value && is_file(JPATH_ROOT . '/' . $this->value))
avatar anibalsanchez anibalsanchez - open - 21 Oct 2021
avatar anibalsanchez anibalsanchez - change - 21 Oct 2021
Labels Removed: ?
avatar joomla-cms-bot joomla-cms-bot - change - 21 Oct 2021
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 21 Oct 2021
avatar brianteeman
brianteeman - comment - 21 Oct 2021

I don't expect a change and this issue more to let people be aware of the incompatibilities to the extensions integrating the Media field type.

The docs site is a better place for this then and not an issue which will be closed

avatar dgrammatiko
dgrammatiko - comment - 21 Oct 2021

The image field is NOT a relative URL, and it is NOT a valid relative filename anymore.

The value was never supposed to be used as a valid relative filename. The value WAS and still IS a URL and URLs can have hashes. FWIW the changes were done in order to keep B/C for the field type media. The alternative was to make the field type value a JSON but that would have broken every existing extension out there.
Also there's an HTMLHelper to get the URL (and other attributes):

public static function cleanImageURL($url)
{
$obj = new \stdClass;
$obj->attributes = [
'width' => 0,
'height' => 0,
];
if (!strpos($url, '?'))
{
$obj->url = $url;
return $obj;
}
$mediaUri = new Uri($url);
// Old image URL format
if ($mediaUri->hasVar('joomla_image_height'))
{
$height = (int) $mediaUri->getVar('joomla_image_height');
$width = (int) $mediaUri->getVar('joomla_image_width');
$mediaUri->delVar('joomla_image_height');
$mediaUri->delVar('joomla_image_width');
}
else
{
// New Image URL format
$fragmentUri = new Uri($mediaUri->getFragment());
$width = (int) $fragmentUri->getVar('width', 0);
$height = (int) $fragmentUri->getVar('height', 0);
}
if ($width > 0)
{
$obj->attributes['width'] = $width;
}
else
{
unset($obj->attributes['width']);
}
if ($height > 0)
{
$obj->attributes['height'] = $height;
}
else
{
unset($obj->attributes['height']);
}
$mediaUri->setFragment('');
$obj->url = $mediaUri->toString();
return $obj;
}

An example of a proper J4 image implementation is https://github.com/joomla/joomla-cms/blob/4.0-dev/layouts/joomla/content/intro_image.php

avatar anibalsanchez
anibalsanchez - comment - 21 Oct 2021

I think that encoding image metadata in the URL is an oversimplification of what is an Image, and it would be better to store the information in proper storage. I'm sure that in a short time, we'll find that more info must be added and the URL won't be enough... and you can compress it, and ... etc.

Thanks for the tip to clean the field with Joomla\CMS\HTML\HTMLHelper::cleanImageURL.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35871.
avatar anibalsanchez
anibalsanchez - comment - 21 Oct 2021

@brianteeman Yes, the docs site is a better place to document all of these changes. My first try was to check the page: https://docs.joomla.org/Potential_backward_compatibility_issues_in_Joomla_4


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35871.
avatar anibalsanchez
anibalsanchez - comment - 21 Oct 2021

@dgrammatiko About the compatibility of URLs with hashtags, they are rejected by several social networks. We noticed that something wasn't right when Facebook and Instagram refused to load the shared images.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35871.
avatar dgrammatiko
dgrammatiko - comment - 21 Oct 2021

@dgrammatiko About the compatibility of URLs with hashtags, they are rejected by several social networks.

There was quite some discussion over this. I started the whole thing as URL params and then @joomdonation and some maintainers figure out that the hash was a better fit.

The one part that requested this change was Image lazy loading:
#30784 (comment)
and the other one was a release blocker for the external adapters:
#33724

avatar anibalsanchez
anibalsanchez - comment - 21 Oct 2021

Well, now, the metadata is encoded in the URL after the hashtags, and since the media field type is used everywhere in config.xml files there are going to be plenty of unaware extensions exposing the metadata.

avatar anibalsanchez
anibalsanchez - comment - 22 Oct 2021
avatar n3t
n3t - comment - 23 Oct 2021

The value was never supposed to be used as a valid relative filename.

If it is so, it was AFAIK nowhere never said. Neither in documentation, nor in source code of media field, not even in some other description of content images fields. Could bring unexpected behaviors and errors for example for plugins for thumbnail creation etc., which works not only with URL but with file itself.

avatar pinochico
pinochico - comment - 24 Oct 2021

I am very confused by careful treading in one place - perhaps not to offend anyone?
After all, this is a critical error that causes the website to malfunction.
Why doesn't this be solved with great vigor and quickly?

 I started the whole thing as URL parameters

Why?
Who asked for it?
Why are you doing something that no one wants you have no idea about - because you don't know the context?

You may be a good developer (although such a developer wouldn't have a job with me for a long time), but you're not a project manager and you don't know the context.

Please return these codes and stop making mistakes-only decisions.

I will not solve with my clients that I will return tens of thousands of euros to them just because a developer thought that his solution was the right one.

Perhaps this example will teach you all - I've heard of other bad decisions in version 4 joomla code that cause incompatibilities and were surprisingly approved because the developer (creator) did not agree to remove them and persevered in his idea.
I still have to verify them, I have no idea how this situation could have happened.

Please follow the testing carefully and inform other developers in advance before publishing in the code.
Otherwise, I will charge you for the development of components and modules and you will repair it yourself.


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

avatar dgrammatiko
dgrammatiko - comment - 24 Oct 2021

Why?

It was explained earlier that this change was needed for lazily loaded images and most importantly because it solved a RELEASE BLOCKER for the external adapters

Who asked for it?

Again, release blocker means that the issue needed to be solved in order for Joomla 4 to be released...

Why are you doing something that no one wants you to have no idea about

Actually, I have a very good grasp of the project architectures thus the change was accompanied with a helper that makes it backward compatible, nothing will be broken if you pass your media field value through HTMLHelper::cleanImageURL

Otherwise, I will charge you for the development of components and modules and you will repair it yourself.

Just a note here: You cannot go around on OPEN SOURCE projects and threat contributors. You are using and profiting from the code that I and others wrote and offered to the project for FREE...

avatar brianteeman
brianteeman - comment - 24 Oct 2021

Otherwise, I will charge you for the development of components and modules and you will repair it yourself.

where do i send the invoice for the hundreds (perhaps thousands) of hours I have spent developing the cms you are using

avatar pinochico
pinochico - comment - 24 Oct 2021

I think, Your ideas are wrong :)

It was explained earlier that this change was needed for lazily loaded images and most importantly because it solved a RELEASE BLOCKER for the external adapters

Really?
I use lazily loaded images and dont need your img URL with params or hash.
So if I admit you're right and I admit your apology, why didn't you think about sharing pictures on facebook, instagram, og, ...
What if I export this information to a feed?
Have you ever seen an image sitemap where image URLs have parameters and hastags?

I don't know who has ever seen any parameters or hashtags added to an image URL

Why, when you need a lazzyload image function, don't you write a new external standalone function that only lazzyload calls, but the normal image URL will remain compatible with all existing external applications?

It bothers me that you don't think systemically and in context.

It bothers me that you think that when something is free, you can program whatever you want, including mistakes and bad decisions.

We already had that here - 50 years of communism and everything for free and we know how it turned out - no one was responsible for anything, everything was all - look at the destroyed lives, monuments, buildings, nature ....

You are responsible for 100 bil. installing the Joomla CMS.
You are responsible for the application developers for Joomla.
You are responsible for the template developers for Joomla.
Don't forget that.

You make the wrong decision and the developers leave.
There will be no applications, no templates, no installations.
Then you can develop your ideas as you want and what you want.

Again, release blocker means that the issue needed to be solved in order for Joomla 4 to be released...
You don't see the world around you for the problem.
You focus too much on the detail (release blocker) and you miss the context.
You may have wanted to help, but the result is not correct.

Actually, I have a very good grasp of the project architectures thus the change was accompanied with a helper that makes it backward compatible, nothing will be broken if you pass your media field value through HTMLHelper::cleanImageURL

OK, but:

Why don't you keep the original functionality and write a new function for a new function (lazzyload or lazily) - that's how it's done logically and correctly in programming.

This will maintain compatibility with all existing applications and allow new functionality with new applications.
(why should I use the new HTMLHelper :: cleanImageURL function now ???)

You are convincing me of something that is bad in terms of application development.

You forget about thousands and thousands of developers who will have to redesign their applications thanks to you. Do you think they will reprogram?

According to discussions at Joomla Cafe, they will not remodel. They would rather not upgrade applications from J3 to J4 (etc. see Form2Content, this developer is done).

 Just a note here: You cannot go around on OPEN SOURCE projects and threat contributors.

I can.
I can come and say you're making a mistake to be careful.
Developers are not infallible and are not gods, so that nothing critical can be said to them.
That's why we have a three-tier code quality system for application development.

To Brian:

where do i send the invoice for the hundreds (perhaps thousands) of hours I have spent developing the cms you are using

It doesn't mean you can do whatever you want, because the code is free.

I wrote it once, you have a responsibility.
If you do not have it, no one will use CMS Joomla and you can send invoices wherever you want.

P.S.
Maybe my contribution is critical, but that's the only way to know the limits of what's possible and how to move forward properly.

Thanks


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

avatar brianteeman
brianteeman - comment - 24 Oct 2021

You dont move forward you move backwards.

avatar dgrammatiko
dgrammatiko - comment - 24 Oct 2021

I use lazily loaded images and dont need your img URL with params or hash.

Screenshot 2021-10-24 at 21 20 19

As per the best practices from web.dev (https://web.dev/lazy-loading-best-practices/#wrong-layout-shifting ) the loading=lazy images SHOULD also include the attributes width and height so if you're doing it right it means that you're reading the image size per file in your runtime and obviously this is extremely naive and waon't scale (at some point the advantages you might get from the lazy loading tag are eliminated and might as well be hurting the performance, from the time wasted to calculate the sizes in the runtime)

@pinochico here's a challenge for you then. Following are the requirements for the media field:

  • The old Media field was saving a string, the new one needs to also save a string
  • The New one needs to save information for the External Adapter (ie: images could live on AWS S3, Dropbox, etc), the unresolved path
  • The new media needs to support saving information for width, height, and in the future versioning of the assets
  • The new media field needs to support also document, video, and audio type files
  • The new media needs to be B/C (eg OLD/Existing data should still be ok)
  • The new media field should not be a different field but an enhancement of the old

I'm very curious about your solution to all these problems with a different approach than the one the project decided to go with.

avatar pinochico
pinochico - comment - 24 Oct 2021

You dont move forward you move backwards.

Yes, I move backwards.
Definitely :)

Like Facebook and Instagram ...
How can they even afford not to accept a progressive "In Future" solution from CMS Joomla for image?

Why don't they accept

osmbanner1.png#joomlaImage://local-images/banners/osmbanner1.png?width=468&height=60

But rather

osmbanner1.png

They move backwards. I'm sorry 😄


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

avatar brianteeman
brianteeman - comment - 24 Oct 2021

add an image to an article. share the article on facebook. facebook uses the image.

your problem is?

avatar pinochico
pinochico - comment - 24 Oct 2021

I have other information:

The result is a relatively large backward incompatibility. This will apply, for example, plugins for OpenGraph data (Facebook rejects images with a hashtag in the url), plugins / templates for creating automatic previews, etc.


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

avatar dgrammatiko
dgrammatiko - comment - 24 Oct 2021

The result is a relatively large backward incompatibility. This will apply, for example, plugins for OpenGraph data (Facebook rejects images with a hashtag in the url), plugins / templates for creating automatic previews, etc.

So? Joomla 4 was never advertised that will be 100% compatible with J3. The project tried and in most cases kept the breaking changes to the minimum. Then there are things that can not satisfy all the requirements and thus you get a breaking change. This one FYI is not the only breaking change...
Also what we are talking about here is that devs that updating their extensions need to use a helper function to get the clean URL, it's not that the project doesn't provide this and they have to come up with their own code for accomplishing this task...

avatar PhocaCz
PhocaCz - comment - 25 Oct 2021

Hi @anibalsanchez

Thank you for reporting this issue and thank you for documenting it.

I stumbled across this post completely by accident and discovered that most of my extensions are affected/broken thanks to this easter egg.

img

To all core developers: thank you for your hard work, but if you think only on a local level (core Joomla) and not on a global level (Joomla is not only core, but is made up of various extensions that are very important for the system - WP popularity is based on extensions), then such undocumented changes will cost us the loss of many users (if there is still someone to lose, over 90% of Joomla users around me disappeared - mostly because of Joomla 4).

For me personally, this is the fifth unpleasant surprise in a short time. I personally will overcome it, Joomla is a hobby for me, but I understand that for developers who use Joomla on a professional level it can be a big problem and a reason to complain.

Joomla has made over 5 million dollars in the last 10 years. The people who are complaining are the ones behind those earnings - they clicked on ads, bought services or goods from sponsors, created a community or circle of customers who in turn interacted with Joomla and thus helped it make money, etc. etc. So to argue that users can't complain because someone is making Joomla for free is false.

I appreciate people like that because these people who care have created a comprehensive community system where I can pursue my hobby. People who don't care, just quietly leave the system without any feedback.

But the main thing is. People don't complain about you creating something for free. People are complaining that something that worked and was created by someone else, you changed or removed. And that's important to understand. In that case, you are not seen as people creating something for free, but the opposite, as people destroying something for free. And this is important to understand when reading such a discussion. You see yourself in the mirror as the creator, but others see you as the destroyer.

Even if the feedback is unpleasant, it's always good to reflect on it.

@dgrammatiko

... reading the image size per file in your runtime ... be hurting the performance

I haven't followed those past discussions, so my thoughts can be completely false. But there is a difference between retrieving information dynamically for each user and once when saving options. If we read the size on the fly, this can be problem but when saving a parameter and wrote that size as other parameters, then the load on the system would not be there. Just like when thumbnails for images are created. They are created once and then used all the time. Wouldn't it be possible to save the size information when saving the options? If there is a function that clears the image url, why can't there be an opposite function that retrieves the image size from the saved parameter options?

avatar anibalsanchez
anibalsanchez - comment - 25 Oct 2021

You are welcome @PhocaCz

The objective of the ticket is to feature and communicate the issue. I think that this is a big one that it's going to resurface frequently.

About the rest of the discussion, I don't have much to say. It's already accepted, and we have to follow.

avatar anibalsanchez
anibalsanchez - comment - 25 Oct 2021

@dgrammatiko For future reference, the URLs#Hashtags are frequently used on fat Web Apps (such as the apps that social networks use) to route the internal requests. So, they naturally block any URL that could mess with the App router.

Consequently, now all Joomla extensions must consistently clean the image URLs before generating them on the frontend to avoid leaking image metadata or internal information to other systems.

avatar dgrammatiko
dgrammatiko - comment - 25 Oct 2021

Just like when thumbnails for images are created. They are created once and then used all the time. Wouldn't it be possible to save the size information when saving the options?

The thumbs are physically created in a predefined path, but I hardly can figure out where the extra data for the field value could be safely stored. The DB endpoint for the media value is a plain text column...

If there is a function that clears the image URL, why can't there be an opposite function that retrieves the image size from the saved parameter options?

The problem is not in the logic but rather in the fact that the field in J4 needs to hold information for:

  • the external adapters
  • the image size
  • the version of the media file
  • more attributes related to video/audio/pdf

In short, it's not JUST the image size attributes but you're welcome to prove all the people involved wrong...

if there is still someone to lose, over 90% of Joomla users around me disappeared - mostly because of Joomla 4

@PhocaCz it's not because of version 4. It's because the web is moving away from the traditional monolithic applications to a more modern architecture that is based around CDNs and EDGE computing. It's not Joomla's fault but things evolve...

@dgrammatiko For future reference, the URLs#Hashtags are frequently used on fat Web Apps (such as the apps that social networks use) to route the internal requests. So, they naturally block any URL that could mess with the App router.

@anibalsanchez thanks for validating that this is not something that was invented by Joomla but it's rather a common pattern for many other apps (especially SPAs)...

avatar PhocaCz
PhocaCz - comment - 25 Oct 2021

@PhocaCz it's not because of version 4. It's because the web is moving away from the traditional monolithic applications to a more modern architecture that is based around CDNs and EDGE computing. It's not Joomla's fault but things evolve...

Yes, there may be some influence, but the Wordpress case shows us that this is not the case. If Joomla had 65% of the market instead of 2.8%, like Wordpress has with outdated code whose main goal is to maintain backward compatibility, probably no one would even think of new modern architecture, etc. Wordpress is proof that modern architecture plays a marginal role in this case.

The problem is not in the logic but rather in the fact that the field in J4 needs to hold information for ...

What exactly is the problem of storing this information in auxiliary fields? That's why we have a database here, to help us store unlimited amounts of data.

From

{"title":"TITLE","title_type":"1","type":"article","image":"images\/joomla_black.png#joomlaImage:\/\/local-images\/joomla_black.png?width=225&height=50",

To

{"title":"TITLE","title_type":"1","type":"article","image":"images\/joomla_black.png","image_params":{"width": 225, "height", 50, ...}

_params or any unique name for each parameter will never be used, unless in function which replaces cleanImageURL and reads the parameters instead of deleting them from image URL.

avatar pinochico
pinochico - comment - 25 Oct 2021

@PhocaCz https://github.com/PhocaCz - bingo Jan :)

I don't know explain to Dimitris or others, why his login is unlogic - he
is like a horse with only one way :( without looking with the context

First MUST BE core - name of images must be without params and must be an
existing function for compatibility by history or 3d party developers
Second CAN BE add core new function - ok, lazzy or others... simply as
params in DB

I don't understand why this logic (from basic to extended, from old to
new...) he doesn't know and doesn't use - logic is a basic knowledge for
developers.

Rudolf Baláš

web support manager
@.***
+420 730 517 521
Minion Interactive s. r. o. | www.minion.cz
Jiráskova 16, 466 01 Jablonec nad Nisou
+420 480 023 139, email: @.*** @.***>

www.minion.cz | www.tvorbaloga.cz | www.tvorbainfografiky.cz
www.facebook.com/MinionInteractive
www.twitter.com/minioncz

po 25. 10. 2021 v 12:36 odesílatel Jan @.***> napsal:

@PhocaCz https://github.com/PhocaCz it's not because of version 4. It's
because the web is moving away from the traditional monolithic applications
to a more modern architecture that is based around CDNs and EDGE computing.
It's not Joomla's fault but things evolve...

Yes, there may be some influence, but the Wordpress case shows us that
this is not the case. If Joomla had 65% of the market instead of 2.8%, like
Wordpress has with outdated code whose main goal is to maintain backward
compatibility, probably no one would even think of new modern architecture,
etc. Wordpress is proof that modern architecture plays a marginal role in
this case.

The problem is not in the logic but rather in the fact that the field in
J4 needs to hold information for ...

What exactly is the problem of storing this information in auxiliary
fields? That's why we have a database here, to help us store unlimited
amounts of data.

From

{"title":"TITLE","title_type":"1","type":"article","image":"images/joomla_black.png#joomlaImage://local-images/joomla_black.png?width=225&height=50",

To

{"title":"TITLE","title_type":"1","type":"article","image":"images/joomla_black.png","image_params":{"width": 225, "height", 50, ...}

_params or any unique name for each parameter will never be used, unless
in function which replaces cleanImageURL and reads the parameters instead
of deleting them.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#35871 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AADASJ7ZT2WMRUFFTW266RDUIUXKZANCNFSM5GOBKEDQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

avatar dgrammatiko
dgrammatiko - comment - 25 Oct 2021

like Wordpress has with outdated code whose main goal is to maintain backward compatibility, probably no one would even think of new modern architecture, etc. Wordpress is proof that modern architecture plays a marginal role in this case.

Didn't WP have had a huge discussion on the Guttenberg changes? Weren't these B/C incompatible? Did their devs bite the bullet and finally support the new Editor instead of fighting against the changes (or at least this is what happened at the end)?

What exactly is the problem of storing this information in auxiliary fields? That's why we have a database here, to help us store unlimited amounts of data.

The example from the article image is not representative of all the use cases of the media field. Yes the articles could have the data saved in the JSON but media fields are not stored like that platform-wide, so in short this is not covering all the cases

avatar dgrammatiko
dgrammatiko - comment - 25 Oct 2021

I don't understand why this logic (from basic to extended, from old to
new...) he doesn't know and doesn't use - logic is a basic knowledge for
developers.

This is EXACTLY what is happening. The old data is preserved as it was, the new data is enhanced with the hash.
The issue raised here is extremely exaggerated. The only place that the images with the hash won't work (ie if the devs didn't clean the URL) is the sharing on Facebook and Twitter. Browsers will happily render those images even if their URL has the hash on them...

avatar PhocaCz
PhocaCz - comment - 25 Oct 2021

The only place that the images with the hash won't work (ie if the devs didn't clean the URL) is the sharing on Facebook and Twitter.

Now we know about one place, but the reality may be quite different. Because there are an unlimited number of extensions, there are an unlimited number of approaches to image processing (previews, thumbnails, caching, alternatives like webp, getting extension, etc). All functions that e.g. use some string or regex function over the image path can now collapse.

avatar dgrammatiko
dgrammatiko - comment - 25 Oct 2021

All functions that e.g. use some string or regex function over the image path can now collapse.

All those extensions they just need to call Joomla\CMS\HTML\HTMLHelper::cleanImageURL to get the clean URL but actually to become really compatible with version 4 they also need to support the external adapters which are now part of the core architecture. Anyways J3 plugins are not compatible with version 4 due to the Events changes so if devs are testing their own code before releasing their extensions they should catch this fairly early in the process...

avatar n3t
n3t - comment - 25 Oct 2021

Now we know about one place, but the reality may be quite different.

Agree. I can understand that we need to move forward, and do not stuck in past as WP did, however such changes should be either fully B/C or communicated well. Media field is used widely in 3rd party extensions, and this change is not obvious on first sight (as old data stays stored in old format), and until now (thanks to @anibalsanchez) not documented.

As I mentioned before, even media field was not explicitly told to hold relative paths, it always hold just relative path, so many developers take it like that. Maybe if media field was kept B/C, marked deprecated and there is new media field with this new functionality, or there is some new attribute of media field like "extformat"...

avatar PhocaCz
PhocaCz - comment - 25 Oct 2021

Another thing is whether an opt-out solution (cleanImageURL) is better in this case than some opt-in solution (addImageSuffix).

avatar pinochico
pinochico - comment - 25 Oct 2021

All those extensions they just need to call
Joomla\CMS\HTML\HTMLHelper::cleanImageURL

Oh My goodness :D

This is the promised right example of work (I saw the word EXACTLY
somewhere) how do you work correctly logically?
Keep old features for compatibility and add new features with new features?

It doesn't look like that at all.
You just decided, for yourself, you disguised it for open source, freedom
of speech and speech (thank you for the embarrassing post on twiter), for
the fact that J4 is not the same as J3 and therefore it is not necessary to
keep anything, for a lot of other bullshit - words just words and finally
you programmed it that way.

Why is the original functionality unchanged and for lazzyload (this is a
new function for external adapters) a new function was not added using
\ CMS \ HTML \ HTMLHelper :: lazzyimageURL

In addition, you do it completely wrong, the parameters are not given in
the image name, but are read from json from the DB and inserted into the
html parameters.

Unfortunately, no one is checking to see if your edits are causing the
problem.
There is no quality coder, no project manager, and no one, Brian, who two
years ago seemed to know what he wanted, now resigns to logical things
under the pressure of launching a new version.

I can already see how Google generates and indexes images from joomla and
e-shops with such a name - what do you think - cough it up.

You don't think about the context at all - I've written it several times
You have no responsibility at all - I've written it several times

I still hear only me, me, me, me and because open source I can do what I
want.

You will be very surprised when the developers cough up J4 and do not
upgrade their extensions. Why would they rewrite when Dimitris comes and
thinks up again.
But it will be too late.

Rudolf Baláš

web support manager
@.***
+420 730 517 521
Minion Interactive s. r. o. | www.minion.cz
Jiráskova 16, 466 01 Jablonec nad Nisou
+420 480 023 139, email: @.*** @.***>

www.minion.cz | www.tvorbaloga.cz | www.tvorbainfografiky.cz
www.facebook.com/MinionInteractive
www.twitter.com/minioncz

po 25. 10. 2021 v 13:19 odesílatel Dimitris Grammatikogiannis <
@.***> napsal:

All functions that e.g. use some string or regex function over the image
path can now collapse.

All those extensions they just need to call
Joomla\CMS\HTML\HTMLHelper::cleanImageURL to get the clean URL but
actually to become really compatible with version 4 they also need to
support the external adapters which are now part of the core architecture.
Anyways J3 plugins are not compatible with version 4 due to the Events
changes so if devs are testing their own code before releasing their
extensions they should catch this fairly early in the process...


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#35871 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AADASJ2RDGPDUJPHW77BYVTUIU4L5ANCNFSM5GOBKEDQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

avatar brianteeman
brianteeman - comment - 25 Oct 2021

wow - you really do talk a lot of ....

avatar dgrammatiko
dgrammatiko - comment - 25 Oct 2021

@pinochico you're extremely rude, you can open a PR with your very awesome code and prove how idiots all of us are...

Unsubscribing, enough is enough...

avatar PhocaCz
PhocaCz - comment - 25 Oct 2021

@dgrammatiko What do you expect when you mock people on Twitter that they won't get rude?

avatar pinochico
pinochico - comment - 25 Oct 2021

What?
Rude?

I describe the situation as it is - I only use your words that you wrote
here.

It doesn't matter, do what is a priority:

  • Correct the image name generation behavior correctly
  • check the results of developers before you release them into the world
    and think logically
  • Take responsibility

Rudolf Baláš

web support manager
@.***
+420 730 517 521
Minion Interactive s. r. o. | www.minion.cz
Jiráskova 16, 466 01 Jablonec nad Nisou
+420 480 023 139, email: @.*** @.***>

www.minion.cz | www.tvorbaloga.cz | www.tvorbainfografiky.cz
www.facebook.com/MinionInteractive
www.twitter.com/minioncz

po 25. 10. 2021 v 13:42 odesílatel Brian Teeman @.***>
napsal:

wow - you really do talk a lot of ....


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#35871 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AADASJ6NX4AUC5A2E3FRQ4LUIU7D7ANCNFSM5GOBKEDQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

avatar PhocaCz
PhocaCz - comment - 25 Oct 2021

BTW I found another problem. The HTMLHelper::cleanImageURL does not clean image url but returns object. It parses the path to object array of attributes and url. So if I understand it correctly, even the naming of the function is wrong.

For me, for example, in the single-file plugin (Phoca Open Graph) this means an extra seven conditions. I don't even want to know what it will mean in components with over 200 000 lines of code :-(

$imgClean = HTMLHelper::cleanImageURL($img);
if ($imgClean->url != '') {
   $img =  $imgClean->url;
}
echo $img;

instead of

echo $img;

In practice it will look like this:

function realCleanImageUrl($img) {

    $imgClean = HTMLHelper::cleanImageURL($img);
    if ($imgClean->url != '') {
        $img =  $imgClean->url;
    }
    return $img;
}

echo realCleanImageUrl($img);

It kind of reminds me of overengeneering.

avatar dgrammatiko
dgrammatiko - comment - 25 Oct 2021

One final comment:

I'm not the one that introduced the hash for the media field URL (my approach was URL params which was short-lived)

avatar pinochico
pinochico - comment - 25 Oct 2021

I have no problem acknowledging the mistake

So if the name of the image with hastag is not your product - I apologize
for your comments.
But I insist that it be programmed logically and correctly.
And it was someone who approves merge (mostly I saw Brian, as a senior
developer) to properly assess and go back to reworking.
Rudolf Baláš

web support manager
@.***
+420 730 517 521
Minion Interactive s. r. o. | www.minion.cz
Jiráskova 16, 466 01 Jablonec nad Nisou
+420 480 023 139, email: @.*** @.***>

www.minion.cz | www.tvorbaloga.cz | www.tvorbainfografiky.cz
www.facebook.com/MinionInteractive
www.twitter.com/minioncz

po 25. 10. 2021 v 14:23 odesílatel Dimitris Grammatikogiannis <
@.***> napsal:

One final comment:

I'm not the one that introduced the hash for the media field URL (my
approach was URL params which was short-lived)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#35871 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AADASJ3FMJDGQVNNX4DP6ZLUIVD4LANCNFSM5GOBKEDQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

avatar dgrammatiko
dgrammatiko - comment - 25 Oct 2021

But I insist that it be programmed logically and correctly.

From God? From a robot? From you?
And probably now you can understand why I posted that tweet. If you want to change something in an open-source project you have to propose the code for that change, nobody will do it for you. That's how open source projects work, contributing is the right way and definitely not just complaining and shaming other people's work...

And it was someone who approves merge (mostly I saw Brian, as a senior developer) to properly assess and go back to reworking.

Actually, the president merged that PR but many highly respected maintainers were involved...

avatar dgrammatiko
dgrammatiko - comment - 25 Oct 2021

@PhocaCz about your demonstrated code, the 1-1 change is like this:

// OLD CODE

echo $imageURL;

// New code
echo (HTMLHelper::cleanImageURL($imageURL))->url;
avatar PhocaCz
PhocaCz - comment - 25 Oct 2021

In case, I cannot use empty string (because in case of empty value, another rule is applied), I need to test it for empty value too.

avatar PhocaCz
PhocaCz - comment - 25 Oct 2021

then such undocumented changes will cost us the loss of many users (if there is still someone to lose, over 90% of Joomla users around me disappeared - mostly because of Joomla 4).

And every time we discuss, another one disappears during the discussion

https://www.joomdev.com/
joomdev

:-(

avatar dgrammatiko
dgrammatiko - comment - 25 Oct 2021

@PhocaCz from their post:

However, as the world reeled, we witnessed waves of rapidly evolving technologies that demand us to pivot for innovative exploration and catch the waves head-on

which align perfectly with my previous comment #35871 (comment):

@PhocaCz it's not because of version 4. It's because the web is moving away from the traditional monolithic applications to a more modern architecture that is based around CDNs and EDGE computing. It's not Joomla's fault but things evolve...

avatar PhocaCz
PhocaCz - comment - 25 Oct 2021

No, you are wrong, I speak with the people directly. People have respect for Joomla and don't talk about problems publicly to avoid unnecessary damage to the product name. Even though people are leaving Joomla, they still love the product and have no need to damage it. If you want to hear the reasons, we can discuss them privately. I discuss this with many people and I don't turn a blind eye to it. There's no point in discussing this further in this topic. It's not about me being right or you being right, it's just bad news for everyone :-(

avatar joomdonation
joomdonation - comment - 29 Oct 2021

Just for information, we have a simple method to get clean value (without adapter information......) from the value which is stored in media form field

use Joomla\CMS\Helper\MediaHelper;
echo MediaHelper::getCleanMediaFieldValue($oldValue);
avatar anibalsanchez
anibalsanchez - comment - 29 Oct 2021

Thanks @joomdonation. I have added it to the page: https://docs.joomla.org/Potential_backward_compatibility_issues_in_Joomla_4#Media_Field

I wonder why there are two methods with a similar name to do the clean as URL or the clean as Field Value. If the new value is an URL then what needs to clean and if it is a valid value what needs to be cleaned. If there is something dirt, what are you cleaning?

public static function cleanImageURL($url)
	{
		$obj = new \stdClass;

		$obj->attributes = [
			'width'  => 0,
			'height' => 0,
		];

		if (!strpos($url, '?'))
		{
			$obj->url = $url;

			return $obj;
		}

		$mediaUri = new Uri($url);

		// Old image URL format
		if ($mediaUri->hasVar('joomla_image_height'))
		{
			$height = (int) $mediaUri->getVar('joomla_image_height');
			$width  = (int) $mediaUri->getVar('joomla_image_width');

			$mediaUri->delVar('joomla_image_height');
			$mediaUri->delVar('joomla_image_width');
		}
		else
		{
			// New Image URL format
			$fragmentUri = new Uri($mediaUri->getFragment());
			$width       = (int) $fragmentUri->getVar('width', 0);
			$height      = (int) $fragmentUri->getVar('height', 0);
		}

		if ($width > 0)
		{
			$obj->attributes['width'] = $width;
		}
		else
		{
			unset($obj->attributes['width']);
		}

		if ($height > 0)
		{
			$obj->attributes['height'] = $height;
		}
		else
		{
			unset($obj->attributes['height']);
		}

		$mediaUri->setFragment('');
		$obj->url = $mediaUri->toString();

		return $obj;
	}
	public static function getCleanMediaFieldValue($value)
	{
		if ($pos = strpos($value, '#'))
		{
			return substr($value, 0, $pos);
		}

		return $value;
	}

It looks to me that you should have defined a new Media field type, Rich Media or something like that, to correctly represent the new domain object and process the new object correctly. In any case, it is too late to give it a new definition.

avatar joomdonation
joomdonation - comment - 30 Oct 2021

@anibalsanchez Thanks for adding information to documentation. Why have two methods? Short answer is each method do different thing:

  • The first one gives you full information of the image (url, width, height) which will be used for image lazy loading (not sure if I use right term, not familiar with these stuffs)
  • The later one gives you a quick and easy way to get just the path of the image (same with which we have on Joomla 3), so you can just have to change your original code abit to have it displayed same with J3.

Anyway, as you said, it is too late now. So we will have to move on with this change.

avatar joomdonation
joomdonation - comment - 30 Oct 2021

@@anibalsanchez If there is nothing else to discuss, could you please close the issue?

avatar PhocaCz
PhocaCz - comment - 31 Oct 2021

@@anibalsanchez If there is nothing else to discuss, could you please close the issue?

I think, someone should say:
a) there will be nothing changed, use the methods to clean the image path
b) or maybe there will be some change because in time we see, it is problematic feature
c) or etc.

So developers know how to manage their extensions regarding this issue.

Jan

avatar dgrammatiko
dgrammatiko - comment - 31 Oct 2021

b) or maybe there will be some change because in time we see, it is problematic feature

if you think that there is something wrong please provide some code to fix it, otherwise we will keep talking on theoretical basis which leads nowhere and confuses people

avatar PhocaCz
PhocaCz - comment - 31 Oct 2021

To not confuse people I just expect clear answer from the core team.

For example: a) is right, we don't plan to change anything there - closed.
I don't think, it is hard to answer a) or b) or c) and don't confuse people.

avatar anibalsanchez
anibalsanchez - comment - 31 Oct 2021

From my side, I think that the issue remains unsolved.

If you think that it is solved as part of the new J4 development definitions and methodology, feel free to close it.

avatar dgrammatiko
dgrammatiko - comment - 31 Oct 2021

@anibalsanchez @PhocaCz if you think there’s a problem with the current code please provide an alternative

avatar PhocaCz
PhocaCz - comment - 31 Oct 2021

@anibalsanchez @PhocaCz if you think there’s a problem with the current code please provide an alternative

What do you exactly mean? There is no alternative but standard feature - it is well known. The standard feature is to store image path as it is. Like it works in Joomla 3. If somebody needs to have/use more information for the image, why not to store the information next to image path parameter - in other database column/other JSON parameter?

If image path parameter is stored in database, why there is not additional parameter which will store all possible information about the image. Such information can be then retrieved via some function, as the image path is now being cleaned.

Why we should provide alternative to standard feature? Storing image path as it is - this is a standard feature.

{"image_intro":"images\/joomla_black.png#joomlaImage:\/\/local-images\/joomla_black.png?width=225&height=50","image_intro_alt":"","float_intro":"","image_intro_caption":"","image_fulltext":"","image_fulltext_alt":"","float_fulltext":"","image_fulltext_caption":""}

vs.

{"image_intro":"images\/joomla_black.png","image_intro_width":"225","image_intro_height":"50","image_intro_alt":"","float_intro":"","image_intro_caption":"","image_fulltext":"","image_fulltext_alt":"","float_fulltext":"","image_fulltext_caption":""}

avatar n3t
n3t - comment - 1 Nov 2021

As mentioned by @anibalsanchez, IMHO there should be new field type "Rich Media", or attributte of media field to return "rich" information, or just filename. As mentioned also before width / height info do not need to be enough in many usecases, commonly used info could be also MIME type.

Also, consider situation, when user goes to media manager and replace the image file with new one (as he preview his new article and decides to put little bit different image). In image field remains completely wrong dimensions then.

I was curious with mentioned performance impact, so I made some profiling - measure time and memory spent for 10k calls of HTMLHelper::cleanImageURL vs. JImage::getImageFileProperties, and of course cleanImageURL is much faster, but, for 10k of calls (which would mean 10k articles in one page) it showed me aprox. 400ms difference, meaning for lets say 100 articles it is cca 4ms, for 10 articles (which is I believe default paging size for Joomla) it is cca 0.4ms, not calculating with caching at this point (so the difference is only on expired cache articles).

As mentioned by @dgrammatiko developers should also modify their extension to allow media from other filesystems (like dropbox etc.). The new rich media field type will give them B/C with option to move forward. New field would also open possibility not to store the data in string format, but as json for example.

The biggest issue of the new media field behavior is IMHO "partial" compatibility, partially is working, but actually it breaks B/C.

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

Why we should provide alternative to standard feature? Storing image path as it is - this is a standard feature.

This DOESN'T work for external adapters as it assumes that the images are stored locally.

If image path parameter is stored in database, why there is not additional parameter which will store all possible information about the image.

Because the media field has ONLY ONE value, what you're proposing is multiple fields/values...

Jan, probably you're missing the point here: Joomla 4 has:

  • support for external adapters
  • support for the lazyloading attribute

These 2 NEW features require extra information. The PT choose to go with the hashed URL for this data. BUT even if they chose to go with the JSON route the requirement to pass the value from a helper function would have been mandatory for keeping the backward compatibility (eg old value images\/joomla_black.png new value {"URL": "mages\/joomla_black.png"} so there would be a try/catch thing to handle the string or JSON value). Keeping the old field and introducing a new one wouldn't save the day and also would introduce 2 ways to do the same thing but the one wouldn't support the new features which makes no sense as Joomla 4 is a major release and devs are required to adjust their code to support the new features/changes. Last but not least, the code has already been shipped, so...

avatar anibalsanchez
anibalsanchez - comment - 1 Nov 2021

I have the idea to create a plugin to process all IMG tags and clean all URLs.

However, it would be better if it is done at the core level.

In the same way that Joomla creates the SEF URLs when they are not jrouted, all the IMG URLs could be cleaned at once. In this way, you avoid enforcing cleaning image URLs to all developers (an error-prone step and easily forgettable).

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

Two other issues (discussed in the community).

The following issues are of course controversial, they may seem very strange to some, but it is also good to point them out:

  • image column size - People who like to optimize the database may have set the image path fields too short, so e.g. for their extension this will be b/c and need to modify database columns

  • this is very controversial but a lot of people try to hide as much as possible information about the tool which generates the website (e.g. because of security reasons), so for them there is new part which they need to hide (as the path included information about the CMS name). Of course such people are not the extension developers and they cannot use clean function directly in the code, so maybe some specific system plugin needs to be done.

As I wrote above, we can make up our own minds about this, but it's worth stating.

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

I have the idea to create a plugin to process all IMG tags and clean all URLs.
However, it would be better if it is done at the core level.
In the same way that Joomla creates the SEF URLs when they are not jrouted, all the IMG URLs could be cleaned at once. In this way, you avoid enforcing cleaning image URLs to all developers (an error-prone step and easily forgettable).

Kinda disagree here, the developers are responsible for the HTML code that their extensions produce. If core needs to handle every small thing for the devs the performance will be impacted very badly

for their extension this will be b/c and need to modify database columns

So they need to fix their DBs to become J4 compatible

people try to hide as much as possible information about the tool which generates the website

How is this even related? Extensions SHOULDN'T echo the value with the hash so if the devs are doing some pretty basic testing they will catch this and pass the value from one of the 2 core provided functions. So this is not even a problem?

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

How is this even related? Extensions SHOULDN'T echo the value with the hash so if the devs are doing some pretty basic testing they will catch this and pass the value from one of the 2 core provided functions. So this is not even a problem?

We are talking about reality here, not dreams, 90% of developers have no idea that they have to clear image path in J4. And if one of the arguments of the proponents of this solution is that the hash is not a problem in most cases, then developers won't clean it either (unless it is e.g. open graph data, etc.). Everyone is struggling with time, and there are many other priorities when rewriting extensions to J4, so I assume, even with the claim that hash is not an issue, that this will not be a priority when rewriting extensions.

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

Whatever the reasons behind this feature, in principle it is a very unfortunate solution. Sort of like, if you want to go swimming in the summer, you have to put on a coat and then take it off before swimming. Logically, most people would then ask: Isn't it possible to just go without a coat?

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

that this will not be a priority when rewriting extensions.

Actually, if a dev wants to declare their extensions J4 ready there are some things that need to be met:

  • Vanilla JS
  • Proper use of all the new features that J4 offers, eg: image lazyloading, external adapters, etc.
  • The list is quite big...

But that's MY definition of Joomla 4 compatible extension (I guess most devs will be happy to just make their things not break in J4)

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

Why should anyone use Vanilla JS when it is economically disadvantageous. Why shouldn't someone use their own framework, or an external framework, or even a helper library like jQuery?

img

If such rules existed, then there would be about 1% of extensions for Joomla and it would immediately end. Satisfied users don't ask what library the developer uses, but how the extension works overall. Such lists remind me of 1939.

BTW Does the media manager use Vue.js?

avatar pinochico
pinochico - comment - 1 Nov 2021

Dimitris,

Your definition is only chaos and broken code
Your definition is only about your BIG ego
*Your *definition is not logic and wrong way for developing

When will you finally understand, apologize and rework the original code?

I don't understand why you're so incomprehensible.

The basic rule of application development is:

  • functional functionality does not change
  • add new functionality as a new function

You have not followed this application development rule.
We are now harvesting the rotten apples of your cultivation.
Finally stop lying in your own pocket, beat your chest and talk: me, me, me
just me and fix back to the working original functionality.
Add new requirements as a new function without destroying the original
function.

I don't know what you're still inventing and writing, start acting and
programming - as my grandmother says - there was enough talk, only
politicians talk, now you need to work.

Rudolf Baláš

web support manager
@.***
+420 730 517 521
Minion Interactive s. r. o. | www.minion.cz
Jiráskova 16, 466 01 Jablonec nad Nisou
+420 480 023 139, email: @.*** @.***>

www.minion.cz | www.tvorbaloga.cz | www.tvorbainfografiky.cz
www.facebook.com/MinionInteractive
www.twitter.com/minioncz

po 1. 11. 2021 v 12:39 odesílatel Dimitris Grammatikogiannis <
@.***> napsal:

that this will not be a priority when rewriting extensions.

Actually, if a dev wants to declare their extensions J4 ready there are
some things that need to be met:

  • Vanilla JS
  • Proper use of all the new features that J4 offers, eg: image
    lazyloading, external adapters, etc.
  • The list is quite big...

But that's MY definition of Joomla 4 compatible extension (I guess
most devs will be happy to just make their things not break in J4)


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#35871 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AADASJYE6V6PHXAALZFPVX3UJZ37FANCNFSM5GOBKEDQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

Why should anyone use Vanilla JS when it is economically disadvantageous. Why shouldn't someone use their own framework, or an external framework, or even a helper library like jQuery?

The inflexion point MATTERS. If you're about to ship SO MUCH javascript that you're passing the infection point probably you should use newer technologies like Web Components or a library (Vue, Preact, etc)

Satisfied users don't ask what library the developer uses,

I thought that users were satisfied because of the performance that J4 offers OOTB which in part is due to vanilla js, but what do I know...

When will you finally understand, apologize and rework the original code?

I HAVE NOTHING TO APOLOGISE FOR AND ESPECIALLY TO YOU. Take your offensive comments elsewhere, ok?

now you need to work.

SERIOUSLY NOW? YOU EXPECT ME TO WORK FOR FREE FOR YOU? WHO DO YOU THINK YOU ARE?

avatar anibalsanchez anibalsanchez - change - 1 Nov 2021
Status New Closed
Closed_Date 0000-00-00 00:00:00 2021-11-01 12:02:11
Closed_By anibalsanchez
avatar anibalsanchez anibalsanchez - close - 1 Nov 2021
avatar anibalsanchez
anibalsanchez - comment - 1 Nov 2021

The discussion is going nowhere.

if someone wants to change how the system works, you can submit a PR. However, it is already accepted that it works in this way.

Case closed.

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

The inflexion point MATTERS. If you're about to ship SO MUCH javascript that you're passing the infection point probably you should use newer technologies like Web Components or a library (Vue, Preact, etc)

The chart I used for jQuery is also valid for other frameworks. Its purpose is to show the economic advantage of using a framework. It's not about what particular framework or library is ultimately used. It's about not prohibiting someone from using the procedures that the advantage of economy of scale bring. Strictly insisting on using Vanilla JS is not OK in my opinion.

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

Strictly insisting on using Vanilla JS is not OK in my opinion.

Yes that was a wrong definition from me, I should write "J4 compatible ext should use more current JS than jQuery"

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

Ok

avatar joomdonation
joomdonation - comment - 1 Nov 2021

@PhocaCz @anibalsanchez Actually, @dgrammatiko is not responsible for this change. It was me who implemented the code in the PR #33724 base on what we were having before and the suggestion by @HLeithner #33724 (comment)

Joomla! 4 media manager supports multiple adapters and we had to find a solution for a release blocker issue #26750. I am unsure if we could find a solution without changing how value is stored in the field. Yes, maybe a new field type would help, but the code was shipped, Joomla 4 was release and we could not change that now

The solution for developer who faces that problem is one line of code change as commented here #35871 (comment) . It is a small change compare to the other works we have to do to make our extensions compatible with Joomla! 4, and won't take much time.

avatar brianteeman
brianteeman - comment - 1 Nov 2021

@joomdonation seems people like the sound of their own voices more than they like to contribute.

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

@PhocaCz @anibalsanchez Actually, @dgrammatiko is not responsible for this change. It was me who implemented the code in the PR #33724 base on what we were having before and the suggestion by @HLeithner #33724 (comment)

I hope it is clear from my comments and Anibal's that we are discussing the merits of the system and are not interested in who is the creator. I personally give examples here of where this issue can be problematic. I just think it's good to list them. How they are dealt with is a matter for the core team. I don't care who made it. I'm interested in the functionality and issues of the feature. And not from my point of view, as a developer I have the advantage that I can treat it. Which is not the case for system administrators.

avatar joomdonation
joomdonation - comment - 1 Nov 2021

@PhocaCz Not follow the discussion from beginning but Yes, I can see the issue (we were not aware about at that time). However, there was a reason for that change and that issue could be addressed easily as mentioned during the process of converting extension to Joomla 4. I don't think we change that from Joomla! core now, so let's close this issue and move forward.

avatar Chaosxmk
Chaosxmk - comment - 1 Nov 2021

I don't quite have the know-how to even suggest a PR for this, but would it be possibly to throw an attribute onto the field that could disable the hash being added on save? This way images that can benefit from the hashed data will be able to continue working as is, and for the rare fringe cases where the hash data isn't wanted or needed it could be turned?

avatar anibalsanchez anibalsanchez - change - 1 Nov 2021
Status Closed New
Closed_Date 2021-11-01 12:02:11
Closed_By anibalsanchez
avatar anibalsanchez anibalsanchez - reopen - 1 Nov 2021
avatar anibalsanchez
anibalsanchez - comment - 1 Nov 2021

In my view, the whole topic of Media Management deserves better storage than a hashtag.

A plugin could need more room to store data than the constraints imposed by an URL/filename-based field. In the future, a plugin could also store more info related to the media and how to reproduce it. For instance, an external video, start time, end time, and captions.

The main problem seems to be how the media manager handles the media selection and how the metadata is stored.

If there is a general will to change the current feature and fix it, we can change the current behavior addressing it as a design bug.

Otherwise, it makes no sense to discuss how the metadata can be stored in an alternative way that's not related to the URL/filename field representation.

avatar anibalsanchez
anibalsanchez - comment - 1 Nov 2021

BTW: The conversation is back in a civil manner. I have re-opened the issue.

avatar joomdonation
joomdonation - comment - 1 Nov 2021

I don't quite have the know-how to even suggest a PR for this, but would it be possibly to throw an attribute onto the field that could disable the hash being added on save

Unfortunately, I don't think it is possible. If I remember correctly, we need to have the following information from the hash to navigate you to the right folder where the file is stored:

  • Adapter name (local, dropbox)
  • Path which the media file/image is stored

Not sure if I miss anything else.

avatar joomdonation
joomdonation - comment - 1 Nov 2021

@anibalsanchez If you or anyone can come up with a solution for the problem, I'm sure people here will be interested. There are few things we need to handle:

  • Multiple adapters
  • Width and height of the image for lazy loading

And the solution must be backward compatible with what we are having right now.

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

@anibalsanchez here's what the selected image is holding

Screenshot 2021-11-01 at 16 10 25

The resolution from path: "local-images:/joomla_black.png" to url: "images/oomla_black.png" is done before closing the modal.

At that point you can dump the Object to JSON, it's that easy. Why didn't that happen before J4 release? That's a big discussion but basically, it's down to lack of contributors, a handful of people tried to fix a large number of Release Blockers, get BS5, etc...

avatar anibalsanchez
anibalsanchez - comment - 1 Nov 2021

@joomdonation The point is separating: 1. the metadata storage 2. the App requirements to render the image 3. The frontend Image rendering.

Now, the three aspects are tied together in the field representation.

avatar Chaosxmk
Chaosxmk - comment - 1 Nov 2021

Unfortunately, I don't think it is possible. If I remember correctly, we need to have the following information from the hash to navigate you to the right folder where the file is stored:

* Adapter name (local, dropbox)

* Path which the media file/image is stored

Not sure if I miss anything else.

Ahh, bummer. Well, I figured I'd suggest it anyways just in case it was doable.

Either way, in the end my future projects will natively support and handle this, it's just a bit of a pain in projects that are upgrading from 3 to 4.

avatar joomdonation
joomdonation - comment - 1 Nov 2021

Either way, in the end my future projects will natively support and handle this, it's just a bit of a pain in projects that are upgrading from 3 to 4

If you have to keep the old behavior, then you just need one line of code changed as mentioned earlier :).

avatar joomdonation
joomdonation - comment - 1 Nov 2021

Now, the three aspects are tied together in the field representation.

So what could be a better solution compare to what we are having right now? I still don't see a way yet :(. If anyone can come up with idea, we would be happy to discuss further.

avatar anibalsanchez
anibalsanchez - comment - 1 Nov 2021

Idea:

  1. The media metadata has its own assets table, managed by a new model and helper RichMediaModel or RichMediaHelper. There is no need to "clean" anything.
  2. The media manager can receive the current data and store it in this field
  3. The media manager can retrieve the required routing data
  4. The frontend can render the image with or without the metadata retrieved from the new assets table
avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

@anibalsanchez you're joking, right?

avatar anibalsanchez
anibalsanchez - comment - 1 Nov 2021

Jokes always have an emoticon

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

From the author or the others?

avatar Chaosxmk
Chaosxmk - comment - 1 Nov 2021

Idea:

1. The media metadata has its own assets table, managed by a new model and helper RichMediaModel or RichMediaHelper. There is no need to "clean" anything.

2. The media manager can receive the current data and store it in this field

3. The media manager can retrieve the required routing data

4. The frontend can render the image with or without the metadata retrieved from the new assets table

Having previous experience with saving that kind of data to a DB table and later fetching it, I can say that it produces a dramatic hit to performance on each image. Sure, it may come off easier for storage, but the performance loss isn't worth it.

avatar anibalsanchez
anibalsanchez - comment - 1 Nov 2021

I mean, I would have expected a better metadata representation than a hashtag. That's why I initially opened the issue when I discovered the problem. At this point, I think that the source of the problem is the misunderstanding of the media metadata size and that it can be hacked into the field where it was originally defined as the media field.

What if I have 2K of media metadata?

Joomla saves everything in tables. So, it is the way to make it work for articles. We are not talking about every little image, only about Rich Media.

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

I mean, I would have expected a better metadata representation than a hashtag.

The hash solution serves primarily the legacy data, eg it will continue to be valid even if you haven't removed it, the hash just means there's more...
BTW I can post the few lines needed for the JSON implementation, the funny part is that you will STILL need a helper function for the try/catch to make the field B/C compatible. So the whole conversation here is kinda going nowhere as most people suggest things that just don't work or the impact will be way more severe...

What if I have 2K of media metadata?

It doesn't matter what you might have, as the core adds only the adapter/path,width,height. In short, you can't save in there whatever you want, it's data coming from the selector part of the core...

avatar n3t
n3t - comment - 1 Nov 2021

What about storing metadata like this

{
  "image_intro":"images\/joomla_black.png",
  "image_intro_meta": {
    "width":"225",
    "height":"50",
    "adapter":"local-images",
    "adapter_path":"images\/joomla_black.png",
  }
}

This would keep B/C with J3 storage and open unlimited space for storing additional meta data.

Still I am skeptical about storing width and height in meta data, first it means, that converted articles from J3 will not have lazy loading, second when someone change the image by media manager or something else (FTP), stored data are invalid (this could solve extra assets table with some kind of regenerate meta data function, but it would mean extra SQL queries).

If stored like this, maybe media field could create one extra hidden field for meta data, and storing in correct format could be handled when saving article (anyway storing in WYSIWYG is solved separately now)? JS just checks if second field for metadat exists or not, and fill only when needed...

avatar Chaosxmk
Chaosxmk - comment - 1 Nov 2021

Still I am skeptical about storing width and height in meta data, first it means, that converted articles from J3 will not have lazy loading, second when someone change the image by media manager or something else (FTP), stored data are invalid (this could solve extra assets table with some kind of regenerate meta data function, but it would mean extra SQL queries).

To be fair, it doesn't necessarily break the images, just any functions that attempt to use the URL to manipulate or obtain data (ie. PHP's getimagesize()). Images with the hash data still display just fine.

avatar n3t
n3t - comment - 1 Nov 2021

I meant, when width and height are stored in metadata (either using hash or anything else), and afterwards I open media manager (not article, but directly media manager), and replace the original image with different size, rendering in frontend will be invalid (lets say original image was 1000x500px, new one is 500x1000px)...

avatar n3t
n3t - comment - 1 Nov 2021

Also, imagine for example plugin creating automatically thumbnails. It could store in meta data info about created thumbnails, which could open option to use srcset and sizes attributes...

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

which could open option to use srcset and sizes attributes...

Yeah, it's solved since 2016 and also proper versioning/cache invalidation: https://responsive-images.dgrammatiko.dev

avatar pinochico
pinochico - comment - 1 Nov 2021

Thank you all for reopening PR and finding a new non-conflict solution. (thank you Paul)


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

avatar n3t
n3t - comment - 1 Nov 2021

@dgrammatiko Thanks for pointing to this plugin, didn't know about it. However I tried on J40 and it doesn't seem to work, there is fatal error on JSON encoding.

However seems to me, that it is solving just images stored in HTML text (introtext, fulltext etc.), replacing directly the HTML code. What I meant was ability to store this in images field like

{
  "image_intro":"images\/joomla_black.png",
  "image_intro_meta": {
    "adapter":"local-images",
    "adapter_path":"images\/joomla_black.png",
    "sizes":{
       "1900":"thumbnails\/1900\/joomla_black.png",
       "1200":"thumbnails\/1200\/joomla_black.png",
    }
  }
}

or whatever else. Just open meta data for storing plugins custom values. But it was just a fast idea :)

avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

@n3t yes that JSON error is known in windows systems but I don't do windows, so...

What I meant was the ability to store this in images field like

Read my previous reply, this WOULD break badly ALL existing extensions...

EDIT: @n3t the plugin should be fine now

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

Read my previous reply, this WOULD break badly ALL existing extensions...

But similar way how the current solution breaks all existing extensions:

  • which do regex on image paths,
  • which don't have enough database column length for image parameters,
  • where the image is set as base for creating thumbnails,
  • where authors do not have any idea about this b/c feature,
  • etc.
avatar dgrammatiko
dgrammatiko - comment - 1 Nov 2021

But similar way how the current solution breaks all existing extensions:

There are a few differences that you forgot to mention here:

  • Joomla 4 was a major version and breaking changes were allowed, your next chance is Joomla 5
  • There are 2 Helper functions to help devs NOT break things
  • The new URL with the hash is totally valid for the browser so the only breaking part has to do with PHP code assuming the URL is a relative path which obviously can't be true anymore since J4 can have the files stored externally
avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

@brianteeman What exactly does your dislike mean? Does it mean, such problems do not exist or such problems are marginal, or? I have listed some possible problems with current solution and don't understand the dislike (please, this is not some kind of confrontation, I would really like to know what exactly did you mean with the dislike?)

avatar n3t
n3t - comment - 1 Nov 2021

Read my previous reply, this WOULD break badly ALL existing extensions...

Not necessarily, if there is opt-in RichMedia field as proposed by @anibalsanchez.

We have already hash solution for J4 with helper function, so keep it, as there are surely already templates, extensions etc. working with this, and surely some stored data. Just provide alternative for storing meta data. Use it in articles, where images are stored in json, so there should not be big impact. Width / Heights for Lazy Loading read on fly, it will bring better compatibility with J3 stored data, as these old articles will start to use lazy loading also, and should solve most of complaints here, as most of it is related with article images.

"Legacy" media field could then work either in current J4 way, or store just filename. In such case it would not provide external adapters option, so developers could decide if they want adapters support in their extension (switch to richmedia field or modify media field to contain hash data), or not at the moment (for example when the file need to be processed locally). The question here is if storing hash data should be opt-in (better compatibility with J3 data), or opt-out (adapters support out of box).

And if there is update check converting those hash urls in article images to json format, this should be fully compatible with everything. I understand this is big change, but IMO it could give better B/C (lazy loading for old articles, invalid handling of new image format), and open more possibilities for plugins how to work with images (store extra metadata).

If we wait for J5, this discussion will propably be here again, as it would mean big change again, so give possibilities just now, and use "deprecated" more than "B/C break".

EDIT: @n3t the plugin should be fine now

Still the same, the issue is in content plugin on line 76 - "($json) must be of type string, stdClass given" (PHP 8, Windows).

avatar brianteeman
brianteeman - comment - 1 Nov 2021

So for those of us that followed development, tested alpha, beta and release candidates and built our sites, extensions and templates accordingly you think it is ok to break all our work.

avatar n3t
n3t - comment - 1 Nov 2021

No, and I didn't say anything like that. As I wrote, keep current solution, plus provide alternative and conversion of stored data.

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

So for those of us that followed development, tested alpha, beta and release candidates and built our sites, extensions and templates accordingly you think it is ok to break all our work.

That's not a good argument, as an inspector I had to break down countless walls, walls that someone had been building for a long time. Yes, I was breaking someone's long term work, but if there was simply a threat that a wall could be dangerous, I had to do it. But we didn't get that far here. No one here has suggested breaking someone's work, but mostly parallel solutions. There can't be any question of breaking someone's work with a parallel solution.

avatar brianteeman
brianteeman - comment - 1 Nov 2021

@n3t then I misunderstood your comment that articles should use a new alternative to the hash option.

avatar brianteeman
brianteeman - comment - 1 Nov 2021

@PhocaCz Then as @dgrammatiko has repeatedly said - propose some code alternative.

avatar PhocaCz
PhocaCz - comment - 1 Nov 2021

I'm a realist, none of my PR has been accepted and basically quietly ignored, my motivation to contribute to core is zero, but that doesn't mean I can't voice my opinion and list the problematic parts here.

avatar pinochico
pinochico - comment - 12 Nov 2021

to @dgrammatiko
Is fixed finally? == are you changed code to back to compatible version from J3 and add NEW function for external adapter or really NOT?

to @brianteeman
Please do not enter into a discussion about a problem that you have caused by insufficient control of the new code, poor project work, and allowing such code == paskvil to see the light of day and be included in version J4.
You don't deserve it.

Once you've corrected your mistakes and mischievous behavior, you can rejoin the discussion.

You enable the work of a person who does not have the basic prerequisites for the development of quality code, you stand up for it, you approve of its bad development practices and you support its behavior outside of Joomla.

This situation is all the sadder and incomprehensible the longer you work for Joomla (when I regularly monitor and check your previous posts in joomla discussions and in working for joomla).

Think and find an excuse for your behavior.

avatar brianteeman
brianteeman - comment - 12 Nov 2021

I have no role. I have no ability to approve or merge a pull request. I am a volunteer just like everyone else. The difference is I roll up my sleeves and contribute my time

avatar brianteeman
brianteeman - comment - 12 Nov 2021

and blocked

avatar dgrammatiko
dgrammatiko - comment - 12 Nov 2021

@pinochico as I said many times I’m not gonna change any code because I believe this was the most b/c version that Joomla could have. Also as I said before you don’t need anyones permission to showcase how this can be done in a better way. Show us your code … 👨‍💻

avatar anibalsanchez
anibalsanchez - comment - 13 Nov 2021

@dgrammatiko Speaking of code, would be accepted a fix to restore the original Field/MediaField and introduce a new Field/RichMediaField? We can discuss the details, but if the Product Team has already settled on the idea that this is "the way", then it is just a waste of our time to think of a solution.

At this point, I prefer to close the issue and let it go.

avatar dgrammatiko
dgrammatiko - comment - 13 Nov 2021

Speaking of code, would be accepted a fix to restore the original Field/MediaField and introduce a new Field/RichMediaField?

I think if you keep b/c with the current state of the media field it could be accepted, but better ask the maintainers, I’m not part of the decision makers, I just contribute ideas/code.

avatar dgrammatiko
dgrammatiko - comment - 13 Nov 2021

@anibalsanchez here's a more constructive reply:

The media field at its current state supports:

  • the hash part that holds the adapter info and for images the width and height
  • a new attribute named types with possible values images, videos, audios, docs defaults to images for b/c

A possible replacement could store the data as a stringified JSON, eg #35871 (comment) but probably would be better to separate the field into 4 new fields: image, video, audio and document (extending a base extendedMediaField or something like that).

So, to recap, the current media field apart from the hash part also supports audio, video and documents. Returning to the old media field would break both the lazyloading (it's a USP for the project afaik) and the external storage and create a breaking change for existing data with the hash. The new field(s) would also require a helper function to convert at the very least the stringified JSON to an object. So, although I personally am in favour of structured data the solution seems hard to satisfy all the requirements (I might be wrong and you might have a viable solution, if so then please share an abstract so we could evaluate it).

avatar anibalsanchez
anibalsanchez - comment - 13 Nov 2021

I am also in favor of a structured data solution. Other systems transitioned to a media representation that supports current and future media types.

For instance, this is Laravel - Spatie's media library (a popular package): https://github.com/spatie/laravel-medialibrary/blob/main/database/migrations/create_media_table.php.stub

        Schema::create('media', function (Blueprint $table) {
            $table->bigIncrements('id');

            $table->morphs('model');
            $table->uuid('uuid')->nullable()->unique();
            $table->string('collection_name');
            $table->string('name');
            $table->string('file_name');
            $table->string('mime_type')->nullable();
            $table->string('disk');
            $table->string('conversions_disk')->nullable();
            $table->unsignedBigInteger('size');
            $table->json('manipulations');
            $table->json('custom_properties');
            $table->json('generated_conversions');
            $table->json('responsive_images');
            $table->unsignedInteger('order_column')->nullable();

            $table->nullableTimestamps();
        });

WordPress has a simple postmeta table with media_key and media_value, where they store whatever they need without worrying too much about the structure, modeling, or compatibility.

Those are two examples of how the metadata can be represented with a dynamic structure. In both cases, there is a table involved.

avatar HLeithner
HLeithner - comment - 13 Nov 2021

In my personal opinion, the solution is working and the change for 3rd party devs (and core) is minimal invasive, it only requires one function call to clean the meta data. Since @anibalsanchez updated the documentation to reflect the change and the fact that we have already release 4 versions with this structure I'm not in favor to change this behavior.

The alternative we discussed was a second metadata field which needed much more work to support by all developer and has a bigger performance impact on frontend. That's also the reason we didn't chose json.

And to tbh while converting my own components, this part was the smallest I had to consider.

Also I'm not sure how you would save the information without adding meta data to the image. As already mentioned we wanted to be minimal b/c breaking so the decision was made to extend the url value with the information.

The string we produce:
images/joomla.jpg#joomlaImage://local-images/joomla.jpg?width=2093&height=3140

Looking at the string that we produce we ultimative need the this part:
joomlaImage://local-images/joomla.jpg

if we don't have this string we cant' reopen the image int he mediamanager. the additional media information width and height are optional but doesn't changed.
I also suggest this format because it would allow us to write a file wrapper for easier and transparent handling.

how the structured data is saved is a discussion made at the end of the development process of j4 (too late but it was not easy and we know it would never be optimal).

In the end it doesn't matter if you safe the data in json or url formated (which could be read by juri).

If you need a plain field in your component you can suggest an alternative solution for the media field that simply only saves the url in the value field and no other information (that would work but breaks some comfort for the user) but if you need it in your component it's possible to add it.

Something with a xml attribute store="plain", don't know at the moment if this is good or not but could work. Also I don't know if I would excepted but this depends not only on me so it's possible.

avatar anibalsanchez
anibalsanchez - comment - 13 Nov 2021

We have already detailed the problems of the current solution above, and the discussion is about how to change it.

If you don't acknowledge what we already detailed above and what other people are doing, there is nothing else to add.

avatar dgrammatiko
dgrammatiko - comment - 13 Nov 2021

Something with a xml attribute store="plain", don't know at the moment if this is good or not but could work. Also I don't know if I would excepted but this depends not only on me so it's possible.

That won't magically work because the JS that populates the input returns the URL with the hash, so, in order for it to work the JS should somehow read some data attribute aand change the injected value to the URL without the hash. It's doable, but Joomla 4 SUPPORTS natively the loading=lazy and external storages so such a solution effectively will be INCOMPATIBLE with Joomla 4...

avatar pinochico
pinochico - comment - 13 Nov 2021

It's doable, but Joomla 4 SUPPORTS natively the loading=lazy and
external storages so such a solution effectively will be INCOMPATIBLE with
Joomla 4...

Because you already ruined it with your code

Fix the functionality to the original function from J3 and do not create
data attributes for JS using a hash, but using attributes as parameters for
media (if someone wants to use it at all in Joomla == they will call a NEW
function and not the crap you messed up)

Rudolf Baláš

web support manager
@.***
+420 730 517 521
Minion Interactive s. r. o. | www.minion.cz
Jiráskova 16, 466 01 Jablonec nad Nisou
+420 480 023 139, email: @.*** @.***>

www.minion.cz | www.tvorbaloga.cz | www.tvorbainfografiky.cz
www.facebook.com/MinionInteractive
www.twitter.com/minioncz

so 13. 11. 2021 v 12:28 odesílatel Dimitris Grammatikogiannis <
@.***> napsal:

Something with a xml attribute store="plain", don't know at the moment if
this is good or not but could work. Also I don't know if I would excepted
but this depends not only on me so it's possible.

That won't magically work because the JS that populates the input returns
the URL with the hash, so, in order for it to work the JS should somehow
read some data attribute aand change the injected value to the URL without
the hash. It's doable, but Joomla 4 SUPPORTS natively the loading=lazy
and external storages so such a solution effectively will be
INCOMPATIBLE with Joomla 4...


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#35871 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AADASJ72367UWOZORUNGG5DULZDVXANCNFSM5GOBKEDQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

avatar dgrammatiko
dgrammatiko - comment - 13 Nov 2021

Because you already ruined it with your code

Thank you for your kind words.

That's it for me then, no more Joomla contributions. I guess @pinochico can do better than my clumsy code.

Bye-bye and thanks for all the fish.

avatar HLeithner
HLeithner - comment - 13 Nov 2021

We have already detailed the problems of the current solution above, and the discussion is about how to change it.

If you don't acknowledge what we already detailed above and what other people are doing, there is nothing else to add.

I read thru most of the thread but didn't found the real problem, maybe you can help me to find the right position?

but it's ok if you don't want to help me understand the problem.

avatar PhocaCz
PhocaCz - comment - 13 Nov 2021

BTW, I am not sure if this information here:
https://docs.joomla.org/Potential_backward_compatibility_issues_in_Joomla_4#Form
img

is valid, because it seems that the function name is wrong.

Function HTMLHelper::cleanImageURL() does not clean the Image URL but returns an object where one part of the object is the cleaned URL

So this:

echo \Joomla\CMS\HTML\HTMLHelper::cleanImageURL($oldValue);

should return Object of class stdClass could not be converted to string instead of cleaned URL

Maybe it should be something like this:

$imgClean = HTMLHelper::cleanImageURL($img);
if ($imgClean->url != '') {
   $img =  $imgClean->url;
}

I didn't analyze the function, so no idea if it can somehow return empty result for url part, so maybe condition to test for empty string is needed. But this depends on the prudence settings of individual programmers, anyway the code should probably be changed (if I'm not mistaken).

Jan

avatar anibalsanchez
anibalsanchez - comment - 13 Nov 2021

@HLeithner As described above, the original media field had a relative path to the file (directly representing the image location). Now, it doesn't have the URL or the file path.

If you lower the bar, you could say that if you don't clean the URL, it is a "compatible" URL; and that all developers should be aware of the change and re-write everything to the new field value. That's where we are now.

Additionally, in the short term, the solution will not be enough to store the metadata that can be associated with a media field. For instance, if you have to store video metadata, it can easily have a length beyond what an URL hash can hold. As a second example of the shortcomings, if, by mistake, the URL is not cleaned, the internal metadata could be shown to the visitors. So, it is better to avoid storing in the hash any sensitive info (like the plugin service account identification).

As shown in the systems referred, you could add at least an assets table with a JSON field to solve the issue related to the metadata storage, instead of trying to save the metatada in an URL hash.

avatar PhocaCz
PhocaCz - comment - 13 Nov 2021

I don't know if it's worthy to keep explaining the essence of the problem here, but imagine if in Joomla 5 we decided to store the titles together with the alias:

My Title#my-title

And we'd want everyone who printed a title to now print that title via the Clean function, which removes everything after the hash.

Doesn't this strike you as odd, unconventional, outside the standard parameter storage process?

If we're talking about the fact that it's easy to convert an URL via the clean function now, it's not. It may also mean changing the database structure, since not everyone has prepared media field as infinite storage type.

So instead of using new rich media field in core and let extension developers decide which field they will use, we force them to use clean function, possibly changing their database structure.

I'm trying to understand both sides of the problem, but I don't understand the missed opportunity to use a new rich media field type.

avatar anibalsanchez
anibalsanchez - comment - 14 Nov 2021

In terms of system design, the original "image-only" media field and the "Rich Media" field should be the same. The original "image-only" case can be supported with a "Rich Media" field.

If we could identify the bug (not a coding bug), then we can fix it. A system design can also have a bug and there is no additional burden on that. It is part of the system designer's life. ;-)

It looks to me that the issue is only in the metadata storage. If we move it to a different place, then it is solved.

PS: Thanks @dgrammatiko for all your efforts. We can't thank you enough for all you have done.

avatar HLeithner
HLeithner - comment - 14 Nov 2021

Thanks @anibalsanchez

So the main point is you want a mediafield that is only able to show local images? That kills the most important feature of the new media manager (adapter). But ok why not if a 3rd party dev want's it, it should be possible.

About the second point (leaking information) normally in the additional information shouldn't be sensitive data. But I understand your concern.

The intention of this upgrade was to support all features of the new media manager with the minimal effort we (or 3rd party) need to change to the source. Seems that didn't worked out sorry for that.

We expected that every brave^^ developer/user uses JHTML::image to display an image which removes the metadata automatically... But I understand that people don't use always our api.

About assets table, I personally have an extension which exactly have this, but tbh I think that should be done with the extension of the media manager to use the database it self. There is/was a roadmap for this but I have no idea if there is any progress or anyone working on it (I think not).

What I can say for the core, it's not possible to change it until 5.0 (at least the return of the #metadata structure), since we already have several 1000? j4 sites that would all break. so we still need to support this syntax.

Introducing a new way to save the metadata could only be an optional addition. added in 4.1 (or 4.2) because 4.1 feature acceptance is coming to an end.

@PhocaCz we have major versions to be allowed to break things and we don't break things because it's funny only when "we" (who ever this is) thinks that it improves joomla with the lowest negative impact on the eco system.

avatar PhocaCz
PhocaCz - comment - 14 Nov 2021

@PhocaCz we have major versions to be allowed to break things and we don't break things because it's funny only when "we" (who ever this is) thinks that it improves joomla with the lowest negative impact on the eco system.

@HLeithner Maybe you wrongly understood this:

I'm trying to understand both sides of the problem, but I don't understand the missed opportunity to use a new rich media field

The problem in this thread is not about making changes, but about how to make them. Please don't impose the position that I am opposed to changes (breaking changes), that is not what my statements are about at all.

Yes, I think, all understand this:

We expected that every brave^^ developer/user uses JHTML::image to display an image which removes the metadata automatically... But I understand that people don't use always our api.

This is unfortunately a fantasy (use 100% Joomla API methods only), I also tried to use only core methods and I ended up rewriting code 3 - 5 times more often than other developers. Not to mention that if we send the url of the image to the meta tags, then this method doesn't help us (the same when we use regex on urls, etc. etc.).

avatar HLeithner
HLeithner - comment - 14 Nov 2021

@PhocaCz the reason we improved / reused the original field was to reduce the effort a developer have to invest to use the new media manager and it's possible that our expectations or own usage differs from reality that's the reason we have RCs so people can tell us if we are wrong of if something is broken before we finally release the software.

Since this field was one of the last release blockers and have been highlighted as such in the issue tracker, it has been fixed at a last stage in the RC release cycle. There was much time to contribute before it has been fixed but sadly not much time after we merged it into the core. Hopefully we make it better next time.

avatar anibalsanchez
anibalsanchez - comment - 14 Nov 2021

@HLeithner Please, understand our feedback about storing the metadata as a hash in what used to be the media field value. We aren't discussing the new features introduced with the new media management.

If you agree that storing media metadata in the hash is not a good solution, then we can move forward with the discussion.

If there is no consensus, then we'll have to live with the decision and wait to see if the hash storage is widely adopted (or avoided) and if the developers start to clean the media URLs. If you receive new issues related to the #data in the media URLs, then you'll notice new waves of complaints. I think it should be fixed sooner than later, pick if it has to be solved for 4.1, 4.2, etc.

avatar PhocaCz
PhocaCz - comment - 27 Nov 2021

Today, I found another issue:

This simple check:

if (File::exists(JPATH_ROOT . '/' . $image)) {

does not work anymore. Needs to be replaced by:

$imgClean = HTMLHelper::cleanImageURL($image);

if ($imgClean->url != '') {
   $image =  $imgClean->url;
}

if (File::exists(JPATH_ROOT . '/' . $image)) {

So it extends problematic areas to:

  • Meta tags images
  • Regex on images
  • Filesystem check on images
avatar brianteeman
brianteeman - comment - 27 Nov 2021

Surely that depends on the contents of $image ?

avatar dgrammatiko
dgrammatiko - comment - 27 Nov 2021

if (File::exists(JPATH_ROOT . '/' . $image)) {

That piece of code was NEVER working as expected. The URL could very easily be https://some.domain/someFolder/someImage.jpg

Also, the solution you provided could be simpler but you missed all the constructive comments, there's another helper to get the clean PATH
That said, ignoring the fact that J4 supports adapters and the image could be in another server makes the extension here incompatible with J4 (the code is making a huge assumption [local image] that is totally wrong...).

avatar PhocaCz
PhocaCz - comment - 27 Nov 2021

Thank you for the info, has the media field some parameter to limit only internal images?

avatar dgrammatiko
dgrammatiko - comment - 27 Nov 2021

Thank you for the info, does the media field have some parameter to limit only internal images?

No, it never had, thus all the comments in this issue as I said before are hugely exaggerated and come from a misunderstanding for what the actual value of the media field was and still is: a URL.

avatar PhocaCz
PhocaCz - comment - 27 Nov 2021

Thanks for the info, that's bad news, I'll have to come up with more control mechanisms to split between internal and external images and to blend the URL with the internal image path.

avatar dgrammatiko
dgrammatiko - comment - 27 Nov 2021

@PhocaCz in the process of creating this mechanism would be helpful for the core and the rest of devs if you can make a suggestion for a helper function that covers your case (I know that the core doesn't have any plugins dealing with images therefore this is unchattered waters)

avatar PhocaCz
PhocaCz - comment - 27 Nov 2021

Come to think of it, my line actually does everything I need:

if (File::exists(JPATH_ROOT . '/' . $image)) {

I just need to know if the image exists on the server. Worked well in J3, in J4 the clean image function needs to be used. What notation will be used, or what conditions, or what function, doesn't really matter. The important thing is that now, compared to J3, some cleanup function must be used.

J3:
if (File::exists(JPATH_ROOT . '/' . $image)) { ... internal image == TRUE
if (File::exists(JPATH_ROOT . '/' . $image)) { ... external image == FALSE

J4:
if (File::exists(JPATH_ROOT . '/' . $image)) { ... internal image == FALSE
if (File::exists(JPATH_ROOT . '/' . $image)) { ... external image == FALSE

avatar n3t
n3t - comment - 27 Nov 2021

No, it never had, thus all the comments in this issue as I said before are hugely exaggerated and come from a misunderstanding for what the actual value of the media field was and still is: a URL.

As I noted before, this is not mentioned in documentation, nor in MediaField file. What could lead to this idea is just naming convention in images field in articles, but media field could be use elsewhere than there.

In fact, in Joomla 3, this was always relative path to image file, so the code
if (File::exists(JPATH_ROOT . '/' . $image)) {
worked well, and is used in many extensions / templates I have seen.

@PhocaCz in the process of creating this mechanism would be helpful for the core and the rest of devs if you can make a suggestion for a helper function that covers your case

Maybe completely new MediaHelper would be nice. With functions like cleanUrl, parseData, isLocal, getPath etc...

From my point of view, one huge incompatibility remains, which is stored data from J3, missing the extra info, so lazy loading will not be applied. Means templates / extensions will still need to create B/C compatibility layer for this.

avatar dgrammatiko
dgrammatiko - comment - 27 Nov 2021

From my point of view, one huge incompatibility remains, which is stored data from J3, missing the extra info, so lazy loading will not be applied. Means templates / extensions will still need to create B/C compatibility layer for this.

I've started such a component: https://github.com/ttc-freebies/com_addlazyloading but I had to pause the development there to help fixing stuff so J4 could be released. You're welcome to contribute to that repo...

avatar n3t
n3t - comment - 28 Nov 2021

I've started such a component: https://github.com/ttc-freebies/com_addlazyloading but I had to pause the development there to help fixing stuff so J4 could be released. You're welcome to contribute to that repo...

Nice, I will check. However IMO this sholud be part of some upgrade process directly in Joomla...

avatar AndreaDelton
AndreaDelton - comment - 25 Feb 2022

I was using XT Adaptive Images and it worked fine with the previous image linking system. Now with the # in the link it can no longer provide alternative images from intro_image or full_image of the articles, but only from images within the article (with intro and full image it only works with old articles and links without #). It is a problem.

avatar dgrammatiko
dgrammatiko - comment - 25 Feb 2022

It is a problem.

No it's a problem for the developer of that extension that should have tackled before announcing that it's compatible with J4. You can use an alternative though: https://responsive-images.dgrammatiko.dev

avatar andreagentil
andreagentil - comment - 25 Feb 2022

I was using XT Adaptive Images and it worked fine with the previous image linking system. Now with the # in the link it can no longer provide alternative images from intro_image or full_image of the articles, but only from images within the article (with intro and full image it only works with old articles and links without #). It is a problem.

Hi,

we are already working on a new feature to support J4.1 media field.

Please, open an issue at https://support.extly.com/ and we will send you the early version to test it.

avatar anibalsanchez
anibalsanchez - comment - 26 Feb 2022

@dgrammatiko I didn't know that now it is possible to promote extensions here. In any case, our extension mentioned before is XT Adaptive Images https://www.extly.com/utilities/xt-adaptive-images.html.

For more information, you can also visit our page on the Joomla Extensions Directory. https://extensions.joomla.org/


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

avatar AndreaDelton
AndreaDelton - comment - 27 Feb 2022

Thanks to Gentil and Sanchez.
@dgrammatiko, thank you and I had already tried his plugin, which he pointed out to me, and which I managed to make it work well in the articles (thanks to the parameters already set), adapting the overrides of two of the most used frameworks, but which I failed to use for other extensions or modules. For example I tried with an Owl Carousel and set all the parameters detectable by the database (the images are in the "params" column), but I couldn't get adaptive images and we know that, especially in the home pages, there are often several modules with images and which is usually the heaviest page for many sites. I think it is not an easy plugin for a non-expert user (it is not easy to correctly identify the parameters to be reported in "Component Views" and "Component Database Columns") and with little documentation and examples to use it easily with other components/modules.


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

avatar brianteeman
brianteeman - comment - 15 Apr 2022

Is there any reason for this to stay open?

Add a Comment

Login with GitHub to post a comment