? ? Pending

User tests: Successful: Unsuccessful:

avatar PhilETaylor
PhilETaylor
17 Aug 2021

Pull Request for Issue #35204 replaces #35207

Summary of Changes

Correctly re-translate the date/time string in the calendar based on language file

Language packs use H M S which is passed the JS in date-helper.js and not H i s like PHP does so needs extra cleaning up to reuse the DATE_FORMAT_CALENDAR_DATETIME="%m-%d-%Y %H:%M:%S" value in PHP

Testing Instructions

Install Joomla 4 with US language set as default
Try to create an article manually specifying a start publishing date and try to save

Actual result BEFORE applying this Pull Request

Screenshot 2021-08-17 at 22 52 52

Expected result AFTER applying this Pull Request

Article Saves

Documentation Changes Required

avatar PhilETaylor PhilETaylor - open - 17 Aug 2021
avatar PhilETaylor PhilETaylor - change - 17 Aug 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 17 Aug 2021
Category Libraries
avatar PhilETaylor PhilETaylor - change - 17 Aug 2021
Title
Correctly format incoming JS date to PHP/SQL date
[4] Correctly format incoming localised JS calendar date to PHP/SQL date
avatar PhilETaylor PhilETaylor - edited - 17 Aug 2021
avatar PhilETaylor
PhilETaylor - comment - 17 Aug 2021

@wilsonge If you consider the fact that no Joomla site using a non GB date string as its DATE_FORMAT_CALENDAR_DATETIME is currently able to create content/articles when specifying a date as the start/end publishing/featured as a release blocker (which I would say it is, not being able to create content in any locale other than some) then this needs a release blocker label :)

avatar PhilETaylor PhilETaylor - change - 17 Aug 2021
Labels Added: ?
avatar wilsonge
wilsonge - comment - 17 Aug 2021

@Fedik also would be good to get a review from you here. I didn't think the refactoring we did should have affected like this?

avatar particthistle
particthistle - comment - 17 Aug 2021

en-AU and en-US 4.0.0.4 versions uploaded to correct some other strings.

New packs will fix #35204 #35205.

As the language string format is a side effect not the primary cause here, I'm leaving date strings as

  • en-US: %m-%d-%Y %H:%M:%S
  • en-AU: %d-%m-%Y %H:%M:%S
avatar infograf768
infograf768 - comment - 18 Aug 2021

FYI:
German and French packs use this without any problem

; Date format
DATE_FORMAT_CALENDAR_DATE="%d-%m-%Y"
DATE_FORMAT_CALENDAR_DATETIME="%d-%m-%Y %H:%M:%S"
avatar brianteeman
brianteeman - comment - 18 Aug 2021

@infograf768 which is why the need for this fix in core seems odd to me

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

It simply cannot be true that those work as there is no code to convert the translated date format to a useable sql format. I'll test them myself when out of bed from my 2 hour sleep.

avatar brianteeman
brianteeman - comment - 18 Aug 2021

As there are sites live on the web in french and j4 and I just personally installed a site only in french and created articles with start date, end date, featured etc then it simply must be you needing more sleep

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

Quite frankly I would have got more sleep if Joomla 4 was not so much of a shit show.

avatar brianteeman
brianteeman - comment - 18 Aug 2021

please delete unnecessary comment

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

Same could be said for most of your comments.

avatar infograf768
infograf768 - comment - 18 Aug 2021

It could just be a feeling, but it looks to me that the important part to format correctly is %H:%M:%S (no i)

%d-%m-%Y can be in any order depending on the language i.e. "%Y-%m-%d for en-GB.

avatar infograf768
infograf768 - comment - 18 Aug 2021

Example of new article for French (frontend)
Screen Shot 2021-08-18 at 09 48 49

avatar Fedik
Fedik - comment - 18 Aug 2021

I didn't think the refactoring we did should have affected like this?

well, need to debug,
the calendar js should convert the date to SQL format, before submit (@dgrammatiko right?)

JoomlaCalendar.prototype.setAltValue = function() {

I do not see any difference between j3 and j4 here on quick look.

Suggested patch is incorrect.

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

You are all wrong.

avatar Fedik
Fedik - comment - 18 Aug 2021

You are all wrong.

Except me ?

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

The Calendar field should be able to cope with ANY translated string. ANY date format. Currently It cannot - there is simply no PHP code to support this.

If you actually understood PHP then you would see why PHP can handle the french date but cannot handle the US format.

PHP DateTime constructor will accept most crap you give it.

You can run this test without Joomla here: https://3v4l.org/mY8oF and you will see it FAILS on the US date

//FR - DATE_FORMAT_CALENDAR_DATETIME="%d-%m-%Y %H:%M:%S"
 new DateTime('19-08-2021 08:41:23');

//US - DATE_FORMAT_CALENDAR_DATETIME="%m-%d-%Y %H:%M:%S"
 new DateTime('08-19-2021 08:41:23');

Here is the important part of the PHP Documentation - actually in a comment, not the docs.

* Date values separated by slash are assumed to be in American order: m/d/y
* Date values separated by dash are assumed to be in European order: d-m-y
* Exact formats for date/time strings can be injected with ::createFromFormat()

Test: https://3v4l.org/cjiqI works and confirms this.

So if you want "The Calendar field should be able to cope with ANY translated string" then you need PHP code (or JS code if you really want to) to translate the "any formatted date string" to a format PHP DateTime can use.

At the moment Joomla is passing the translated date direct to php DateTime __construct - its by pure luck that the french has hyphens in its translated format. If it had slashes then it would be interpreted differently. If it had been a custom format it would have failed.

If I want to translate Joomla into the new "TeemanTango" language, with a date format of

DATE_FORMAT_CALENDAR_DATETIME="%d...%m...%Y %H:%M:%S"

Then currently Joomla would fail the same way as the US does.

Screenshot 2021-08-18 at 08 56 38

However after applying this PR - Joomla can handle that custom translated date correctly.

I should not have to justify each and every PR, if you cannot be bothered to test the PR then please just don't comment.

This PR as proposed fixes the reported issue with US formatted dates and improves Joomla so that ALL FORMATS of dates - even crappy %d...%m...%Y %H:%M:%S" work in Joomla

Screenshot 2021-08-18 at 08 59 22

avatar Fedik
Fedik - comment - 18 Aug 2021

The Calendar field should be able to cope with ANY translated string. ANY date format. Currently It cannot - there is simply no PHP code to support this.

I am talking about JavaScript. Take a rest ;)

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

