Fixed non-iso8601 Tuesday/Thursday dates #149

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants

Oddly this bug only appeared in Firefox (25) and IE (10) while remaining hidden in Chrome (30).

$.timeago("Tue Nov 12 20:45:30 2013")

Chrome: "about 13 hours ago"
Firefox/IE: "NaN years ago"

Now you might say you don't like people shoving non-iso8601 into your api, but at the time I wrote my site I had no idea that new Date(strange_timestamp) solved my conversion problem. I just pushed my ugly timestamp into timeago and thought it worked. So either you actually intercept and fail on bad timestamps or do this minor error avoidance tactic.

@ubershmekel ubershmekel Fixed non-iso8601 Tuesday/Thursday dates
Oddly this bug only appeared in Firefox (25) and IE (10) while remaining hidden in Chrome (30).

$.timeago("Tue Nov 12 20:45:30 2013")

Chrome: "about 13 hours ago"
Firefox/IE: "NaN years ago"

Now you might say you don't like people shoving non-iso8601 into your api, but at the time I wrote my site I had no idea that `new Date(strange_timestamp)` solved my conversion problem. I just pushed my ugly timestamp into timeago and thought it worked. So either you actually intercept and fail on bad timestamps or do this minor error avoidance tactic.
97682d1
Owner

rmm5t commented Nov 13, 2013

Normally, I would say "I don't like people shoving non-iso8601 into my api."

However, this is potentially a uniquely trivial solution to the problem of supporting another date format. Please add a test case for this scenario, and I'll consider it.

I cannot guarantee this will continue to work when Timeago goes 2.x, but I cannot guarantee a v2.x will ever be released anyway.

Added a test case though I can't guarantee it's how you would have written it. This is my first time seeing QUnit. There's a certain style of tests there that I don't understand fully yet.

Owner

rmm5t commented Nov 14, 2013

@ubershmekel This new regexp replacement breaks 9 existing test cases when you run the test suite. Sorry, I don't think I can pull it in. It looks like ISO-8601 parsing is going to be required.

rmm5t closed this Nov 14, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment