Big bad DateTime bug workaround for correct ulocalized_time() timezone support #47

merged 3 commits into from Aug 22, 2012

2 participants

Plone Foundation member

See for discussion of bug.

We have to work around it to display the times correct for the timezone displayed in event_view. Any use of ulocaliized_time() in an add-on to Plone where dates involved have timezones should be suspect. This solution wraps the call, and replaces the instance with an instance of a fixed DateTime subclass that does strftime without bug.

seanupton added some commits Aug 22, 2012
@seanupton seanupton commented out pdb.set_trace() in tests cbe6a86
@seanupton seanupton work around bug in DateTime.strftime that always normalizes displayed…
… date to the system timezone, regardless of the timezone on the DateTime value itself. This bug is not likely to get fixed in Zope DateTime upstream soon (long-standing), so Products.CMFPlone.i18nl10n.ulocalized_time is subject to triggering it -- here that function is wrapped with a local copy that does the right thing with the timezone by using datetime.datetime.strftime without broken magic of DateTime.strftime.
Plone Foundation member

without trying to understand the whole issue in detail (have my head in another tricky problem ATM), i shoot out some naive questions:

  • do you think, the solution can be better done in zope's DateTime? then the fix should go there, probably

  • this affects mostly the AT types of, or? (just asking to understand you have worked with the ATEvent type of too)

  • regarding the DT2dt function, did you see the plone.event.utils.pydt function? if it can be used with your patch, we should use this one

Plone Foundation member

Yes, DT2dt() is (in practical terms) duplicative of pydt(). Both preserve the timezone and DST correctly, which was my goal, so I will remove DT2dt and use pydt().

@seanupton seanupton remove duplicative transformation function DT2dt, use plone.event.uti…
…ls.pydt() instead, as both behave similarly with respect to timezone and DST in conversion.
Plone Foundation member

Since this affects any date display in where the date is not in the system timezone, it seems reasonable to try and fix this here now (with the value-wrapping) and then push this to both Plone (Products.CMFPlone.i18nl10n) and Zope (DateTime in zope svn) upstreams -- in that order.

@thet thet merged commit 44544a9 into plone:api-refactoring Aug 22, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment