Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Undated RSS and Atom feeds result in display of bad date

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
Feed Discovery and Preview
RESOLVED FIXED
8 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

8 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

8 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

8 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

8 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

8 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

8 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

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

Comment 6

8 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

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

Comment 7

8 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

8 years ago
Keywords: checkin-needed
(Assignee)

Updated

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

Comment 8

8 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

8 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: 8 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.