Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 4 years ago

#2693 closed defect (bug) (fixed)

bp_format_time returns incorrect value when gmt_offset is empty

Reported by: stevejohnson Owned by:
Milestone: 2.3 Priority: normal
Severity: normal Version:
Component: Core Keywords: needs-patch needs-unit-tests
Cc: steve@…, jjj@…

Description

When the root blog's timezone is set to a Region/City string, WP sets the option timezone_string, and gmt_offset value is empty. Only when the timezone is set to a GMT time does gmt_offset contain a value. This results in incorrect time strings for anything that uses bp_format_time.

Function bp_format_time should be modified to take this into account.

Change History (13)

#1 follow-up: @DJPaul
9 years ago

Can you confirm if you tested this on PHP4 or PHP5?

#2 in reply to: ↑ 1 @stevejohnson
9 years ago

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

PHP 5.2.4

This ticket may need to be modified/deleted. The issue is more widespread than in just bp_format_time. I need to do some more investigation. To date, I've had to write 3 filters affecting 3 different date/time reporting functions.

I'll see if I can dig deeper.

Replying to DJPaul:

Can you confirm if you tested this on PHP4 or PHP5?

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


5 years ago

#4 @johnjamesjacoby
5 years ago

  • Cc jjj@… added
  • Keywords needs-patch needs-unit-tests added; gmt_offset timezone_string removed
  • Milestone changed from 1.5 to 2.3
  • Resolution invalid deleted
  • Severity set to normal
  • Status changed from closed to reopened

#5 @johnjamesjacoby
5 years ago

In 9294:

Unit tests for bp_format_time() to test gmt_offset and timezone_string. See #2693.

#6 @johnjamesjacoby
5 years ago

This might be invalid, thanks to wp_timezone_override_offset() introduced in WordPress 2.8.0. Added the above unit tests to help confirm some suspicions, though I think there are more conditions worth testing here.

#7 @johnjamesjacoby
5 years ago

In 9295:

Explicitly set date_format and time_format options in bp_format_time() tests. See #2693.

#8 @johnjamesjacoby
5 years ago

In 9616:

Core: Update tests for bp_format_time:

  • Include 2 new tests without localization, useful for maintaining sanity.
  • Update 2 existing tests to include WordPress core default options.
  • Update timezone_string test time to look for "2:00 pm" (as it is theoretically DST aware) though there is likely more to test here.

See #2693.

#9 @johnjamesjacoby
5 years ago

In 9617:

Core: Update bp_format_time:

  • Use bp_get_option() over get_blog_option() and (incorrect) get_option() calls.
  • Rename variables for clarity, as generic date/time/offset variables were easy to accidentally stomp.
  • Port logic from wp_timezone_override_offset() and calculate timezone offset if timezone_string is set in the root blog's options.
  • Pass $localize_time parameter into date_i18n() function call, to ensure the most accurate results possible are returned back from WordPress.

See #2693.

#10 @johnjamesjacoby
5 years ago

In 9618:

Core: Clean up bp_format_time:

  • More detailed phpdoc block.
  • Add @since tag.
  • White space is delicious.
  • Rename parameters to avoid double negative checks.
  • Rename another variable.

See #2693.

#11 @boonebgorges
4 years ago

Is this done?

#12 @johnjamesjacoby
4 years ago

In trunk, yes. I am about 90% certain. :)

#13 @johnjamesjacoby
4 years ago

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

Confirmed complete in trunk for 2.3.0.

Closing as fixed.

Note: See TracTickets for help on using tickets.