Re: [PATCH 1/1] gitweb: javascript ability to adjust time based on timezone
- From
J.H. <warthog9@eaglescrag.net>
- Date
- Mar 24, 2011, 23:04 UTC
- Message-ID
- <4D8BCE0D.3080504@eaglescrag.net>
- In-Reply-To
- <201103242119.40214.jnareb@gmail.com>
On 03/24/2011 01:19 PM, Jakub Narebski wrote:
Show 17 quoted lines
> 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...
Not that I know of, would be useful but probably horrifically complicated. When I was doing Javascript and web stuff as my day job, about half of what you would write were wrappers to work around the brokenness on various browsers (mostly IE - ohhh IE...)
Show 13 quoted lines
>> 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...
I'd rather use one that exists than recreate it, probably worth moving that to common-lib.js, but that is a separate commit.
Sorry about that, didn't notice it in testing (it took me a while to find one of my local repos with a commit i nthe first 10 minutes or so), and the documentation I was reading implied that should have been padded.
Show 10 quoted lines
> [...]
>> 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?Parsing it with regexps is doable if completely inelegant, that said this is Javascript...
Looky that MS provides Virtual PC images for compat testing for IE6 http://www.microsoft.com/downloads/en/details.aspx?FamilyID=21eabb90-958f-4b64-b5f1-73d0a413c8ef&displaylang=en
Ok reading through the documentation I can find for MS and their Date.parse ( http://msdn.microsoft.com/en-us/library/dctx55bc(v=VS7.1).aspx ) and some quick experimentation:
2011-03-09 - breaks 2011/03/09 - works
2011/03/09T01:01:01 - works, sorta - the hour comes out as 10 vs. 01 2011/03/09 01:01:01 - works, hour is correct 2011/03/09 01:01:01Z - works and seems to get the TZ correct 2011/03/09T01:01:01Z - breaks
Not really sure what the "right" way to fix this is going to end up being. Suppose our options are:
1) Try and find a format that is generally accepted and parseable on all the browsers 2) Declare IE6 an unsupported browser 3) Trap what browser we are on and do a regex parsing of the string and do appropriate sets or a more verbose format that works everywhere 4) Use a regex and sets for every browser. 5) Switch all the embedded times over to epochs in the title=""'s
4 is probably the "most" right that I can see of those options, and avoids possible other browser inconsistencies with respect to date parsing. I'm going to code that up (it shouldn't be terribly complicated), unless someone likes one of the other ideas better. Suppose we could even go down the more complicate microformat route and just write some code to parse that, just more complicated all the way around.
5 has the appeal that everything handles the epoch correctly, but it does have the downside of being less readable in the html code (and shifts us slightly further away from the accepted "standard" of the microcode formats)
[...]
Show 9 quoted lines
>> 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).
I already provide the option for "local" in the cookie, the other fixed timesets are more for people wanting to be more explicit or look at time zones from a different location.
Show 12 quoted lines
>> 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?
Already made my comments on that in the other e-mail.
- John 'Warthog9' Hawley