Last Comment Bug 543375 - Undated RSS and Atom feeds result in display of bad date
: Undated RSS and Atom feeds result in display of bad date
Status: RESOLVED FIXED
: fixed-seamonkey2.0.4
Product: SeaMonkey
Classification: Client Software
Component: Feed Discovery and Preview (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: seamonkey2.1a1
Assigned To: Philip Chee
:
:
Mentors:
Depends on:
Blocks: 415372
  Show dependency treegraph
 
Reported: 2010-01-31 15:12 PST by Andy Boze
Modified: 2010-03-31 09:00 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 one line fix. (1.31 KB, patch)
2010-02-01 04:29 PST, Philip Chee
neil: review-
Details | Diff | Splinter Review
Patch v1.1 (1.59 KB, patch)
2010-02-05 02:20 PST, Philip Chee
neil: review+
neil: superreview+
Details | Diff | Splinter Review
Patch v1.2 Fix nits [Checkin: Comment 9 & 10] (1.57 KB, patch)
2010-02-06 01:26 PST, Philip Chee
philip.chee: review+
philip.chee: superreview+
kairo: approval‑seamonkey2.0.4+
Details | Diff | Splinter Review

Description Andy Boze 2010-01-31 15:12:42 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8pre) Gecko/20100129 SeaMonkey/2.0.3pre compatible Firefox/3.5.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8pre) Gecko/20100129 SeaMonkey/2.0.3pre

If you display an RSS or Atom feed that had no date, SeaMonkey displays the wrong date instead of no date.

Reproducible: Always

Steps to Reproduce:
1. Go to http://akapuma.info/
2. Click on the feed indicator in the location bar and select Atom.
3.
Actual Results:  
The feed is dated "Wednesday, December 31, 1969 7:00 PM".

Expected Results:  
No date should be displayed.

See http://www.nd.edu/~fboze/index.shtml for an example of a similar RSS malfunction.
Comment 1 Philip Chee 2010-02-01 03:09:01 PST
Firefox (Minefield) does this:

  _parseDate: function FW__parseDate(dateString) {
    // Convert the date into the user's local time zone
    dateObj = new Date(dateString);

    // Make sure the date we're given is valid.
    if (!dateObj.getTime())
      return false;

SeaMonkey 2.1a1pre does this:

  _parseDate: function FW__parseDate(dateString) {
    // Convert the date into the user's local time zone
    dateObj = new Date(dateString);

    // Make sure the date we're given is valid.
    if (!dateObj.getTime())
      return false;

var dateObj = new Date(null);
dateObj.getTime(); <= result 0

isNaN(dateObj.getTime()); <= result false

!dateObj.getTime(); <= result true

So |new Date(null)| basically gives the current date.
Comment 2 Philip Chee 2010-02-01 03:17:42 PST
From Bug 415372 Comment 24    neil@parkwaycc.co.uk      2009-01-25 13:53:48 PST

>>+    if (!dateObj.getTime())
> if (isNAN(dateObj.getTime()))
Woo!
Comment 3 Philip Chee 2010-02-01 04:29:52 PST
Created attachment 424572 [details] [diff] [review]
Patch v1.0 one line fix.

-    if (isNaN(dateObj.getTime()))
+    if (!dateObj.getTime())

Correctness fix.
Comment 4 neil@parkwaycc.co.uk 2010-02-01 06:05:58 PST
Comment on attachment 424572 [details] [diff] [review]
Patch v1.0 one line fix.

The problem here is that we're expecting new Date(null) to fail, but it doesn't, since the null gets converted to zero, which is a valid date.

So, you could either return early for an empty or null date string, or you could use Date.parse to convert it into a number first, and if you don't get NaN then you can then make a date from that number.
Comment 5 Philip Chee 2010-02-05 02:20:07 PST
Created attachment 425428 [details] [diff] [review]
Patch v1.1

> So, you could either return early for an empty or null date string, or you
> could use Date.parse to convert it into a number first, and if you don't get
> NaN then you can then make a date from that number.
I've used the latter recommendation.
Comment 6 neil@parkwaycc.co.uk 2010-02-05 09:09:21 PST
Comment on attachment 425428 [details] [diff] [review]
Patch v1.1

>+    // Make sure the date we're given is valid.
>+    if (isNaN(Date.parse(dateString)))
So, we get to parse the date twice... passing the return value of Date.parse into new Date would avoid that, but probably look uglier instead.

>+      return null;
>+    // Convert the date into the user's local time zone.
>     var dateObj = new Date(dateString);
> 
Nit: odd spacing, could do with the blank line above moved to after the return.
Comment 7 Philip Chee 2010-02-06 01:26:43 PST
Created attachment 425629 [details] [diff] [review]
Patch v1.2 Fix nits
[Checkin: Comment 9 & 10]

Carrying forward r=neil sr=neil

> So, we get to parse the date twice... passing the return value of Date.parse
> into new Date would avoid that, but probably look uglier instead.
I agree.

>>+      return null;
>>+    // Convert the date into the user's local time zone.
>>     var dateObj = new Date(dateString);
>>
> Nit: odd spacing, could do with the blank line above moved to after the return.
Fixed.
Comment 8 Philip Chee 2010-02-06 01:29:23 PST
Comment on attachment 425629 [details] [diff] [review]
Patch v1.2 Fix nits
[Checkin: Comment 9 & 10]

Correctness fix to bogus dates in Feed Preview.
Risk factors: none I can think of.
Comment 9 Serge Gautherie (:sgautherie) 2010-02-07 15:52:32 PST
Comment on attachment 425629 [details] [diff] [review]
Patch v1.2 Fix nits
[Checkin: Comment 9 & 10]


http://hg.mozilla.org/comm-central/rev/7073dcec0ba3
Comment 10 Serge Gautherie (:sgautherie) 2010-02-08 08:16:14 PST
Comment on attachment 425629 [details] [diff] [review]
Patch v1.2 Fix nits
[Checkin: Comment 9 & 10]


http://hg.mozilla.org/releases/comm-1.9.1/rev/70d0c9ec8baa
Comment 11 Thomas 2010-03-31 08:52:13 PDT
Hello,

seamonkey 2.0.4 is published, and the bug is fixed.

After seamonkey works fine, I updated my homepage with the new "easy feed editor" version 3.0. In this version, the atom feed is with date. Therefore, my homepage http://akapuma.info is not longer appropriate for testing.

Best regards

akapuma

Note You need to log in before you can comment on or make changes to this bug.