WordPress.org

Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#9730 closed defect (bug) (fixed)

The new date_i18n hook creates potential bugs in the admin interface

Reported by: Denis-de-Bernardy Owned by: ryan
Milestone: 2.8 Priority: normal
Severity: major Version: 2.8
Component: I18N Keywords: has-patch tested commit
Focuses: Cc:

Description

Details here:

http://core.trac.wordpress.org/ticket/7753#comment:17

If somebody write a plugin and use the new date_i18n hook to override calendar system or to localize date/time digits, it will corrupt the fields of publish date. While editing a post, the fields of its publish date/time should kept in Gregorian calendar with Latin digits to prevent such problem.

I'll scan through the various calls.

Attachments (10)

template.diff (1.3 KB) - added by kambiz.k 11 years ago.
Patch for [wp-admin\includes\template.php] to fix the issue
9730-wp-admin.diff (4.7 KB) - added by Denis-de-Bernardy 11 years ago.
wp-admin area
9730-wp-includes.diff (4.2 KB) - added by Denis-de-Bernardy 11 years ago.
wp-includes area
9730-misc.diff (4.5 KB) - added by Denis-de-Bernardy 11 years ago.
wp-app and xml-rpc
9730-plugin.php (1.9 KB) - added by hakre 11 years ago.
Test plugin. Have fun with the new mooddate standard
9730.docblock.patch (689 bytes) - added by hakre 11 years ago.
Docblock patch
9730.diff (18.1 KB) - added by Denis-de-Bernardy 11 years ago.
fix comment feeds too
9730.2.diff (20.9 KB) - added by Denis-de-Bernardy 11 years ago.
same as 9730.diff, with a slightly different approach for get_post_time() and get_post_modified_time()
9730.extra.wp_get_archives.patch (775 bytes) - added by hakre 11 years ago.
one line left. should be editied wether true or false.
9730.3.diff (20.9 KB) - added by Denis-de-Bernardy 11 years ago.
refreshed against 11256

Download all attachments as: .zip

Change History (40)

@kambiz.k
11 years ago

Patch for [wp-admin\includes\template.php] to fix the issue

#1 @hakre
11 years ago

  • Keywords has-patch needs-testing reporter-feedback added; needs-patch removed

patch looks good according to description on the linked comments. hook is date_i18n hook. i wonder if someone can provide a crazy conversion funktion for that hook sothat i can try to break it? a plugin as attachment would be nice sothat this can be tested easily.

#2 @hakre
11 years ago

might be releated #9396

#3 @hakre
11 years ago

filter born here [10897]

#4 @Denis-de-Bernardy
11 years ago

@kambiz: There are plenty more. Doing the wp-admin functions as I write.

@hakre: There is a latin2persian number formatting function in #7753.

strtr 0-9 into arbitrary strings is a good test too. :-)

#5 @hakre
11 years ago

in function date_i18n() docblock should be changed to signal that parameter $unixtimestamp should be UTC value.

@Denis-de-Bernardy
11 years ago

wp-admin area

#6 follow-up: @Denis-de-Bernardy
11 years ago

there is one I'm not 100% on:

  • wp_get_archives(), $text = mysql2date($archive_day_date_format, $date); no?

and the calendar function might need to be looked into.

@Denis-de-Bernardy
11 years ago

wp-includes area

@Denis-de-Bernardy
11 years ago

wp-app and xml-rpc

#7 @hakre
11 years ago

in function date_i18n() docblock should be changed to signal that parameter $dateformatstring should get a detailed description which of the formats. link to approriate resources online etc. .

#8 @Denis-de-Bernardy
11 years ago

  • Keywords reporter-feedback removed

@hakre: please upload the docblock patch.

@kambiz: 9730.diff changes every call to mysql2date that doesn't show on the front-end. Can you give that one a shot and verify that everything continues to work as expected when you translate latin digits to persian digits?

@hakre
11 years ago

Test plugin. Have fun with the new mooddate standard

@hakre
11 years ago

Docblock patch

#9 @Denis-de-Bernardy
11 years ago

thanks for the test plugin.

#10 @hakre
11 years ago

docblock patch having my suggestions for date_i18n. testplugin that will convert any date into a mooddate. mooddate uses persian numbers and know only one year: the year I felt like a timeless beauty.

i will test 9730.diff now.

#11 @Denis-de-Bernardy
11 years ago

@hakre: I updated 9730.diff to fix the date_i18n hook on a false timestamp

#12 @Denis-de-Bernardy
11 years ago

admin interface remains functional. options screen, new/edit post, page, image, comment all work fine, no scripts seem to be breaking.

#13 @Denis-de-Bernardy
11 years ago

archives and calendar are not translated, but that is what #7753 was about in the first place.

#14 in reply to: ↑ 6 @hakre
11 years ago

Replying to Denis-de-Bernardy:

there is one I'm not 100% on:

  • wp_get_archives(), $text = mysql2date($archive_day_date_format, $date); no?

and the calendar function might need to be looked into.

wp_get_archives() is a template function used in themes. should be okay to be hooked.

calender function? if you mean get_calendar() in genreal-template the I would say, is a place to take a look into:

		$thismonth = $wpdb->get_var("SELECT DATE_FORMAT((DATE_ADD('${thisyear}0101', INTERVAL $d DAY) ), '%m')");

and it makes heavy use of $wp_locale.


patch looks good to me. i tested it in the backend and the frontend. where applicable, the translated date is displayed. editing for posts works as known.

#15 @hakre
11 years ago

I'll take a look at archives widget.

#16 @Denis-de-Bernardy
11 years ago

found issues in the atom feed, uploading a new patch in a sec.

@Denis-de-Bernardy
11 years ago

fix comment feeds too

#17 @Denis-de-Bernardy
11 years ago

latest patch fixes an issue in the original one related to comment feeds. xml-rpc needs a few tests too.

#18 @Denis-de-Bernardy
11 years ago

  • Keywords tested added; needs-testing removed

I'm good with the latest patch.

#19 @Denis-de-Bernardy
11 years ago

  • Keywords commit added

and so is hakre... let's hope this quickly gets into trunk, as I wouldn't want to end up needing to redo it after trunk gets edited all over the place.

#20 @Denis-de-Bernardy
11 years ago

@Ryan or Andrew -- this one is related and good to go as well:

http://core.trac.wordpress.org/attachment/ticket/7753/7753.2.diff

#21 @Denis-de-Bernardy
11 years ago

  • Keywords commit removed

mm, nm that. I'm going to give get_post_time() et al the same treatment as get_comment_time(), for consistency's sake.

#22 follow-up: @hakre
11 years ago

i could take a look into archives, that is especially wp_get_archives() in /wp-includes/general-template.php .

compareable with the calender stuff, this function uses misc approaches to translate date and time information:

  • Just numbers. If only a fragment of a date/time is used (like year or day), often only a number is used.
  • WP_locale / $wp_locale - this is used to translate month names etc.
  • {{()}} - often the result is "translated" again.

i assume that all this is done sothat current users can already translate (at least some) of those values.

as denis already pointed, archives and calendar are not translated, but that is what #7753 is after.

to enable translation of archives and calendar, these functions must make use of date_i18n() or make use of the $translate (the 3rd) parameter of mysql2date().

i suggest to use that parameter at least for wp_get_archives() and get_calendar() in general-template.php.

other potention functions: the_date(), get_post_modified_time().

i must first read on how to get UTC timestamps from partial data values before continue.

@Denis-de-Bernardy
11 years ago

same as 9730.diff, with a slightly different approach for get_post_time() and get_post_modified_time()

#23 @Denis-de-Bernardy
11 years ago

  • Keywords commit added

9730.2.diff is the preferable imo

#24 in reply to: ↑ 22 @Denis-de-Bernardy
11 years ago

Replying to hakre:

i could take a look into archives, that is especially wp_get_archives() in /wp-includes/general-template.php .

compareable with the calender stuff, this function uses misc approaches to translate date and time information:

See: http://core.trac.wordpress.org/ticket/7753#comment:13

#25 @hakre
11 years ago

yeah i think it is a good idea to stop at this certain level. this will at least offer a lot more possibilities.

for the frontend / theme / widget areas some best practice suggestions should be made, like let's say, convert partially values (number of the year), convert it into a timestamp and the pass it over with the 'Y' formatstring to date_i18n() or mysql2date() with $translate = true.

plust +1 to have widgets with their own calendars. maybe archive strings are fixable, there is one myslq2date left i mean:

			$text = mysql2date($archive_day_date_format, $date, true);

line 838. will add a patch for that one.

@hakre
11 years ago

one line left. should be editied wether true or false.

#26 @Denis-de-Bernardy
11 years ago

@hakre: true is the default. :-)

#27 @ryan
11 years ago

  • Owner changed from Denis-de-Bernardy to ryan

@Denis-de-Bernardy
11 years ago

refreshed against 11256

#28 @ryan
11 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11323]) Don't localize dates where not appropriate. Props Denis-de-Bernardy, hakre. fixes #9730

#29 @miqrogroove
11 years ago

Please see #11421. It looks like feed-atom.php and feed-rss2.php were missed.

#30 @miqrogroove
11 years ago

Disregard last comment. The code is very hard to read with mismatched implicit parameters, but is working.

Note: See TracTickets for help on using tickets.