Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.1] Do not throw a warning when the image url is null on clean #37888

Merged
merged 10 commits into from Jun 18, 2022

Conversation

laoneo
Copy link
Member

@laoneo laoneo commented May 25, 2022

Summary of Changes

On PHP 8.1 a warning is displayed when the image to clean is null. This happens on empty media form fields.

Testing Instructions

  • Open the back end article form
  • Open the source of the HTML page
  • Search for deprecated

Actual result BEFORE applying this Pull Request

A deprecated message is shown for images.

Expected result AFTER applying this Pull Request

No deprecated message.

@laoneo laoneo added PHP 8.x PHP 8.x deprecated issues and removed PR-4.1-dev labels May 25, 2022
@alikon
Copy link
Contributor

alikon commented May 26, 2022

did you mean this ?
image

@laoneo
Copy link
Member Author

laoneo commented May 26, 2022

Nope, there should be a Notice in the source itself around the subform

@joomdonation
Copy link
Contributor

I can see this notice from PHP error logs. It comes from from our media form field. I wonder if we should fix the code which call this method instead? Shouldn't we only call the method if the url is not empty ?

@alikon
Copy link
Contributor

alikon commented May 27, 2022

i've replicated the issue opening System -> Global Configuration cause of offline image

@laoneo
Copy link
Member Author

laoneo commented May 30, 2022

I can see this notice from PHP error logs. It comes from from our media form field. I wonder if we should fix the code which call this method instead? Shouldn't we only call the method if the url is not empty ?

What would be the advantage?

@joomdonation
Copy link
Contributor

What would be the advantage?

Better code, save the system from having to execute unnecessary commands. It is logical that before displaying image, we check that the image is available before running more code to prepare data to display that image.

@laoneo
Copy link
Member Author

laoneo commented May 30, 2022

Then make your own pr with your better code. I prefer to fix it on the source and make a stable library instead of telling everyone "please do the null check" when using this function.

@bembelimen
Copy link
Contributor

As this PR fixes it confirmed, I will merge it.

@bembelimen bembelimen merged commit 76345e7 into joomla:4.1-dev Jun 18, 2022
@bembelimen
Copy link
Contributor

Thx

@bembelimen bembelimen added this to the Joomla 4.1.5 milestone Jun 18, 2022
@laoneo laoneo deleted the j4/html/image-81 branch January 13, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHP 8.x PHP 8.x deprecated issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants