? PBF PHP 8.x ? Pending

User tests: Successful: Unsuccessful:

avatar joomdonation
joomdonation
25 Mar 2022

Pull Request for Issue # .

Summary of Changes

This PR adds a new attribute filterformat to Calendar form field. This attribute, if provided, must provide equivalent format with format defined in format attribute of the field. It is used to solve deprecated warning issue when this field type is used on PHP 8.1 (and without translateformat="true" in it's definition)2

From what I see, all calendar form fields use on Joomla core have translateformat="true", so this PR will only affect third party extensions use this form field type.

Testing Instructions

  1. Use Joomla 4.2 on PHP 8.1
  2. Modify this block of xml https://github.com/joomla/joomla-cms/blob/4.2-dev/administrator/components/com_content/forms/article.xml#L90-L97 to :
<field
			name="created"
			type="calendar"
			label="COM_CONTENT_FIELD_CREATED_LABEL"
			format="%Y-%m-%d %H:%M:%S"
			filterformat="Y-m-d H:i:s"
			showtime="true"
			filter="user_utc"
		/>
  1. Edit article, look at Created Date field on Publishing tab

Actual result BEFORE applying this Pull Request

You see warnings like below

Deprecated: Function strftime() is deprecated in [ROOT]\libraries\src\Form\Field\CalendarField.php on line 273

Expected result AFTER applying this Pull Request

No warnings anymore. Article can be saved properly.

avatar joomdonation joomdonation - open - 25 Mar 2022
avatar joomdonation joomdonation - change - 25 Mar 2022
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 25 Mar 2022
Category Libraries
avatar joomdonation joomdonation - change - 25 Mar 2022
The description was changed
avatar joomdonation joomdonation - edited - 25 Mar 2022
avatar richard67
richard67 - comment - 26 Mar 2022

And just to be safe, we will only do this format calculation for PHP 8.1 only.

@joomdonation Any reason for that? I know, the PHP deprecated warning only happens on PHP 8.1+. But would it be unsafe to do this calculation also on lower versions so we have the same behaviour on all versions? I'd really like to avoid functional behaviour depending on the PHP version.

avatar joomdonation
joomdonation - comment - 26 Mar 2022

@richard67 It is just because I don't know how reliable the code for strftimeFormatToDateFormat is. As you can see from the comment, it is just a script from stackoverflow

Honestly, I'm unsure if we should do that format conversion automatically like that. I don't know which one is better:

  • Do the conversion like how we are doing on this PR
  • Or Do not do that and force developer to update extension, add filterFormat attribute to the form to make it fully compatible with 8.1
avatar laoneo
laoneo - comment - 19 May 2022

I second here the argument of @richard67, there should not be PHP dependent code. Personally I would introduce another attribute where the extension dev can pass a PHP date command instead of strftime. I expect some issues when we do some magic transformation.

avatar joomla-bot
joomla-bot - comment - 27 Jun 2022

This pull requests has been automatically converted to the PSR-12 coding standard.

avatar xjdfhbv
xjdfhbv - comment - 1 Nov 2022

What is happening with this problem please? At the moment there is no way to make a date field without warnings in PHP 8.1, without using translateformat="true". In this case both format and filterformat are ignored, and the format is DATE_FORMAT_CALENDAR_DATE. In English, "%Y-%m-%d, but in Russian (for example) "%d-%m-%Y". In other words there is currently no way to make a calendar field with a specific date format. Or am I missing something?


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

avatar joomdonation joomdonation - change - 28 Nov 2022
Labels Added: PBF PHP 8.x ? ?
avatar joomdonation joomdonation - change - 28 Nov 2022
The description was changed
avatar joomdonation joomdonation - edited - 28 Nov 2022
avatar joomdonation joomdonation - change - 28 Nov 2022
The description was changed
avatar joomdonation joomdonation - edited - 28 Nov 2022
avatar joomdonation
joomdonation - comment - 28 Nov 2022

I updated the PR to remove the magic format conversion. If someone uses Calendar form field without translateformat="true" and want to prevent deprecated warnings on PHP, he needs to add filterformat attribute with right value to his field definition.

@xjdfhbv Could you please help testing this PR ? If this is accepted, it will give a way to solve your problem.

avatar xjdfhbv
xjdfhbv - comment - 28 Nov 2022

I would be happy to test it but I have no idea how to download the modified code!

avatar joomdonation
joomdonation - comment - 28 Nov 2022

@xjdfhbv You can try to get the attached
CalendarField.zip
file, unzip it, upload the received file to libraries/src/Form/Field folder on your Joomla installation to test the change.

avatar xjdfhbv
xjdfhbv - comment - 28 Nov 2022

Well, it works with filterformat="Y-m-d", but when I tried filterformat="d-m-Y" and default="27-03-2023", the field appears with the value 2032-09-12.

avatar joomdonation
joomdonation - comment - 29 Nov 2022

Well, it works with filterformat="Y-m-d", but when I tried filterformat="d-m-Y" and default="27-03-2023", the field appears with the value 2032-09-12.

Could not re-procedure this issue myself. I can only guess that it happens because you do not add format="%d-%m-%Y" to the field ? Please note that in order to have it works, you need to have both format and filterformat defined. Something like:

<field
    name="test_calendar"
    type="calendar"
    label="Test Calendar Field"
    filterformat="d-m-Y"
    default="27-03-2023"
    showtime="false"
    filter="user_utc"
    />

Could you please try again and update us with the result? Thanks !

avatar xjdfhbv
xjdfhbv - comment - 29 Nov 2022

You are right, it works correctly if you specify a non-empty value for format. Even something like format="yes" is ok. It would be better if that wasn't necessary.

avatar joomdonation
joomdonation - comment - 29 Nov 2022

That format is used by the javascript code for calendar form field. I haven't read the code to understand exactly how it is used, but I think it is needed. I don't know how format="yes" works in this case, I would say that the format is needed so that when you select a date time from the popup, the value in the right format will be displayed in the text input. But I'm unsure.

avatar xjdfhbv
xjdfhbv - comment - 29 Nov 2022

Oh yes, you are right, an invalid format crashes the javascript. Both formats are needed, and they must be compatible, for example format="%m-%d-%Y" filterformat="m-d-Y". Ok, so with that understood, this is now a working solution.

avatar joomdonation
joomdonation - comment - 29 Nov 2022

Yes, that's right. Both format are needed and must be compatible with each other. format is used by javascript and filterformat is used by PHP for formatting the datetime.

Could you please go to https://issues.joomla.org/tracker/joomla-cms/37376 and mark your test result? Each PR needs 2 successful tests before It could be merged. Thanks !

avatar xjdfhbv xjdfhbv - test_item - 29 Nov 2022 - Tested successfully
avatar xjdfhbv
xjdfhbv - comment - 29 Nov 2022

I have tested this item successfully on 84fb229

Tested OK


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

avatar xjdfhbv
xjdfhbv - comment - 29 Nov 2022

Just for information, when setting a calendar field value dynamically using Form::bind() or Form::setValue() you always have to use Y-m-d format, even if a custom date format is specified with format and filterformat. If you try to dynamically set a date like "03-29-2023", a PHP error occurs.

This also applies when using translateformat="true". Most countries have DATE_FORMAT_CALENDAR_DATE="%Y-%m-%d" so there is no problem. But some countries (e.g. ru-RU) have DATE_FORMAT_CALENDAR_DATE="%d-%m-%Y", and in this case if you recycle the form post data straight back to re-initialise the form values, you get a PHP error. You always have to transform dates back to Y-m-d format to initialise calendar field values.

It's a bit messy, but this is how it works historically so we are stuck with it. At some time in the future, an easier solution will be to use format="%Y-%m-%d" filterformat="Y-m-d".

avatar carlitorweb carlitorweb - test_item - 12 Jan 2023 - Tested successfully
avatar carlitorweb
carlitorweb - comment - 12 Jan 2023

I have tested this item successfully on 84fb229


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

avatar carlitorweb
carlitorweb - comment - 12 Jan 2023

@joomdonation can this PR handle the Deprecated: Creation of dynamic property... messages this class have on php 8.2?

avatar joomdonation
joomdonation - comment - 12 Jan 2023

@carlitorweb We can do that in a separate PR later . There are still so many Deprecated messages in our code base for with PHP 8.2.

avatar joomdonation joomdonation - change - 12 Jan 2023
Status Pending Ready to Commit
avatar joomdonation
joomdonation - comment - 12 Jan 2023

RTC. Thanks all for testing and feedbacks !


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

avatar laoneo laoneo - change - 12 Jan 2023
Labels Added: ?
avatar laoneo
laoneo - comment - 12 Jan 2023

@carlitorweb there is already a pr for it #39605

avatar laoneo laoneo - change - 12 Jan 2023
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-01-12 08:11:45
Closed_By laoneo
Labels Removed: ?
avatar laoneo laoneo - close - 12 Jan 2023
avatar laoneo laoneo - merge - 12 Jan 2023
avatar laoneo
laoneo - comment - 12 Jan 2023

Thanks!

Add a Comment

Login with GitHub to post a comment