parse W3C-DTF dates correctly

RESOLVED FIXED in Firefox1.5

Status

defect
RESOLVED FIXED
14 years ago
5 months ago

People

(Reporter: myk, Assigned: myk)

Tracking

({fixed1.8})

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

14 years ago
feedview's W3C-DTF date parser doesn't always parse dates correctly.  For
example, mozilla.org's news feed's dates show up as "Invalid Dat @ NaN:NaN".

http://www.mozilla.org/news.rdf

feedview should use the W3CToIETFDate conversion function per bug 302121,
comment 44.
Assignee

Comment 1

14 years ago
Unfortunately because feedview.js is included in content it has no access to
chrome JavaScript, so we can't factor out W3CToIETFDate from FeedItem.js and
use it both here and in Thunderbird's newsblog extension.

So I've just copied the function into feedview.js.  Note that this almost
certainly conflicts with the patch on bug 303848.  Perhaps it makes sense for
Ben to land that (much larger and more complex) patch first.
Attachment #192416 - Flags: review?(mconnor)
*** Bug 303052 has been marked as a duplicate of this bug. ***
Assignee

Comment 3

14 years ago
Comment on attachment 192416 [details] [diff] [review]
patch v1: switches to W3CToIETFDate function

Going to attach a new patch.
Attachment #192416 - Flags: review?(mconnor)
Assignee

Comment 4

14 years ago
This patch updates the code for the bug 303848 landing, which changed a lot of
things.  In addition to implementing a much better W3C-DTF to IETF date
conversion function, it also fixes some problems with the code from bug 303848
that cause it not to work, namely:

* feeds don't get initialized because Feed.init() is commented out;
* even if feeds did get initialized, dates wouldn't appear because
initializeDates() calls xmlDate() instead of this._xmlDate() (but of course
with this patch it now calls this._W3CToIETFDate());
* the sidebar disappears as soon as it appears because it looks for a
mixed-case "showMenu" attribute, but the XSLT processor converted it to the
lower-case "showmenu", so init() thinks the user doesn't want to see the menu.

It also fixes two JavaScript strict warnings about invalid commas.

I can supply these as separate patches if you would prefer that.
Attachment #192416 - Attachment is obsolete: true
Attachment #192491 - Flags: review?(mconnor)
Comment on attachment 192491 [details] [diff] [review]
patch v1: unrotted from bug 303848 landing

>Index: browser/base/content/feedview.js
>     // Set up the auto-reload timer    
>     setTimeout("RSSPrettyPrint.refresh()", 
>-               this._getIntegerPref("reloadInterval") * 1000);
>-  },
>+               this._getIntegerPref("reloadinterval") * 1000);
>+  }

One thing I noticed - I didn't update the variable name here... that should be
"FeedView.refresh()" not "RSSPrettyPrint.refresh()"

-Ben
Assignee

Comment 6

14 years ago
> One thing I noticed - I didn't update the variable name here... that should be
> "FeedView.refresh()" not "RSSPrettyPrint.refresh()"

Indeed, but that's a good thing, since over in browser.js, line 6214 you
hardcoded reloadInterval to "30", so if this code worked it would make all RSS
feeds refresh every 30 seconds, causing many RSS providers to block Firefox users.

http://lxr.mozilla.org/mozilla/source/browser/base/content/browser.js#6214

I'll update the patch to use the browser.feedview.reloadInterval preference (set
to "0", a.k.a. disabled, by default).
Assignee

Comment 7

14 years ago
This patch adds back the "showMenu" and "reloadInterval" preferences, removes
the others, sets the "reloadInterval" preference to "0" (disabled) by default,
makes the "reloadInterval" preference be minutes rather than seconds, and makes
reloading work when that preference isn't disabled.

But I see that over in bug 303848 Ben is about to back out his fixes, so
perhaps this is all for naught.
Attachment #192491 - Attachment is obsolete: true
Attachment #192570 - Flags: review?(mconnor)
Comment on attachment 192570 [details] [diff] [review]
patch v2: fixes additional issues

sr=ben@mozilla.org
Attachment #192570 - Flags: superreview+
Attachment #192570 - Flags: review?(mconnor) → review+
Assignee

Updated

14 years ago
Attachment #192491 - Flags: review?(mconnor)
Assignee

Updated

14 years ago
Attachment #192570 - Flags: approval1.8b4?
Assignee

Comment 9

14 years ago
Thanks Ben and Mike!  Fix checked in to the trunk; still waiting for approval to
check it in to the branch.

Checking in browser/base/content/feedview.js;
/cvsroot/mozilla/browser/base/content/feedview.js,v  <--  feedview.js
new revision: 1.2; previous revision: 1.1
done
Checking in browser/base/content/feedview.xsl;
/cvsroot/mozilla/browser/base/content/feedview.xsl,v  <--  feedview.xsl
new revision: 1.2; previous revision: 1.1
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.482; previous revision: 1.481
done
Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.72; previous revision: 1.71
done
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox1.1

Updated

14 years ago
Attachment #192570 - Flags: approval1.8b4? → approval1.8b4+
Assignee

Comment 10

14 years ago
Patch checked in to 1.8 branch:

Checking in browser/base/content/feedview.js;
/cvsroot/mozilla/browser/base/content/feedview.js,v  <--  feedview.js
new revision: 1.1.2.1; previous revision: 1.1
done
Checking in browser/base/content/feedview.xsl;
/cvsroot/mozilla/browser/base/content/feedview.xsl,v  <--  feedview.xsl
new revision: 1.1.2.1; previous revision: 1.1
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.479.2.4; previous revision: 1.479.2.3
done
Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.71.2.1; previous revision: 1.71
done
Resetting QA Contact to default.
QA Contact: nobody → rss.preview

Updated

5 months ago
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.