I am talking about JavaScript. Take a rest ;)

The same is true. The Calendar field should be able to cope with ANY translated string. ANY date format. Currently It cannot - there is simply no PHP ///or Javascript Code/// to support this.

This PR fixes the reported issue with a simple 2 line fix. Take it or leave it. But it works and should be tested.

avatar Fedik
Fedik - comment - 18 Aug 2021

PHP should receive 0000-00-00 00:00:00 (with real numbers) from <input/>, (at least how it in j3, if I right).
Need to debug why it not happen.

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

Need to debug why it not happen.

Because there is no code written to support changing the Calendarfield input in date-helper.js/min.js to that format when the form is submitted.

If you want to write a competing PR in the Javascript layer (in date-helper.js) then go ahead. Im done.

avatar Fedik
Fedik - comment - 18 Aug 2021

Make a break

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

Make a break

Or time to report/fix ANTOHER release blocker - like, when using ANY language, you cannot set the featured start/finish dates... they just disappear!

avatar dgrammatiko
dgrammatiko - comment - 18 Aug 2021

AFAIR the calendar field has a value (SQL formatted) and an data-value-alt (might be named differently). The second is the localized (translated) string that’s displayed in the calendar input. This code didn’t change in any iteration of j4.0 beta alpha afaik

avatar Fedik
Fedik - comment - 18 Aug 2021

This code didn’t change in any iteration of j4.0 beta alpha afaik

Yeah, that why I wonder.
But somewhere something definitely changes, because people say it works in 3.x.

@dgrammatiko Can you correct me:
data-alt-value - the value use "Custom" dateFormat (from lang constant), but it always "gregorian"
data-local-value - the value use "Custom" dateFormat (from lang constant), but use local calendar "non gregorian" or "gregorian" depend from settings.

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

Well its obvious no one wants this fixed in the PHP level (as no one has tested this PR and is only talking about the JS layer) therefore Im closing this PR as it has no chance of being merged.

I look forward to testing the JS solution to this release blocker.

avatar PhilETaylor PhilETaylor - change - 18 Aug 2021
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2021-08-18 10:09:13
Closed_By PhilETaylor
Labels Added: ? ?
Removed: ?
avatar PhilETaylor PhilETaylor - close - 18 Aug 2021
avatar dgrammatiko
dgrammatiko - comment - 18 Aug 2021

I look forward to testing the JS solution to this release blocker.

@PhilETaylor please reopen, the PHP side is really strict thus the problem.

@Fedik that seems correct but I have to actually read the code to check what the calendar is actually doing there.

The problem with the calendar is fundamentally the architectural design failure: the dates should ALWAYS BE UTC & SQL format. Less transforms and only one point of failure (server timezone). Unfortunately kinda impossible to do it without a huge B/C break...

avatar Fedik
Fedik - comment - 18 Aug 2021

The problem with the calendar is fundamentally the architectural design failure: the dates should ALWAYS BE UTC & SQL format.

This part is working correct, we have filters for that. Display in User time zone, and store in UTC.
However, the formatting is failing somewhere.

What @PhilETaylor suggested here it is a "work around".

avatar PhilETaylor
PhilETaylor - comment - 18 Aug 2021

However, the formatting is failing somewhere.

What is "failing" is that the form is submitting the "pretty translated date formats" and not a rock solid SQL/PHP format.

The form is correctly sending the value of the inputbox

I guess in JS you need to trigger a onFormSubmit to replace that value with your pseudo value from the data attributes. But this relies on JS.

avatar Fedik
Fedik - comment - 18 Aug 2021

I guess in JS you need to trigger a onFormSubmit to replace that value with your pseudo value from the data attributes. But this relies on JS.

If you have read my previous comments, I already have showed the part that responsible for this exact behavior ?

But strange part here, that it is not changed since j3, there exactly the same.

avatar dgrammatiko
dgrammatiko - comment - 18 Aug 2021

But strange part here, that it is not changed since j3, there exactly the same.

If I read correctly the code both the data parsing in PHP and in Javascript have no fallbacks for malformed data, code always assumes that data is valid date string...

avatar Bakual
Bakual - comment - 20 Aug 2021

I haven't read every comment, so this may have been written already. For reference my original PR: #12102
There are four formats here which play together:

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

One pair is using strftime format (it's used also by the JS code) and the other is using PHP Date format. With both set and used correctly, translation of dates should work seamless. If they don't match, it's broken. But it's a bug in the language pack then.

Add a Comment

Login with GitHub to post a comment