Problem/Motivation
Dates are being mangled by the PHP-intl module
Proposed resolution
Not sure -- symfony needs the php-intl module, but there's definitely problems here
Remaining tasks
Patch needed
User interface changes
None
API changes
None that I know of
Related Issues
http://drupal.org/node/1944636
Steps to reproduce:
* Check out a fresh Drupal 8 install via git
* Make sure the PHP "intl" module is installed on the web server (and that the web server was restarted)
* Create a new "Basic Page" node
* Click on the "Edit" tab
* Click on the "Authoring Information" accordian tab
* Notice that the date is set as "131313-MayMay-2121" and that the time is similarly garbled.
* Disable the php-intl module
* Restart the web server
* Notice that when you edit the node, the authoring information is normal
Note that this issue looks very similar to http://drupal.org/node/1944636, which was supposed to solve this a similar issue, but apparently doesn't fix this particular case.
Also note that I've tested this across several different browsers, and even gone so far as to fire up a packet sniffer to show that bad data really is coming from the web server.
Comments
Comment #1
BerdirClosed #1992562: invalid "authored on" date as a duplicate of this, raising priority.
Comment #2
Micha1111 CreditAttribution: Micha1111 commentedSame error here:
php 5.4.9 cgi-mode
intl server configuration attached
Ist not possible to save an edited article/page
Comment #3
Cameron Tod CreditAttribution: Cameron Tod commentedCannot reproduce error with the following config:
MAMP PHP 5.3.14
INTL version PECL-2.0.1
ICU version 51.1
ICU Data version 51.1
Head at 3cdae05
Comment #4
BerdirCan you try to run the DateTime tests, especially DateTimePlusIntl on your system and check if that's green?
Comment #5
Cameron Tod CreditAttribution: Cameron Tod commentedComment #6
jaredsmith CreditAttribution: jaredsmith commented@cam8001 -- do you have the intl module installed that comes with PHP, or just the PECL module? This problem seems to happen with the intl module built into PHP (at least with 5.3 and later). On Red Hat / Fedora / CentOS systems, it is provided as part of the "php-intl" package.
Comment #7
jaredsmith CreditAttribution: jaredsmith commentedTesting under Fedora, with PHP version 5.4.15 and MySQL version 5.5.31, from a fresh installation via git, 8.x branch at commit 08e824610c7c09e8bd0c023b756ee9712f649e12.
I have the following PHP packages installed:
and will be testing with and without:
Output of php -m is:
(obviously, this list also includes
intl
when I have that package installed.)First of all, the test *without* php-intl installed
Now with the php-intl package installed:
Comment #8
Cameron Tod CreditAttribution: Cameron Tod commented@jaredsmith - I was testing with the PECL module.
Comment #9
marcingy CreditAttribution: marcingy commentedI can't recreate this on php 5.4.7 (xampp) and intl
Comment #10
Micha1111 CreditAttribution: Micha1111 commentedNo solution ?
Can not test D8 without saving nodes !
Comment #11
rodsouto CreditAttribution: rodsouto commentedi have just installed drupal 8 and i get this error every time i try to create/edit a node, i'll try to dig in it tomorrow to see what's happening :)
Comment #12
catchBumping to critical.
Comment #13
jaredsmith CreditAttribution: jaredsmith commentedI'm still seeing this on a fresh Drupal 8 installation via git as well.
Comment #14
jaredsmith CreditAttribution: jaredsmith commentedJust for curiosity, I also tried this on PHP 5.5 (along with the 5.5 version of it's php-intl module):
I try to run the tests, but they're now failing:
Unfortunately, I still get the same results as I did with PHP 5.4.
Comment #15
ybabel CreditAttribution: ybabel commented+1 after fres install from GIT, unable to create nodes
Comment #16
g_miric CreditAttribution: g_miric commented+1 - unable to edit nodes
(I have this problem in Firefox, but not in Chrome)
Comment #17
PanchoSee also comments on #2006484-31: Remove dependency on datetime from node and following.
Comment #18
Damien Tournoud CreditAttribution: Damien Tournoud commentedLooking at the code, it seems we are missing a lot of those, everywhere. It's not completely obvious how to fix the field widget implementations, but this should at least go in the direction of fixing the form element.
Comment #19
webchickClosed #2057295: php-intl support breaks node add page as another dupe of this. It has some nice steps to reproduce.
Comment #20
dlu CreditAttribution: dlu commentedMoved to datetime.module per #2050763-16: Refine "base system" component (notes on refactoring of "base system" category here: https://docs.google.com/a/acquia.com/spreadsheet/ccc?key=0AusehVccVSq2dF...).
Comment #21
Micha1111 CreditAttribution: Micha1111 commentedSame error again
PHP Version 5.4.16 nmm1 (CGI)
INTL 1.1.0
ICU 4.8.1.1
ICU Data 4.8.1
Comment #22
derheap CreditAttribution: derheap commentedI spend some time inserting debug()-calls in the core.
I'm not able to tell why, but what is happing is more or less the following:
* When PHP-Itnl is present the correct(?) date and time from
system.date_format.html_time.yml
are loaded:Intl: 'H:mm:ss'
;* But the PHP-Intl formatter is sometimes(?) not executed;
* Somtimes as some dates are correct …
* Instead the parent formatter is called and this is the normal PHP-DateTime formatter;
* This formatter produces from
H:mm:ss
the plain wrong time18:0909:0606
and date.Maybe this helps.
I am just right now not able to write a test against it. It was my first peek into Drupal 8 core today. To much to learn for one day.
Comment #23
yuradoc CreditAttribution: yuradoc commentedLike I described here https://drupal.org/node/2087505 I have pb with creating nodes.
I have built-in intl module in PHP. I did try both PECL and module:
version 1.1.0
ICU version 4.8.1.1
ICU Data version 4.8.1
I use Drupal 8-dev from GIT - 12.09.2013.
I use "XAMPP for Linux 1.8.3-0". You could try it to reproduce pb.
Comment #24
yuradoc CreditAttribution: yuradoc commented1. datetime.module function datetime_datetime_form_process has callings of the DrupalDateTime format function without second parameter where we must pass in array with key format_string_type (simeth like described in patch #18). Another functions has such pbs too. It must be changed here, I think, or DrupalDateTime and DateTimePlus format functions must be changed.
2. DrupalDateTime format function:
this line $format = preg_replace(array('/\\\\\\\\/', '/(?<!\\\\)([AaeDlMTF])/'), array("\xEF\\\\\\\\\xFF", "\xEF\\\\\$1\$1\xFF"), $format);
cause sometimes pb with creating new IntlDateFormatter at DateTimePlus format function.
3. datetime.module function datetime_datetime_validate not worked
$date = $input['object']; is null when validate executed I think $date = $element['#default_value'] must be used instead (not sure). After changes it starts work.
Comment #25
derheap CreditAttribution: derheap commentedAll calls in DateTime.Module to date->format() are missing the second parameter for the format type.
So the formating defaults to PHP, which leeds to the error.
I will try to work out a patch, and first look again on the patch from #18.
Comment #26
yuradoc CreditAttribution: yuradoc commentedHello. Please try my patch, I'm not sure it's OK fully, but resolves a pb for me.
Comment #27
yuradoc CreditAttribution: yuradoc commentedComment #29
yuradoc CreditAttribution: yuradoc commentedI think that patch fails because of the using DrupalDateTime format function without passing format_string_type in tests definations.
Check it for a while without automatic testing. Tests may need to be modify.
Comment #30
yuradoc CreditAttribution: yuradoc commentedI unassign myself because of my busy. Very interesting if somebody tested patch from #26.
Comment #31
derheap CreditAttribution: derheap commentedThe patch seems to work for me. But the tests are failing also localy.
Two things about the patch:
1/ You added an extra parameter to
datetime_format_example()
. That changes the the API.datetime_format_example()
could/should calldatetime_default_format_type()
likedatetime_html5_format()
it does. That would make the patch smaller, we dont have to change that many function calls.2/ Why the changes to
lib/Drupal/Core/Datetime/DrupalDateTime.php
? I don’t think they are related to the error.I can’t fix the patch right now. Simpeltest is failing for me even with a fresh Drupal 8 repo right now.
Comment #32
yuradoc CreditAttribution: yuradoc commentedHello. I make changes like described at #31.1. It placed in new patch.
The 2. DrupalDateTime I changed for two reasons:
1. $settings must be passed to the parent::format. It's related to this pb, because format_string_type needs to be passed too.
2. see #24.2. I comment it while this pb wouldn't be resolved.
One note: after some testing I found that some values pass validation that are wrong. It seems to me that date_parse and other functions that parse date, time from string has a bug. In validation I use DateTime::modify and it seems that it works the same like date_parse. Example 2013-0u-15.
Comment #34
yuradoc CreditAttribution: yuradoc commentedSorry, found some bug. Reuploaded.
Comment #35
yuradoc CreditAttribution: yuradoc commentedComment #37
derheap CreditAttribution: derheap commentedI spend some time on this patch.
1/ The changes to
/core/lib/Drupal/Core/Datetime/DrupalDateTime.php
are breaking some of the tests.And for me the don't fix the bug.
2/ Most changes to
datetime.module
are working. Date and Time are displayed correct.3/ This chunk is working, but is breaking the tests.
4/ Today I have found the reason:
The
function form_type_datetime_value()
reads the values from the form and creates an date object, if the formats are ok.Thefore it calls
DrupalDateTime::createFromFormat()
which the calls the PHP Core DateTime::createFromFormat().And this functions works only with normal PHP format strings. So the date is not valid, when it is called with an INTL format string.
So: Either we create a working createFromFormat function, that works with INTL format strings or scrape the whole INTL stuff alltogether and use only PHP formats.
Comment #38
yuradoc CreditAttribution: yuradoc commentedHello. I made an experimental patch that contains modification of the createFromFormat.
I know that this function called by a lot of other places, so every changes must be done very careful. Please help to check it if it's work in most ways, maybe it requires some improvements.
Comment #39
yuradoc CreditAttribution: yuradoc commentedComment #41
yuradoc CreditAttribution: yuradoc commentedUpdated patch. Trying to remove some warnings.
Comment #43
yuradoc CreditAttribution: yuradoc commentedHello. I post patch again, sorry. Only now found how to use testing in WEB-interface of the D8 instalation.
Must be OK with passing.
Comment #44
yuradoc CreditAttribution: yuradoc commentedComment #46
yuradoc CreditAttribution: yuradoc commented#43: resolving-pb-with-date-format-output_and_validation-2000384-43.patch queued for re-testing.
Comment #47
yuradoc CreditAttribution: yuradoc commentedCool! Automatically testings finished successfully.
Now guys, please help us to review this patch - may you have some notices or ideas for improving of the DateTimePlus::createFromFormat.
Big thanks to derheap for #37 and other comments. Also thanks to Damien Tournoud for patch #18 that helped me too.
Comment #48
yuradoc CreditAttribution: yuradoc commentedHello. I removed some whitespaces, because git apply thrown them.
Comment #50
yuradoc CreditAttribution: yuradoc commented#48: resolving-pb-with-date-format-output_and_validation-2000384-48.patch queued for re-testing.
Comment #51
derheap CreditAttribution: derheap commentedI tested the patch with PHP-Intl enabled.
I can now create a node and change the date.
There are no more mangled date-examples.
I tested again without PHP-Intl enabled.
Also no errors.
So for me the bug is fixed.
@everbody
I am new to the issue queque. What I am supposed to do now?
Comment #52
yuradoc CreditAttribution: yuradoc commentedThanks for reporting derheap.
Does anybody has some results too?
I have the same question - what to do next with patch?
Comment #53
yuradoc CreditAttribution: yuradoc commentedI talked on IRC contribute chat. They said that users that made review of patch could set status to "reviewed & tested by the community".
Comment #54
jthorson CreditAttribution: jthorson commentedyuradoc:
The next step, now that the 'manual testing' step is complete, is for someone to do a detailed patch review (which will look at the approach to solving the problem, coding standards, etc.).
Once that is complete, and if no issues found, the reviewer will update the issue to 'Reviewed and tested by community' status; otherwise they will update it to 'needs work' and identify any further changes which need to be taken.
See https://drupal.org/patch/review for more information on the patch review process.
Comment #55
yuradoc CreditAttribution: yuradoc commentedHello.
I upload the patch that contains "Coding Standards" improvements founded after chattings with chx on #drupal-contribute IRC.
Comment #56
Cyberwolf CreditAttribution: Cyberwolf commentedClosed #2074535: Warning: date_format() expects parameter 1 to be DateTime, boolean given in Drupal\node\NodeFormController->form() (line 202 of as a duplicate of this one.
Edit: undid this, does not seem to be the exact same issue.
Comment #57
yuradoc CreditAttribution: yuradoc commentedHello. I upload patch with Unit Test included.
Comment #59
yuradoc CreditAttribution: yuradoc commentedHello - as previous patch is not possible to apply - please use #55.
I upload Unit Test separately (i made an archieve because it's not possible to upload php file). Please place file in "/core/modules/system/lib/Drupal/system/Tests/Datetime" folder. Don't forget clean test environment. After that test can be founded in Datetime group.
Comment #60
yuradoc CreditAttribution: yuradoc commentedUpdated patch with Unit Test.
Comment #61
chx CreditAttribution: chx commentedThanks, this looks really great!
Comment #62
alexpottI think this change can be neater. An expanded context is
So we're assigning
$format_string_type
based on $settings['format_string_type'] or a default just above. So we could assign $settings['format_string_type'] instead and use it. One less line of code to maintain :)Attaching patch that does this - since this change is so minor leaving at rtbc.
Comment #63
alexpottCommitted 5b3ff0d and pushed to 8.x. Thanks!
Comment #65
BerdirEditing the node form is apparently still broken when using intl and not having datetime.module enabled. See #2031183: Improve test coverage for node authored on widget. Help would be very much welcome.