Skip to content

[4.0] Add possibility to add a custom path in media field#31398

Closed
Razzo1987 wants to merge 1 commit intojoomla:4.0-devfrom
Razzo1987:patch-2
Closed

[4.0] Add possibility to add a custom path in media field#31398
Razzo1987 wants to merge 1 commit intojoomla:4.0-devfrom
Razzo1987:patch-2

Conversation

@Razzo1987
Copy link
Contributor

Pull Request for Issue #28852 .

Summary of Changes

Enable the media field for input custom url

Testing Instructions

Enable the patch, try to create an article and set a custom URL in the Intro Image, for example https://via.placeholder.com/350x150

Actual result BEFORE applying this Pull Request

You can not insert custom URL:
immagine

Expected result AFTER applying this Pull Request

You can insert custom URL and after save the preview is displayed:
immagine

Documentation Changes Required

NO

@dgrammatiko
Copy link
Contributor

Sorry but this is wrong as explained already #28852 (comment). You can do that in your installation (eg layout override) but not for core, you're breaking the intended functionality

@Razzo1987 Razzo1987 changed the title Add possibility to add a custom path in media field [4.0] Add possibility to add a custom path in media field Nov 14, 2020
@Razzo1987
Copy link
Contributor Author

In J3 there is a validation from the URL?

@dgrammatiko
Copy link
Contributor

In J3 there is a validation from the URL?

No and that was wrong and fixed in J4

@Razzo1987
Copy link
Contributor Author

The feature of "validate the URL" is planned for J4.0 or J4.1 (or higher)?

@dgrammatiko
Copy link
Contributor

The feature of "validate the URL" is planned for J4.0 or J4.1 (or higher)?

No AFAIK

@Razzo1987
Copy link
Contributor Author

What do you think about enable the field in J4.0 and disable in the release in which the feature is added?
So in J4.0 we can add custom URL as in J3.x
If someone input in J4.0 a non valid URL in the filed is the same issue as he input in J3.x and then update to J4.0

@dgrammatiko
Copy link
Contributor

dgrammatiko commented Nov 14, 2020

@Razzo1987 Also a more broad comment here:

Allowing any kind of url in that field is allowing basically what is known as hotlinking which in most cases is plain plagiarism ( https://simple.wikipedia.org/wiki/Hotlinking ). If you need an image in your site upload it there. If you're advanced user and you need the ability of freely inserting URLs then create an override and remove the readonly attribute (good luck providing all the needed width and height params for correctly using lazyloading: #30784 ).

So in J4.0 we can add custom URL as in J3.x

This was an intended change (eg prevents hotlinking and allows the media manager integration the way it was envisioned). So from me it's a no. IMO the core shouldn't allow manually inserted url's in the media field

@adj9
Copy link

adj9 commented Nov 14, 2020

I have tested this item ✅ successfully on 5dee54b

Done

Frontend - Category List Articles

  • Article
    Schermata 2020-11-14 alle 15 13 30

Backend

If you change the URL to an invalid one (https://www.wiki) there is currently no control.

Checking it seems useful to me.


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

@ghost
Copy link

ghost commented Nov 25, 2020

I have tested this item ✅ successfully on 5dee54b


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

@ghost
Copy link

ghost commented Nov 27, 2020

set RTC

@ghost
Copy link

ghost commented Nov 28, 2020

why not RTC?

@dgrammatiko
Copy link
Contributor

why not RTC?

Read my comments, this PR is wrong!!!

@ghost
Copy link

ghost commented Nov 28, 2020

then close it

@infograf768
Copy link
Member

@Gostn FYI, @dgrammatiko is not authorised to close a PR.
Only the original poster and maintainers/dev leaders can.

@Razzo1987
I'm afraid Dimitris is right. Closing the PR now. Thanks for the proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants