| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | system.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | needs backport to D7 |
Issue Summary
Problem/Motivation
The function system_date_format_save() implements the functionality to save localized date formats in the table {date_format_locale}. Unfortunately, the check whether or not to save a format localized is broken. A list of enabled languages is fetched with the following code:
<?php
$languages = language_list('enabled');
$languages = $languages[1];
?>When checking each date format locale against this list to see if the language is enabled, the function uses if (in_array($langcode, $languages)). However, the langcodes are in the keys of this array. The values are full database records for each language.
Proposed resolution
Correct the function to use if (isset($languages[$langcode])).
Remaining tasks
- The patch in #981524-18: system_date_format_save() doesn't save localized date formats is RTBC and includes tests.
- #1234848: language_list() doc is missing return value has been opened to address the lack of documentation for the return value of language_list().
User interface changes
None.
API changes
None.
Original report by @das-peter
The function system_date_format_save() already implements the functionality to save localized date formats in the table date_format_locale.Unfortunately, the check whether or not to save a format localized is broken.The attached patch fixes this.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| system-system_date_format_save-localized-date-format-not-saved.patch | 796 bytes | Idle | FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in system-system_date_format_save-localized-date-format-not-saved.patch. | View details |
Comments
#1
The last submitted patch, system-system_date_format_save-localized-date-format-not-saved.patch, failed testing.
#2
Removed path prefix
#3
+++ modules/system/system.module@@ -3740,7 +3740,7 @@ function system_date_format_save($date_format, $dfid = 0) {
// Only proceed if language is enabled.
- if (in_array($langcode, $languages)) {
+ if (isset($languages[$langcode])) {
Disabled languages are contained in $languages, too, AFAIK.
Powered by Dreditor.
#4
$languagesis fetched using this code:$languages = language_list('enabled');$languages = $languages[1];
Thus there should be only the enabled languages in the variable.
Besides that I'm not sure how evil it would be to store the date formats from disabled languages ;)
#5
Thanks for clarifying!
#6
Sorry, this needs tests.
#7
Hmm, I'm not sure if the attached tests are sufficient. There is a lot more that could be covered...
But at least the topic of this issue is covered ;)
#8
#9
Testbot says invalid patch file.
#10
#11
The last submitted patch, system-system_date_format_save-localized-date-format-not-saved-tests-981524-10.patch, failed testing.
#12
+++ modules/system/system.test@@ -1044,6 +1044,70 @@ class DateTimeFunctionalTest extends DrupalWebTestCase {
+ $query = db_select('date_formats', 'df');
+ $query->addField('df', 'format');
+ $query->condition('type', 'short');
+ $query->condition('format', 'dmYHis');
+ $format = $query->execute()->fetchColumn();
Can you rewrite these queries to:
$format = db_select('date_formats', 'df')->fields('df', array('format'))
->condition('type', 'short')
->condition('format', 'dmYHis')
->execute()
->fetchField();
Powered by Dreditor.
#13
Thx sun - nice hint. Didn't recognized that always the
addFieldkills the chaining.#14
#15
The last submitted patch, system-system_date_format_save-localized-date-format-not-saved-tests-981524-13.patch, failed testing.
#16
Rerolled the patch against the current 7.x branch and verified that the tests run successfully.
#17
+++ b/modules/system/system.test@@ -1048,6 +1048,74 @@ class DateTimeFunctionalTest extends DrupalWebTestCase {
}
+ /**
Missing blank line.
2 days to next Drupal core point release.
#18
Sorry for that, should be corrected now.
#19
Thank you sun for the review & thank you Pisco for the updated patch.
Would be nice to get rid of this diff in our own drupal repo. :)
#20
Thanks! Let's get this in.
#21
Issue summary added. I also opened #1234848: language_list() doc is missing return value to address the (lack of) documentation for
language_list().#22
Committed and pushed to 8.x and 7.x. Thanks!
#23
Automatically closed -- issue fixed for 2 weeks with no activity.