? ? Success

User tests: Successful: Unsuccessful:

avatar Bakual
Bakual
22 Nov 2016

PR #11138 removed the new "translateFormat" feature of the calendar formfield which was introduced with #12102

Summary of Changes

Brings back the same code.

Testing Instructions

Testing instructions from original PR apply here as well:

  1. Apply PR
  2. Add the new language strings to your second language you have installed. Eg for de-DE add this to both de-DE.ini files (frontend and backend):
DATE_FORMAT_CALENDAR_DATE="%d.%m.%Y"
DATE_FORMAT_CALENDAR_DATETIME="%d.%m.%Y %H:%M:%S"
DATE_FORMAT_FILTER_DATETIME="d.m.Y H:i:s"
DATE_FORMAT_FILTER_DATE="d.m.Y"
  1. Check backend article edits and watch the format changing from YYYY-MM-DD to DD.MM.YYYY if you change the language.
  2. Active the user - profile plugin and edit your profil in frontend. Check the birthday field and watch the change in format. Also this field should not show the time.

Note about the strings

avatar Bakual Bakual - open - 22 Nov 2016
avatar Bakual Bakual - change - 22 Nov 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 22 Nov 2016
Category Administration com_content Libraries
avatar Bakual Bakual - change - 22 Nov 2016
Labels Added: ?
avatar Bakual Bakual - change - 22 Nov 2016
Milestone Added:
avatar Bakual
Bakual - comment - 22 Nov 2016

Actually, it looks like there is another bug in the new calendar. It doesn't respect the format value passed by XML or this translateFormat feature.
Current staging, the "created" field in an article (ignore the missing time for now)
staging
If we add format="%d.%m.%Y in the XML
format
With this PR
translateformat

As you see, this PR actually would work fine, but the calendar breaks as soon as non-english formats are given. Which worked fine with the old calendar.
As far as I remember that was fixed in the PR at some time and seems to have broken again. @dgt41 can you please have a look at this?

avatar infograf768
infograf768 - comment - 22 Nov 2016

This PR is exactly what I did locally. One can enter the localized format, but it breaks as soon as saving.

avatar Bakual
Bakual - comment - 22 Nov 2016

There is even another thing. The readonly calendar field (eg for modified) seems to change the value back to YYYY-MM-DD before sending the form, which again breaks the code if transFormat is active.

avatar dgt41
dgt41 - comment - 22 Nov 2016

Will look at this one, later on or tomorrow

avatar Bakual Bakual - change - 23 Nov 2016
The description was changed
avatar Bakual Bakual - edited - 23 Nov 2016
avatar dgt41
dgt41 - comment - 24 Nov 2016

I am a bit confused, here are my mods:
screen shot 2016-11-24 at 15 04 36
screen shot 2016-11-24 at 15 04 23
screen shot 2016-11-24 at 15 04 02

And here's what I get:
screen shot 2016-11-24 at 15 03 44

screen shot 2016-11-24 at 15 07 02

So the cal is returning the date with the correct format but php somehow doesn't accept it (the m/d/y is scrambled)
@Bakual maybe there is something more here that's lost between the staging/3.7/staging process ?
It seems that the form when loading is always setting the wrong format:
screen shot 2016-11-24 at 15 42 19

This should be 24.11.2016 not with dashes

avatar Bakual
Bakual - comment - 24 Nov 2016

In the old calendar, the format was applied in JHtml::calendar. It looks like you bypass this method now and thus the format isn't applied anymore. So you need to apply it in the layout I guess.

Or you use again JHtml::calendar and keep the logic in one place. Which I personally would prefer.

avatar dgt41
dgt41 - comment - 25 Nov 2016

@Bakual @infograf768 so I found a solution here, but let me explain what was failing first:
This code executes after the change of the format:

		// If a known filter is given use it.
		switch (strtoupper($this->filter))
		{
			case 'SERVER_UTC':
				// Convert a date to UTC based on the server timezone.
				if ($this->value && $this->value != JFactory::getDbo()->getNullDate())
				{
					// Get a date object based on the correct timezone.
					$date = JFactory::getDate($this->value, 'UTC');
					$date->setTimezone(new DateTimeZone($config->get('offset')));

					// Transform the date string.
					$this->value = $date->format('Y-m-d H:i:s', true, false);
				}
				break;
			case 'USER_UTC':
				// Convert a date to UTC based on the user timezone.
				if ($this->value && $this->value != JFactory::getDbo()->getNullDate())
				{
					// Get a date object based on the correct timezone.
					$date = JFactory::getDate($this->value, 'UTC');
					$date->setTimezone(new DateTimeZone($user->getParam('timezone', $config->get('offset'))));

					// Transform the date string.
					$this->value = $date->format('Y-m-d H:i:s', true, false);
				}
				break;
		}

Observe that the format is HARDCODED and in Thomas example is different thus failing...
Solution:
We need to translate strftime format to php date format, e.g. https://gist.github.com/mcaskill/02636e5970be1bb22270

Quick edit to verify that this works, add after line 190 (libraries/joomla/form/fields/calendar.php):

	/**
	 * Convert date/time format between `date()` and `strftime()`
	 *
	 * Timezone conversion is done for Unix. Windows users must exchange %z and %Z.
	 *
	 * Unsupported date formats : S, n, t, L, B, G, u, e, I, P, Z, c, r
	 * Unsupported strftime formats : %U, %W, %C, %g, %r, %R, %T, %X, %c, %D, %F, %x
	 *
	 * @example Convert `%A, %B %e, %Y, %l:%M %P` to `l, F j, Y, g:i a`, and vice versa for "Saturday, March 10, 2001, 5:16 pm"
	 * @link http://php.net/manual/en/function.strftime.php#96424
	 *
	 * @param string $format The format to parse.
	 * @param string $syntax The format's syntax. Either 'strf' for `strtime()` or 'date' for `date()`.
	 * @return bool|string Returns a string formatted according $syntax using the given $format or `false`.
	 */
	public static function date_format_to( $format, $syntax )
	{
		// http://php.net/manual/en/function.strftime.php
		$strf_syntax = [
			// Day - no strf eq : S (created one called %O)
			'%O', '%d', '%a', '%e', '%A', '%u', '%w', '%j',
			// Week - no date eq : %U, %W
			'%V',
			// Month - no strf eq : n, t
			'%B', '%m', '%b', '%-m',
			// Year - no strf eq : L; no date eq : %C, %g
			'%G', '%Y', '%y',
			// Time - no strf eq : B, G, u; no date eq : %r, %R, %T, %X
			'%P', '%p', '%l', '%I', '%H', '%M', '%S',
			// Timezone - no strf eq : e, I, P, Z
			'%z', '%Z',
			// Full Date / Time - no strf eq : c, r; no date eq : %c, %D, %F, %x
			'%s'
		];
		// http://php.net/manual/en/function.date.php
		$date_syntax = [
			'S', 'd', 'D', 'j', 'l', 'N', 'w', 'z',
			'W',
			'F', 'm', 'M', 'n',
			'o', 'Y', 'y',
			'a', 'A', 'g', 'h', 'H', 'i', 's',
			'O', 'T',
			'U'
		];
		switch ( $syntax ) {
			case 'date':
				$from = $strf_syntax;
				$to   = $date_syntax;
				break;
			case 'strf':
				$from = $date_syntax;
				$to   = $strf_syntax;
				break;
			default:
				return false;
		}
		$pattern = array_map(
			function ( $s ) {
				return '/(?<!\\\\|\%)' . $s . '/';
			},
			$from
		);
		return preg_replace( $pattern, $to, $format );
	}

And replace

		// If a known filter is given use it.
		switch (strtoupper($this->filter))
		{
			case 'SERVER_UTC':
				// Convert a date to UTC based on the server timezone.
				if ($this->value && $this->value != JFactory::getDbo()->getNullDate())
				{
					// Get a date object based on the correct timezone.
					$date = JFactory::getDate($this->value, 'UTC');
					$date->setTimezone(new DateTimeZone($config->get('offset')));

					// Transform the date string.
					$this->value = $date->format(self::date_format_to('Y-m-d H:i:s', 'date'), true, false);
				}
				break;
			case 'USER_UTC':
				// Convert a date to UTC based on the user timezone.
				if ($this->value && $this->value != JFactory::getDbo()->getNullDate())
				{
					// Get a date object based on the correct timezone.
					$date = JFactory::getDate($this->value, 'UTC');
					$date->setTimezone(new DateTimeZone($user->getParam('timezone', $config->get('offset'))));

					// Transform the date string.
					$this->value = $date->format('Y-m-d H:i:s', true, false);
				}
				break;
		}

With

		// If a known filter is given use it.
		switch (strtoupper($this->filter))
		{
			case 'SERVER_UTC':
				// Convert a date to UTC based on the server timezone.
				if ($this->value && $this->value != JFactory::getDbo()->getNullDate())
				{
					// Get a date object based on the correct timezone.
					$date = JFactory::getDate($this->value, 'UTC');
					$date->setTimezone(new DateTimeZone($config->get('offset')));

					// Transform the date string.
					$this->value = $date->format(self::date_format_to($this->format, 'date'), true, false);
				}
				break;
			case 'USER_UTC':
				// Convert a date to UTC based on the user timezone.
				if ($this->value && $this->value != JFactory::getDbo()->getNullDate())
				{
					// Get a date object based on the correct timezone.
					$date = JFactory::getDate($this->value, 'UTC');
					$date->setTimezone(new DateTimeZone($user->getParam('timezone', $config->get('offset'))));

					// Transform the date string.
					$this->value = $date->format(self::date_format_to($this->format, 'date'), true, false);
				}
				break;
		}
avatar dgt41
dgt41 - comment - 25 Nov 2016

Proof:
screen shot 2016-11-25 at 20 52 41

avatar Bakual
Bakual - comment - 25 Nov 2016

We need to translate strftime format to php date format, e.g.

NO! That has been tried several times already and always failed or was ugly code.
But most importantly it is not the cause of the issue. The converting you found was always done on the raw value. The formatting came into play at a later stage. See https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/html/html.php#L1031-L1037
Since you're no longer using that JHtml method in your new calendar formfield (the old one used it), it now is missing on this and the format is never applied anymore. That is the issue as I said. Ideally the formfield would still call JHtml::calendar, which would also still allow overriding (B/C!) if needed.

avatar dgt41
dgt41 - comment - 25 Nov 2016

@Bakual so what's missing from the field is

		// Format value when not nulldate ('0000-00-00 00:00:00'), otherwise blank it as it would result in 1970-01-01.
		if ($value && $value != JFactory::getDbo()->getNullDate() && strtotime($value) !== false)
		{
			$tz = date_default_timezone_get();
			date_default_timezone_set('UTC');
			$inputvalue = strftime($format, strtotime($value));
			date_default_timezone_set($tz);
		}
		else
		{
			$inputvalue = '';
		}

I don't see any differences (except this one) between the old and new way. Also why should a form field rely on a JHtml function?

avatar Bakual
Bakual - comment - 25 Nov 2016

so what's missing from the field is

Yes, I think that is missing for the display part. At least I didn't see where you would apply the format in the field.

Also why should a form field rely on a JHtml function?

Main argument is to avoid duplicated code. Since we have two ways (formfield and JHtml call) of adding a calendar to a site, it makes sense to have the main part of the code in one place and only the formfield specific code in the formfield.
One could do that in the JLayout but I think it already has to much logic in it for a layout file.

avatar dgt41
dgt41 - comment - 25 Nov 2016

OK, I confirm that part is missing from the form field... ?

avatar dgt41
dgt41 - comment - 25 Nov 2016
avatar Bakual
Bakual - comment - 25 Nov 2016

Merged, thanks! ?

avatar dgt41
dgt41 - comment - 25 Nov 2016

I have tested this item successfully on 6eecb9c


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

avatar dgt41 dgt41 - test_item - 25 Nov 2016 - Tested successfully
avatar infograf768
infograf768 - comment - 27 Nov 2016

Works fine here.

Note: we have more than 70 places in core xml where format is still format="%Y-%m-%d %H:%M:%S"
Would not it be sensible to change all these?

At this time, only article.xml and profile.xml use it.
It would really be confusing for users to enter dates with their language translateformat in places where it is not implemented.

Just did it for Finish Publishing for a module:
I entered the French format 10-09-2018 and I got 2033-05-24 00:00:00 in the field.


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

avatar infograf768 infograf768 - test_item - 27 Nov 2016 - Not tested
avatar infograf768
infograf768 - comment - 27 Nov 2016

I have tested this item successfully on 6eecb9c


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

avatar infograf768 infograf768 - test_item - 27 Nov 2016 - Tested successfully
avatar Bakual
Bakual - comment - 27 Nov 2016

I can do it for the other fields if this is working fine here. It will be an easy PR then. ?

avatar infograf768
infograf768 - comment - 27 Nov 2016

@Bakual
The issue I see is that it is indeed easy to modify the core xmls (let's not forget weblinks), but it would have no impact on 3pd.
The ideal evidently would be to keep both hardcoded format and translateformat to get some sort of translateformat default for any calendar field

avatar Bakual
Bakual - comment - 27 Nov 2016

The issue I see is that it is indeed easy to modify the core xmls (let's not forget weblinks), but it would have no impact on 3pd.

That's true. They will need to adjust their XML as well. However they will only be able to do it if they raise the minimum requirement to J3.7. It actually works by leaving the format attribute there: Pre J3.7 it will use the specified format and with J3.7 the format attribute will be ignored and the translated one will be used. So it's possible to support that feature and at the same time don't break in older versions.

The ideal evidently would be to keep both hardcoded format and translateformat to get some sort of translateformat default for any calendar field

We can make it the default behaviour with J4.

avatar joomla-cms-bot joomla-cms-bot - change - 27 Nov 2016
Category Administration com_content Libraries Administration com_content Language & Strings Libraries
avatar infograf768
infograf768 - comment - 28 Nov 2016

I have tested this item successfully on 985d931


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

avatar infograf768 infograf768 - test_item - 28 Nov 2016 - Tested successfully
avatar dgt41
dgt41 - comment - 9 Dec 2016

I have tested this item successfully on 985d931


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

avatar dgt41 dgt41 - test_item - 9 Dec 2016 - Tested successfully
avatar dgt41 dgt41 - change - 9 Dec 2016
Status Pending Ready to Commit
avatar jeckodevelopment jeckodevelopment - change - 9 Dec 2016
Labels
avatar jeckodevelopment
jeckodevelopment - comment - 9 Dec 2016

RTC


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

avatar rdeutz rdeutz - change - 9 Dec 2016
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-12-09 22:33:20
Closed_By rdeutz
Labels Added: ?
avatar rdeutz rdeutz - close - 9 Dec 2016
avatar rdeutz rdeutz - merge - 9 Dec 2016
avatar rdeutz rdeutz - reference | c0668cd - 9 Dec 16
avatar rdeutz rdeutz - merge - 9 Dec 2016
avatar rdeutz rdeutz - close - 9 Dec 2016
avatar Bakual Bakual - head_ref_deleted - 9 Dec 2016
avatar cpfeifer cpfeifer - reference | fb3e6ba - 22 Dec 16

Add a Comment

Login with GitHub to post a comment