381357 - Live bookmarks for feeds served as text/html don't load

Status: VERIFIED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, defect, P2)

Description

[Pablo Greco]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070520 Minefield/3.0a5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070520 Minefield/3.0a5pre

While trying to add a new livemark, errors appear on console and new livemark doesn't load even after restart. On Other profile I have the same livemarks and don't load either (works ok on 2.x and Pre-places Minefield)
Error on console is:
Error: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISAXXMLReader.onDataAvailable]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "JS frame :: file:///C:/Archivos%20de%20programa/Mozilla%20Firefox-Trunk/components/FeedProcessor.js :: FP_onDataAvailable :: line 1303"  data: no]
Source File: file:///C:/Archivos%20de%20programa/Mozilla%20Firefox-Trunk/components/FeedProcessor.js
Line: 1303

Reproducible: Always

Steps to Reproduce:
1.Go to www.lanacion.com.ar
2.Try to add any of the livemars
3.Watch console after clicking "add"
Actual Results:  
New livemark doesn't load and errors on console

Expected Results:  
Livemark should load without problems

Comment 1

[(not reading, please use [e-mail] instead)]

confirming, I'm able to reproduce this.

Comment 2

[Ria Klaassen (not reading all bugmail)]

Regressed after 2007-05-18 19.

Comment 3

[Robert Sayre]

taking

Comment 6

[Mike Connor [:mconnor]]

Seems to be a bit of an edgecase, M9

Comment 7

[Michael Wu [:mwu]]

Actually, this seems to be WFM now.. not sure what fixed it. can someone confirm?

Comment 8

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007081206 Minefield/3.0a8pre

I'm still seeing this, the never loading livemark and the error.

Comment 11

[Phil Ringnalda (:philor)]

I missed it at the time, but the (first, anyway) key to this was there for the seeing in bug 386317 - 

works: http://www.pcinpact.com/include/articles.xml (Content-type: text/xml)
fails: http://www.pcinpact.com/include/news.xml (Content-type: text/html)

Comment 14

[Mike Kroger]

Incorrect content type is certainly one of the issues but it isn't the only one.

I hacked FeedProcessor.js. I commented out this._reader = null in FP_sendResult() and added this._reader = null to FP_onStopRequest(). I've no idea if this is a good fix or not, but it seems to solve the problem.

Comment 15

[Mike Connor [:mconnor]]

moving out bugs that don't need to block b1

Comment 17

[Axel Hecht [:Pike]]

This is going to be a regression in Firefox 3 B1 German, the affected feed is http://www.tagesschau.de/xml/rss2/.

I didn't see any errors in the error console, but I see text/html as content type in wget --save-headers.

Can we beaf this up to B2?

Comment 18

[Axel Hecht [:Pike]]

This might be worth relnoting for B1.

Comment 19

[Phil Ringnalda (:philor)]

Yeah, the error message went away somewhere along the line, but you can see that the problem's (still, again, also) coming from the mimetype from http://dev.philringnalda.com/rsstests/tagesschau.de.rss which is just wget, then served as application/rss+xml.

Comment 21

[Joone Hur]

