joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

[J4][PHP8.1] HTMLHelper, method calendar - strftime - deprecated

Open PhocaCz opened this issue 2 years ago • 8 comments

Hi,

in Joomla 4.1.5, there is deprecated code for PHP 8.1 (strftime function):

libraries/src/HTML/HTMLHelper.php line cca 1199: $inputvalue = strftime($format, strtotime($value));

( https://www.php.net/manual/en/function.strftime.php )

PhocaCz avatar Jul 14 '22 22:07 PhocaCz

#37376

chmst avatar Jul 15 '22 06:07 chmst

Closing as duplicate

Fedik avatar Jul 15 '22 07:07 Fedik

Actually, that PR is not related to the issue. The PR propose a fix for Joomla Calendar form field, while the issue described here is for calendar method in HtmlHelper https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/HTML/HTMLHelper.php#L1029

joomdonation avatar Jul 15 '22 08:07 joomdonation

Okay :)

Fedik avatar Jul 15 '22 09:07 Fedik

Yes, this problem occurs in all Joomla templates, although Joomla is updated regularly, but what is Deprecated today will soon stop working altogether. PHP is also being updated, that's what I see in my templates:

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

kosh2323 avatar Sep 17 '22 16:09 kosh2323

I tried this change and it seems to work however my limited knowledge maybe showing here?

libraries\src\Form\Field\CalendarField.php - line 273 changed - $this->value = strftime($this->format, strtotime($this->value)); to - $this->value = date($this->format, strtotime($this->value));

and in libraries\src\HTML\HTMLHelper.php - line 1076 changed - $inputvalue = strftime($format, strtotime($value)); to - $inputvalue = date($format, strtotime($value));

garkell avatar Sep 20 '22 04:09 garkell

I tried this change and it seems to work however my limited knowledge maybe showing here?

libraries\src\Form\Field\CalendarField.php - line 273 changed - $this->value = strftime($this->format, strtotime($this->value)); to - $this->value = date($this->format, strtotime($this->value));

and in libraries\src\HTML\HTMLHelper.php - line 1076 changed - $inputvalue = strftime($format, strtotime($value)); to - $inputvalue = date($format, strtotime($value));

Thanks for the advice, I'll try.

kosh2323 avatar Sep 20 '22 08:09 kosh2323

@garkell strftime and date formats is incompatible.

Fedik avatar Sep 20 '22 10:09 Fedik

In version 4.2.3, the same warnings remained.

kosh2323 avatar Sep 28 '22 13:09 kosh2323

In 4.2.3 strftime() calls occur in libraries/src/Form/Field/CalendarField.php on line 273 and libraries/src/HTML/HTMLHelper.php on line 1076 with a note on line 855.


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

ceford avatar Oct 06 '22 14:10 ceford

In 4.2.3 strftime() calls occur in

Well, why should we, ordinary users, look for this and try to fix it, we don't know the plans of the developers, maybe they will change something radically at all, and all these user methods will stop working.

kosh2323 avatar Oct 06 '22 16:10 kosh2323

@kosh2323 Because we "developers" are ordinary users as well. None of us is paid to do this work and we are all volunteers. So if you want to see some change, then you have to make that change.

Hackwar avatar Nov 17 '22 10:11 Hackwar

@kosh2323 Because we "developers" are ordinary users as well. None of us is paid to do this work and we are all volunteers. So if you want to see some change, then you have to make that change.

Hi, yes, I have no complaints, I understand, and I don't want to judge anyone, these are just facts and that's it....

kosh2323 avatar Nov 17 '22 10:11 kosh2323

I just looked at this issue. Similar to PR https://github.com/joomla/joomla-cms/pull/37376, I think we need to add a new parameter to calendar method https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/HTML/HTMLHelper.php#L1033 to allow passing an datetime format. Something like:

public static function calendar($value, $name, $id, $format = '%Y-%m-%d', $attribs = array(), $dateTimeFormat = '') {

}

Then in the code of that method, we will use $dateTimeFormat variable (if provided) to convert the $value to right format. The downside of this option is :

  • The sequence of method parameters are not logic. $format and $dateTimeFormat variables are related, so it should be defined next to each other, not separated by $attribs parameter like that.
  • We could not provide default value for $dateTimeFormat parameter, similar to $format parameter (due to backward compatible reason)

The other option is deprecated calendar method and introduce new method like

public static function datePicker($value, $name, $id, $format = '%Y-%m-%d', $dateTimeFormat = 'Y-m-d', $attribs = array()) {

}

Anyone has better idea to address this issue?

joomdonation avatar Nov 28 '22 14:11 joomdonation

@roland-d Could you please take a look at this issue? This method is widely use, so would be great if we could come up with a solution.

joomdonation avatar Jan 12 '23 02:01 joomdonation

@joomdonation The order of the arguments is not a big concern if you ask me. At some point in the future we can move to named arguments :)

There is a polyfill for strftime at https://github.com/alphp/strftime

According to the deprecation RFC the better alternative is IntlDateFormatter::format(). That would require some work to figure out which settings to use. Both instances where we use strftotime sets the timezone to UTC, so it may not be too hard.

Some psuedo code:

$fmt = new IntlDateFormatter(
    Factory::getLanguage()->getTag(),
    IntlDateFormatter::FULL,
    IntlDateFormatter::FULL,
    $this->config->get('offset'),
    IntlDateFormatter::GREGORIAN
);
echo 'Formatted output is ' . $fmt->format($value);

The easiest would be just to move to the polyfill but that makes us dependent on yet another library otherwise I think the IntlDateFormatter is worth checking.

roland-d avatar Jan 14 '23 10:01 roland-d

@roland-d I'm unsure using IntlDateFormatter could be the solution because it requires enabling intl extension.

