Skip to content

[4.1] Fix Deprecated warning for Calendar Form Field#37373

Merged
rdeutz merged 6 commits intojoomla:4.1-devfrom
joomdonation:patch-6
Apr 20, 2022
Merged

[4.1] Fix Deprecated warning for Calendar Form Field#37373
rdeutz merged 6 commits intojoomla:4.1-devfrom
joomdonation:patch-6

Conversation

@joomdonation
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

This PR fixes deprecated warning for Calendar form field (use in core which all has translateformat="true" in it's form field definition). For calendar form field does not have translateformat="true", we need a different PR. I think this is safe to go in 4.1 to prevent warnings like this in core.

Testing Instructions

  1. Use Joomla 4 on PHP 8.1
  2. Go to System -> Global Configuration, set Error Reporting to Maximum
  3. Edit an article, look at Publishing tab

Actual result BEFORE applying this Pull Request

You would see some deprecated warnings like:

Deprecated: Function strftime() is deprecated in /libraries/src/Form/Field/CalendarField.php on line 277

Expected result AFTER applying this Pull Request

No deprecated warning anymore, the date time you select for Start Publishing, Finish Start Publishing... are being saved properly.

Documentation Changes Required

None

@laoneo
Copy link
Member

laoneo commented Mar 25, 2022

I would love to see a way, that extensions can define a filter format (not sure why it has that name) in the XML. Can you add that @joomdonation?

@joomdonation
Copy link
Contributor Author

Yes, in second PR #37376 (via filterformat attribute). For this PR, I want to have safe fix which can go into 4.1. It prevent deprecated warnings for calendar form fields use in core.

@joomdonation joomdonation changed the title [4.1] Fix Deprecated warning for Calendar Form Field [4.1] Fix Deprecated warning for Calendar form field use in core Mar 25, 2022
@joomdonation joomdonation added the PHP 8.x PHP 8.x deprecated issues label Mar 25, 2022
@laoneo
Copy link
Member

laoneo commented Mar 25, 2022

I will see if I can make a pr for 4.1 which will also work for extensions based on this one as I have a bad feeling shipping core for the next months with no way for extension developers to use a core field without a warning on PHP 8.1.

@laoneo
Copy link
Member

laoneo commented Mar 25, 2022

I have tested this item ✅ successfully on ce908fe


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

@joomdonation
Copy link
Contributor Author

I will see if I can make a pr for 4.1 which will also work for extensions based on this one as I have a bad feeling shipping core for the next months with no way for extension developers to use a core field without a warning on PHP 8.1.

@laoneo All we need to do is add one more line of code to this PR:

$this->filterFormat = (string) $this->element['filterformat'] ? (string) $this->element['filterformat'] : '';

Then extension developers can just add filterformat attribute with right value to his field and it will work well. It is backward compatible, does not change any existing behavior. It is just if maintainers (especially @bembelimen ) want to accept this change to 4.1.

@laoneo
Copy link
Member

laoneo commented Mar 26, 2022

I don't see this as an issue not to get merged into 4.1 as the benefit is high enough to have a clean way. For 4.2 we can then even deprecate the old format attribute. What I was thinking of is that the name filterformat is a bit strange.

@joomdonation
Copy link
Contributor Author

For 4.2 we can then even deprecate the old format attribute

We use that format in calendar javascript code, so it could not be deprecated

What I was thinking of is that the name filterformat is a bit strange

Yes, agree. I guess it is named like that because the format is used in filter method of the field https://github.com/joomla/joomla-cms/blob/4.1-dev/libraries/src/Form/Field/CalendarField.php#L358-L361.

@Quy Quy mentioned this pull request Apr 7, 2022
@exstreme
Copy link

exstreme commented Apr 7, 2022

I have tested this item 🔴 unsuccessfully on ce908fe

Deprecated: Function strftime() is deprecated in C:\OpenServer\domains\joomla.test\libraries\src\Form\Field\CalendarField.php on line 285

But I can it reproduce only in custom extension https://github.com/exstreme/Jcomments-4/releases by editing any comment


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

@joomdonation
Copy link
Contributor Author

@exstreme As mentioned, this PR only fixes the issue for calendar form fields use in Joomla core (which has translateformat="true" in the field definition). For calendar form fields use by third party extension, more change is needed, one possible solution could be #37373 (comment) . However, that needs to be a new PR for 4.2. For this PR, I only made small change, not introduce a new thing so that it could be merged into 4.1.

@exstreme
Copy link

exstreme commented Apr 8, 2022

I have tested this item ✅ successfully on ce908fe


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

@richard67 richard67 changed the title [4.1] Fix Deprecated warning for Calendar form field use in core [4.1] Fix Deprecated warning for Calendar Form Field Apr 8, 2022
@richard67 richard67 added PHP 8 and removed PHP 8.x PHP 8.x deprecated issues labels Apr 8, 2022
@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 8, 2022
@richard67 richard67 added PHP 8.x PHP 8.x deprecated issues and removed PHP 8 labels Apr 8, 2022
@richard67 richard67 added this to the Joomla 4.1.3 milestone Apr 8, 2022
@bembelimen
Copy link
Contributor

Puh, I see what you did there and it's the most painless solution without rewriting everything I guess but as you stated also not a full solution.
So could you make a remark in the code (or make a deprecation PR for 4.2?), that is has to be changed in the future and create a 5.0 issue (release blocker?) that this has to be fixed.

@joomdonation
Copy link
Contributor Author

@bembelimen I also made a PR #37376 for 4.2 which could be the full solution. The purpose of this PR is prevent warnings for Calendar fields use in Joomla core and I think it should be merged for 4.1

@rdeutz rdeutz merged commit ad73297 into joomla:4.1-dev Apr 20, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 20, 2022
@joomdonation joomdonation deleted the patch-6 branch April 20, 2022 10:24
iamrobert added a commit to iamrobert/flexicontent-cck that referenced this pull request Jan 6, 2024
Deprecated: Function strftime() is deprecated and comes up in PHP8.1. Its a problem here:

joomla/joomla-cms#37373
micker pushed a commit to FLEXIcontent/flexicontent-cck that referenced this pull request Jan 7, 2024
* Update item.xml

Deprecated: Function strftime() is deprecated and comes up in PHP8.1. Its a problem here:

joomla/joomla-cms#37373

* Update user.xml

Update user.xml to prevent depreciated notice
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.

7 participants