Mozilla/5.0 (Windows; U; Windows NT 5.1; ko; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9

I have seen the same problem before in Firefox2 for Windows, Linux

Comment 23

[Carsten Book [:Tomcat]]

phil, could this be also the reason why the french rss feed not load (http://fr.fxfeeds.mozilla.com/fr/firefox/headlines.xml) in the beta 2 build ?

Comment 24

[Phil Ringnalda (:philor)]

Looks like that's just server troubles at lemonde.fr - reloading it randomly gives the feed or a redirect to a 404 page.

Comment 32

[Julian Sikorski]

I'm not sure if this is related, but after updating to 20080206 snapshot, things got even worse. Most of the bookmarks fail to load now (including those that did work before), and few that do, display dates instead of titles.

Comment 33

[Julian Sikorski]

OK, I opened a new bug regarding the issue mentioned in comment #32: bug #416517.

Comment 35

[Mike Beltzner [:beltzner, not reading bugmail]]

Why is this P3? It's starting to hit a lot of major feeds (ie: craigslist) and is a regression from Firefox 2.

I suggest at least P2.

Comment 36

[Dietrich Ayala (:dietrich)]

and not making beta 3.

Comment 37

[Neale Upstone]

Another one not working that does in latest FF2: http://www.fridgemountain.com/services/givetake/rsssearchresults.do?submit=true&values(PostCode)=CB1&wantRange=30&values(wantFmCategory)=&categoryAttrName=wantFmCategory&scorePeriodDays=90&values(ItemStatus)=(null)

According to feedvalidator.org, this is a valid feed.

Surely this bug cannot go out not fixed in FF3...?

Comment 38

[Steve England [:stevee]]

Neale read the bug. It's marked blocking‑firefox3+ and the drivers are aware of its severity.

Comment 39

[Neale Upstone]

To eliminate some angles, I've just tested FF3 with updates to ensure that the dates have 4 digit years (not a requirement of RSS).

The dates now appear in the feed preview when adding a live bookmark, but, ONLY if they're GMT (or +0000).  BST does not show.

My view is that FF3 does not regress from FF2.  However, I'm going to change my feed anyway as, according to http://tech.groups.yahoo.com/group/rss-public/message/536, many feed aggregators have similar problems.

Comment 40

[Neale Upstone]

Sorry if this is going to have hit a few inboxes...

I've got the feeds working in FF3 and think I can point the dev team at the use cases that fail in FF3.  

I've ammended the format of all dates to use 4 digit years AND the -0800 style format.  

I did this by changing from yy to yyyy and z to Z in my use of SimpleDateFormat thus giving dates as <pubDate>Mon, 30 Jul 2007 08:37:57 +0100</pubDate> in my items.

The working version is to use: SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss Z")

As far as getting FF3 compatible with FF2, I hope that this is now a case of supporting: 

SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss z")
i.e. <pubDate>Mon, 30 Jul 2007 08:37:57 BST</pubDate>
and 
SimpleDateFormat("EEE, dd MMM yy HH:mm:ss z")
i.e. <pubDate>Mon, 30 Jul 07 08:37:57 BST</pubDate>

The last of these is where I started, which does work in FF2.

Good luck.

Neale

Comment 41

[Robert Sayre]

I can reproduce this, per comment 0, using the feed at http://www.tagesschau.de/xml/rss2/. Just add it to the bookmarks toolbar and right-click reload to reproduce.

Comment 42

[Robert Sayre]

Sure enough, this a the product of the "text/html" MIME type. We're taking the HTML parser path in nsParser, and that doesn't make for a happy SAX handler.

The best way to fix this for now is to force an application/xml type in the livemarks code.

Comment 43

[Robert Sayre]

Attachment: the fix

I changed my mind, an XML type should be forced in the feed processor, since that's all it can deal with right now.

Comment 44

[Neale Upstone]

Robert.. will you be looking at the change in the date handling, as I pointed out in comment 40?  

Your patch only fixes one element of regression compared to what FF2 will allow.

Comment 45

[Robert Sayre]

(In reply to comment #44)
> Robert.. will you be looking at the change in the date handling, as I pointed
> out in comment 40?  

That would be a different bug, please file it separately and CC me.

Comment 46

[Justin Dolske [:Dolske]]

Comment on attachment 308690 [details] [diff] [review]
the fix

>   onStartRequest: function FP_onStartRequest(request, context) {
>+    var channel = request.QueryInterface(Ci.nsIChannel);
>+    if (channel)
>+      channel.contentType = "application/xml";
>     this._reader.onStartRequest(request, context);
>   },

I always mix this up, but doesn't QueryInterface throw if it fails?

So this should instead be:

if (channel instanceof Ci.nsIChannel)
  channel.contentType = "application/xml";

Comment 47

[:Gavin Sharp [email: [e-mail]]]

Yes, I should have caught that. Don't think it's related to bug 423060, though - the code will still work fine.

Comment 48

[Neale Upstone]

Verified against my URL as per comment 37.  Nice one folks.

Comment 49

[Tony Chung [:tchung]]

Awaiting fix for bug 425010 before we can verify this. 

Comment 50

[Aditya Kher]

I noticed that although 415204 is marked duplicate of this, it doesn't appeared to be fixed after comment 43 .
I tried 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008032505 Minefield/3.0b5pre

Comment 51

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008032704 Minefield/3.0pre

Comment 52

[Robert Sayre]

(In reply to comment #46)
> 
> I always mix this up, but doesn't QueryInterface throw if it fails?

Yeah, I always mess that up too. Fixed in the patch for bug 423060.