Undated RSS and Atom feeds result in display of bad date

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
Feed Discovery and Preview
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Andy Boze, Assigned: Philip Chee)

Tracking

({fixed-seamonkey2.0.4})

Trunk
seamonkey2.1a1
x86
Windows XP
fixed-seamonkey2.0.4

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

1.57 KB, patch
Philip Chee
: review+
Philip Chee
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
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.
(Assignee)

Comment 1

7 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

7 years ago
From Bug 415372 Comment 24    neil@parkwaycc.co.uk      2009-01-25 13:53:48 PST

>>+    if (!dateObj.getTime())
> if (isNAN(dateObj.getTime()))
Woo!
Blocks: 415372
(Assignee)

Comment 3

7 years ago
Created attachment 424572 [details] [diff] [review]
Patch v1.0 one line fix.

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

Correctness fix.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Attachment #424572 - Flags: superreview?(neil)
Attachment #424572 - Flags: review?(neil)

Comment 4

7 years ago
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.
Attachment #424572 - Flags: superreview?(neil)
Attachment #424572 - Flags: review?(neil)
Attachment #424572 - Flags: review-
(Assignee)

Comment 5

7 years ago
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.
Attachment #425428 - Flags: superreview?(neil)
Attachment #425428 - Flags: review?(neil)

Updated

7 years ago
Attachment #425428 - Flags: superreview?(neil)
Attachment #425428 - Flags: superreview+
Attachment #425428 - Flags: review?(neil)
Attachment #425428 - Flags: review+

Comment 6

7 years ago
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.
Version: unspecified → SeaMonkey 2.0 Branch
(Assignee)

Updated

7 years ago
Version: SeaMonkey 2.0 Branch → Trunk
(Assignee)

Comment 7

7 years ago
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.
Attachment #424572 - Attachment is obsolete: true
Attachment #425428 - Attachment is obsolete: true
Attachment #425629 - Flags: superreview+
Attachment #425629 - Flags: review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
(Assignee)

Updated

7 years ago
Target Milestone: --- → seamonkey2.1a1
(Assignee)

Comment 8

7 years ago
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.
Attachment #425629 - Flags: approval-seamonkey2.0.4?

Updated

7 years ago
Attachment #425629 - Flags: approval-seamonkey2.0.4? → approval-seamonkey2.0.4+
Comment on attachment 425629 [details] [diff] [review]
Patch v1.2 Fix nits
[Checkin: Comment 9 & 10]


http://hg.mozilla.org/comm-central/rev/7073dcec0ba3
Attachment #425629 - Attachment description: [for check-in] Patch v1.2 Fix nits r/sr=neil → Patch v1.2 Fix nits [Checkin: Comment 9]
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
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
Attachment #425629 - Attachment description: Patch v1.2 Fix nits [Checkin: Comment 9] → Patch v1.2 Fix nits [Checkin: Comment 9 & 10]
Keywords: checkin-needed → fixed-seamonkey2.0.4

Comment 11

7 years ago
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
(Assignee)

Updated

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