Re: [PATCH 1/1] gitweb: javascript ability to adjust time based on timezone

[Jakub Narebski <jnareb@gmail.com>]

On Thu, 24 Mar 2011, Kevin Cernekee wrote:
> On Wed, Mar 23, 2011 at 5:08 PM, John 'Warthog9' Hawley
> <warthog9@eaglescrag.net> wrote:

> > This patch takes the same basic goal, display the appropriate times
> > in a given timezone, and implements it in Javascript.  This requires
> > adding / using a new class, dtcommit, which is based on the
> > dtstart/dtend microformats.  Appropriate commit dates are wrapped in
> > a span with this class, and a title of the time in ISO8601 format.
> 
> John,
> 
> Thanks for coding this up.  I tested it on a couple of different
> browsers and wanted to share my observations with you.

I wonder if there is any site that allows to check JavaScript for
compatibility with different browsers...

> 
> First, the easy stuff:
> 
> 1) "git am" complains about whitespace violations

It would be more helpful if you wrote here what are those whitespace
violations.

> 
> 2) HH:MM:SS times need zero padding; otherwise you see:
> 
> Tue, 8 Mar 2011 20:29:9 -0700

There is even padLeft function in gitweb.js ready to be (re)used...

[...] 
> 4) IE6 does not seem to like ISO 8601 format:
> 
> x = new Date("2011-03-09T03:29:09Z");
> 
> This sets all fields to NaN.  I suspect that getTime() values
> (milliseconds since 1970-01-01) are more portable.

Do you mean using epoch in title attribute, or fallback to parsing
ISO 8601 UTC format with regexps?

> I have attached a trivial patch for these four items; it applies on
> top of your original submission.

It would be much, much easier to review your patch if you either
put it inline at the end of your email, separating it from the rest
of email with e.g. "-- >8 --" separator (so called 'scissors'),
or at least using 'text/plain' as mimetype, '8bit' and not 'base64'
encoding, and perhaps 'disposition=inline' rather than 
'disposition=attachement'.

[...]
> 6) Most U.S. timezones honor daylight savings, so they could be
> something like -0700 for part of the year, and -0800 for the rest of
> the year.  Picking the "local" option would automatically adjust for
> this, but DST limits the usefulness of permanently storing a fixed TZ
> offset in the cookie.

Dealing with DST (zoneinfo library) is simply too hard for JavaScript 
IMHO.  What we could do is to store "local" in cookie, not a fixed TZ
offset (or perhaps store both as to not recalculate it).


> 8) The " + " popup menu is a little quirky.  On FF 3.6 it partially
> collapses after selecting a value from the dropdown.  On IE6 it shows
> "Error in parsing value for 'display'" and does not render.  On Opera
> 11 it seemed to work OK.
> 
> Firefox breakage: http://img217.imageshack.us/f/firefoxa.png/
> 
> I'm wondering if there might be a better place on the page to put the
> TZ selection.  It isn't immediately obvious to the user what the extra
> " + " does, and it seems to cause some issues.

Hmmm... perhaps a 'config' page?

-- 
Jakub Narebski
Poland