PhocaCz commented 2 years ago

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 )

chmst commented 2 years ago

37376

Fedik commented 2 years ago

Closing as duplicate

joomdonation commented 2 years ago

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

Fedik commented 2 years ago

Okay :)

kosh2323 commented 2 years ago

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

garkell commented 2 years ago

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));

kosh2323 commented 2 years ago

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.

Fedik commented 2 years ago

@garkell strftime and date formats is incompatible.

kosh2323 commented 2 years ago

In version 4.2.3, the same warnings remained.

ceford commented 2 years ago

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.}

kosh2323 commented 2 years ago

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.

Hackwar commented 1 year ago

@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.

kosh2323 commented 1 year ago

@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....

joomdonation commented 1 year ago

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 commented 1 year ago

@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.

roland-d commented 1 year ago

@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.

joomdonation commented 1 year ago

@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?

roland-d commented 1 year ago

@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?

Chaosxmk commented 1 year ago

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.

joomdonation commented 1 year ago

@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 ?

roland-d commented 1 year ago

@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 ;)

joomdonation commented 1 year ago

@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 :

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
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.

roland-d commented 1 year ago

@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.

joomdonation commented 1 year ago

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.

roland-d commented 1 year ago

@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.

joomdonation commented 1 year ago

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.

shstaples commented 1 year ago

@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

PhocaCz commented 1 year ago

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

exstreme commented 1 year ago

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

joomla / joomla-cms

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

37376