[4.1] Remove readonly on media fields#36945
[4.1] Remove readonly on media fields#36945brianteeman wants to merge 1 commit intojoomla:4.1-devfrom
Conversation
PR for joomla#28852 The field was readonly for the reasons states by @dgrammatiko and I respect those views. But the request for the ability to paste an image url directly into the field by @coolcat-creations and others also makes sense. As you can see its a super simple PR to achieve this. All the features of the media field are maintained and you can paste an image url directly. However when pasting a url you do not get the data attributes and lazy loading. I believe this to be an acceptable solution.
|
Hi @brianteeman thank you so much for creating the PR! I thought the right solution would be to "validate the URL, get the image adapter, get the image dimensions" I see it coming that people complain that lazy load does not work. So how can we make a good solution to 1) make the paths "pasteable" 2) have it working like intended by media manager team ? Or would this be the first step, and maybe in 4.2 we can create a pseudocron which checks the image paths and adds this markup ? |
That's kind of what Dimitiris was telling you. If you want to paste the image then you cant have the other parts when you do |
|
Yes, maybe I misunderstood him, I thought it was possible to do these checks for pasted paths but that he does not want to do it so he suggested a bad solution over the right solution. Did I misunderstood? |
|
Fwiw the code for getting the size of the image already exists since #34177 so what is missing is a way to:
just Because it’s easy to remove an attribute it doesn’t mean that the result will be automatically acceptable. For me it’s not, the media field goes from a field that always produces valid urls to accepting any kind of strings… |
|
Why isn't it possible to let the user put in the path and have a button which says "validate and save" or something? |
That's not how form fields validation works in HTML. You have an event |
|
So what is the right approach ? Where do we need to change what line? |
|
@brianteeman the preview dosn't work with image generated like https://via.placeholder.com/350x150 |
|
Hi @dgrammatiko , Today, the situation does not seem to have changed. Here (#28852 (comment)) you were also talking about integration with third-party sources. Another beautiful idea that I don't see it in the roeadmap for Joomla! 4.1 or 4.2. Personally I think that for now the best way is to allow users to enter the custom URL as they did in Joomla 3, when we have a media manager who does the necessary checks and inserts images from Dropbox, AWS, S3, custom, etc we can again put back the read-only attribute. |
|
I already stated what needs to be done here #36945 (comment) but then again I'm not a gatekeeper or some kind of authority here, so if allowing any string in this field value is acceptable then this PR is fine |
|
Yes thanks but for me you could have write it in Japanese and i would not know what to do now :-) Maybe someone knows. I hope. |
|
I have tested this item ✅ successfully on 233ad78 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36945. |
|
I have tested this item 🔴 unsuccessfully on 233ad78 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36945. |
|
I have tested this item ✅ successfully on 233ad78 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36945. |
|
I have tested this item 🔴 unsuccessfully on 233ad78 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36945. |
|
I have tested this item ✅ successfully on 233ad78 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36945. |
2 similar comments
|
I have tested this item ✅ successfully on 233ad78 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36945. |
|
I have tested this item ✅ successfully on 233ad78 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36945. |
|
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36945. |
|
@RickR2H @brianteeman I object here. What this PR is doing is WRONG! ignoring that there's no validation and you can end up with FOOBAR values and 404 in your pages and the fact that this essentially bypasses the lazy loading feature there's something even more hurtful: the images will NOT have width and height attributes meaning that these images will contribute negatively in the Layout shifting metrics and thus hurting SEO!. Just let the people that want this functionality to create a layout override and themselves remove the readonly attribute OR find someone to write the few lines of JS needed so this could not have any MAJOR impact in performance and SEO. Web Archive 2022 is in the way and afaik Joomla is not helping itself if keeps the last position in the ranking. My 2c and unsubscribing |
|
It would be really good to have the possibility to paste the path AND have width height - User Experience for selecting images is zero right now. |
|
Setting the RLDQ (release leads decision queue) label to get a decision. |
You're not improving things by just making them worst, do you? I, keep repeating that this COULD be done reasonably with some JS but not just removing the readonly attribute simply because the results are not the same. Either fix it properly or keep the bad UX (according to you) which at least was done to improve SEO and perf... |
|
As for "security" anybody with some wit can remove the readonly attribute using DevTools from their browser. Validation should always happen server side. |
If it's so easy to improve it why won't you help? I can't because I don't know how. |
i tried |
|
@dgrammatiko how much would it cost to fix it in a proper way? |
|
Please close this and test #37654 |
|
Happy to |
9 participants
PR for #28852
The field was readonly for the reasons states by @dgrammatiko and I respect those views. But the request for the ability to paste an image url directly into the field by @coolcat-creations and others also makes sense.
As you can see its a super simple PR to achieve this. All the features of the media field are maintained and you can paste an image url directly. However when pasting a url you do not get the data attributes and lazy loading. I believe this to be an acceptable solution.
To test try to use the full article image or a custom media field and make sure you can still select an image and that you can now paste an image url