Sometime ago, someone comes up with a solution to convert the format using by strftime to the format used by Datetime class. The code could be seen in this link https://github.com/joomla/joomla-cms/compare/4.2-dev...joomdonation:fix_html_helper_calendar?expand=1 (note the method strftimeFormatToDateFormat ) . Will you accept that magic format converter to solve this issue?

joomdonation avatar Jan 16 '23 07:01 joomdonation

@joomdonation Both the IntlDateFormatter as the polyfill require the intl extension. If we cannot rely on that or require that all we can do is some hackjob.

Your SO alternative looks good enough for me but we may want to deprecate it immediately so it can be removed in Joomla 5 to be replaced by the IntlDateFormatter`. What do you think of that?

roland-d avatar Jan 16 '23 19:01 roland-d

I don't think marking this as deprecated and removing it in Joomla 5 is good enough. As is, PHP is pushing my users to upgrade to 8.0 and 8.1, and for the sake of future proofing they all want 8.1 where this doesn't work.

I'm attempting to write out a block of code to fix this without making it backwards incompatible or breaking, but with all of the differences between strftime()'s accepted formats and other methods of handling date is creating a real pain.

I'm almost at a point of just suggesting a calendarNew() which just handles formats completely differently so that going forward code won't trigger this deprecation warning, and old sites can continue using this older deprecated function until it's just outright dropped.

Chaosxmk avatar Feb 06 '23 21:02 Chaosxmk

@roland-d @Chaosxmk I think the following approach would be the safest options

  • Do the automatic format conversion for the parameters which we know for sure that it works well. I would only do for the parameters which we are using on calendar form fields from core. These parameters include %Y, %m, %d, %H, %M, %S . That helps developers who use these popular format parameters not have to update their code. I would expect most of them just use these parameters.
  • Introduce new method for developers who use different format which we could not do automatic conversion. I think the new method could be:
public static function datepicker($value, $name, $id, $format = '%Y-%m-%d', $dateTimeFormat = 'Y-m-d', $attribs = array()) {

}

The new method would contain most of the code from our original calendar method with a small change. How do you think about it ?

joomdonation avatar Feb 08 '23 10:02 joomdonation

@joomdonation Would that mean that we change the core to use the datepicker() function and leave the calender() as-is? I don't really see any alternative than what you propose.

At some point I think that Joomla should just require the intl module if a user wants to be international ;)

roland-d avatar Feb 15 '23 13:02 roland-d

@roland-d I have to check it again but I don't think Joomla core use the calendar method. In Joomla core, we use Calendar form field. This calendar method from HtmlHelper is only used by third party extensions developer.

What I proposed is :

  1. Make change to calendar method and do automatic format conversion for most common used format parameters %Y, %m, %d, %H, %M, %S . It is a kind of hack but help developers who use these format do not have to update their code. It will continue working as how it is
  2. If we continue receiving report (mean someone uses a format not supported by the safe automatic conversion), we can add a new helper method datetimePicker. In this case, developers will have to pass both the format themself, no format conversion here.

joomdonation avatar Feb 15 '23 13:02 joomdonation

@joomdonation

It is a kind of hack but help developers who use these format do not have to update their code.

That is a good enough point. Especially if it is not used by core, the impact is minimal.

we can add a new helper method datetimePicker

That still is not a 100% match, I would be more inclined for Joomla to add a method that really fixes the issue. Yes, that would require the intl extension to be enabled in PHP. This is probably best for production department to decide on.

roland-d avatar Feb 15 '23 13:02 roland-d

That is a good enough point. Especially if it is not used by core, the impact is minimal.

Should I make a new PR for that so that we can look at it and decide ?

That still is not a 100% match, I would be more inclined for Joomla to add a method that really fixes the issue. Yes, that would require the intl extension to be enabled in PHP. This is probably best for production department to decide on

I don't know if we should require intl extension. Adding a new parameters make it works in the same way with other places we handle format for Calendar :

  • For Calendar form fields use in core (always has translateformat="true"), we need to define two formats (via language items) for it to works:

DATE_FORMAT_CALENDAR_DATETIME="%Y-%m-%d %H:%M:%S" DATE_FORMAT_FILTER_DATETIME="Y-m-d H:i:s"

  • For Calendar field (use by third party extension) in case translateFormat attribute is not defined, we also allow passing two format via two attributes:

format filterformat

So the new helper method, if we add, have two format parameters seems to be the right choice to me. But Yes, we can ask production department to decide.

joomdonation avatar Feb 15 '23 14:02 joomdonation

@joomdonation

Should I make a new PR for that so that we can look at it and decide ?

Let's do it.

As for the requirement of the intl extension, that will not happen in the J4 cycle anyway, if and only if they would want it, it start at 5 the earliest.

roland-d avatar Feb 15 '23 14:02 roland-d

OK. I made PR https://github.com/joomla/joomla-cms/pull/39869 try to solve this issue. Please help testing. If any one uses format parameters not supported by the automatic conversion code and could not migrate to supported formats, please report back the format you are using. If it could be converted, we will expand the supported format parameters.

I will close the current issue and will re-open it if needed.

joomdonation avatar Feb 16 '23 08:02 joomdonation

@joomdonation and @PhocaCz I still get Deprecated: Function strftime() is deprecated in C:\xampp\joomla4\libraries\src\Form\Field\CalendarField.php on line 322 with Joomla 4.3.3. and PHP 8.2

shstaples avatar Jul 30 '23 15:07 shstaples

Hi, yes, I get the same problem too in Joomla 4.3.3 (PHP8.2)

PhocaCz avatar Sep 07 '23 20:09 PhocaCz

5.0 - the problem again( But was fixed in https://github.com/joomla/joomla-cms/pull/37373

exstreme avatar Oct 19 '23 17:10 exstreme