User tests: Successful: Unsuccessful:
Pull Request for ISSUE: #41265
Before:
$this->value = strftime($this->format, strtotime($this->value));
After
$this->value = (new DateTime($this->value))->format($this->format);
<field name="date_start" type="calendar" default="NOW"
label="MOD_MULTIMODULE_FIELD_FILTERDATESTART_LABEL" showon="date_is:on"
showtime="false" singleheader="true" />
This XML code add to XML file module.
Enabled all Errors and enable debug mode.
Change PHP to 8.2 in for this site.
And open configuration this module.
You see error on depricated for function strftime().
After change you not see this message.
But this field will be work.
Category | ⇒ | Libraries |
Status | New | ⇒ | Pending |
Labels |
Added:
PR-4.3-dev
|
This will not work, there were other PR somewhere around.
strftime
format https://www.php.net/manual/en/function.strftime.php and DateTime
format https://www.php.net/manual/en/datetime.format.php is incompatible.
Doesn't work - sorry
joomla-cms/libraries/src/HTML/HTMLHelper.php
Line 1251 in 5ce99f2
It was the PR that introduced this very dodgy helper function (for better or worse)
@wilsonge That helper method is not used by Calendar Form Field, so it is unrelated. It was created to address the deprecated warnings HTMLHelper calendar in method with with the most common DateTime format parameters (until we a better solution).
in site https://www.php.net/manual/en/function.strftime.php
Below is a method to support the old code.
https://github.com/alphp/strftime/blob/master/src/php-8.1-strftime.php
This function is very large.
$format = HTMLHelper::strftimeFormatToDateFormat($this->format);
$this->value = (new DateTime($this->value))->format($format);
In the corrected format, we use DateTime object. The function also converts the format to a new one for DateTime.
strftime
for the JDate
class.From https://github.com/alphp/strftime/blob/master/src/php-8.1-strftime.php
And let's call it obsolete - deprecated
before Joomla 5
It's funny, but it's the right decision.
@korenevskiy The right solution is adding filterformat to your field
<field name="date_start" type="calendar" default="NOW" filterformat="Y-m-d"
label="MOD_MULTIMODULE_FIELD_FILTERDATESTART_LABEL" showon="date_is:on"
showtime="false" singleheader="true" />
Then it will work well. No warnings anymore.
Then it will work well. No warnings anymore.
But where is backward compatibility? The CALENDAR field should work without the filters attribute.
But where is backward compatibility? The CALENDAR field should work without the filters attribute.
It was PHP, not Joomla! introduced that backward incompatible change and Joomla! already provided you the solution with the undocumented filterformat
attribute, you will just have to adapt your code to work with the changes introduced by PHP.
There is no reliable way to to convert all the format parameters supported by strftime to format parameters uses by DateTime class, so you cannot do the magic conversion (there are some format parameters in strftime has no equivalent format in DateTime class).
We need to ensure backward compatibility with older codes. At the same time, support PHP 8.2. At the same time, the format field should fully support backward compatibility, with full support strftime()
@joomdonation According to the PHP.net website. The function laid out with full support for the old function.
There's a link at the bottom of the page.
strftime
for the JDate
class.Labels |
Added:
bug
|
strftime()
method to the Date
classThis method works
$this->value = JDate::strftime($this->format, strtotime($this->value));
@korenevskiy do not waste your time, re-implementing of strftime
is not a correct way to go.
Of course that is wrong. For example, Joomla 5 should support PHP 8. Which means We can't release a release with an error popping up. There is another option to abandon the old format support. But it's also a backward compatibility violation. I wouldn't waste time if I didn't weigh the options. My option is very bad, BUT it is the only one that will allow you to maintain backward compatibility without errors. And already with the release of Joomla 6, this method can be removed.
There is another option, to pretend that nothing is happening. Meanwhile, 100% of Joomla 4 users on PHP 8.1 will display an error.
There is another option. We simply write the contents of this function into the CalendarField class. This will allow, as you say, not to create new functions. But even such a swollen Calendar FIeld is even worse than my version.
Each participant will believe that there will be a magic way of 3 lines that will solve all the problems, and will think that someone else will guess for sure. I read the contents of this function. And I will say that it will not be possible to replace it with 3 lines. There is absolutely no way to replace it.
Joomla 5 should support PHP 8. Which means We can't release a release with an error popping up
It works. It not an error, it a warning for developers.
A proper fix:
Introduce a new property datetime-format=""
The calendar class checks for this property, and when it exists then use it for value display and for value filter,
Othervise it continue to use strftime
.
However, this is require changes in calendar.js, new lang constant, and review all our forms.
If you feels brave enough, you are welcome to do it.
Otherwise, any hacking around strftime
is a waste of time.
Introduce a new property
datetime-format=""
The calendar class checks for this property, and when it exists then use it for value display and for value filter, Othervise it continue to usestrftime
.
@Fedik I think we have filterformat
attribute for that purpose already https://github.com/joomla/joomla-cms/blob/4.3-dev/libraries/src/Form/Field/CalendarField.php#L238 . Shouldn't this PR be closed ?
I think we have
filterformat
attribute for that purpose
That for filter
, I have suggest to use new attribute datetime-format=""
for display, this should use DateTime style format instead of strftime
.
But in general the Calendar need a bigger fix, see #41444
A deprecated strftime
is a last thing I would worry ?
This pull request has been automatically rebased to 4.4-dev.
We discussed this PR in the maintainers meeting and would suggest the following.
include the function with composer based on https://github.com/alphp/strftime
and detect if the function exists in the bootstrap.php, if it's missing load the polyfill and include the function.
As Harald said,
please use the composer dependency and make a PR against 4.4
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-03-27 19:20:55 |
Closed_By | ⇒ | bembelimen | |
Labels |
Added:
Updates Requested
PR-4.4-dev
Removed: PR-4.3-dev |
As Harald said, please use the composer dependency and make a PR against 4.4
Explain what it means to make a dependency on the compositor in our case.
@korenevskiy Will this solve issue #41265 ? If so, please link to the issue at the top of the description of this PR (
Pull Request for Issue #xxxxx
).