350365 - Malformed feeds look empty

Status: RESOLVED WONTFIX

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

Description

[Nickolay_Ponomarev]

When you open a malformed feed, you get an error from here: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/feeds/src/FeedProcessor.js&rev=1.13&mark=1177#1176
reported to Error console, and the displayed feed page looks broken. There should be a user-visible error message displayed instead.

Comment 1

[Nickolay_Ponomarev]

Attachment: An example malformed feed (from DevNews)

Comment 2

[Robert Sayre]

IE7 refuses to display this, good for them. We shouldn't have JS Console errors here. It's up to management to decide whether we want to show a partial feed or throw our hands up similarly.

Comment 3

[Robert Sayre]

Mike, could we get an opinion on what to do here? We have a reliable indication that the feed has fatal errors.

Comment 4

[Robert Sayre]

Attachment: set the XMLReader to null in OnStopRequest, so we keep consuming events after a fatal error

Comment 5

[Peter van der Woude [:Peter6]]

Upping to MAJOR.

It is plain bad if we leave a user staring at an empty prettyprint page without informing him the feed broke.

Comment 6

[Phil Ringnalda (:philor)]

*** Bug 363490 has been marked as a duplicate of this bug. ***

Comment 8

[Marcia Knous [:marcia]]

Robert: Is there a reason your patch did not get checked in? I would err on the side of displaying something letting the user know that the feed is malformed.

Comment 9

[Robert Sayre]

(In reply to comment #8)
> Robert: Is there a reason your patch did not get checked in? I would err on the
> side of displaying something letting the user know that the feed is malformed.

Yeah, this needs to get fixed. The patch is checked in, it just makes the situation not as bad.

Comment 11

[sendmail.to]

(In reply to comment #9)
> (In reply to comment #8)
> > Robert: Is there a reason your patch did not get checked in? I would err on the
> > side of displaying something letting the user know that the feed is malformed.
> 
> Yeah, this needs to get fixed. The patch is checked in, it just makes the
> situation not as bad.
> 

As mentioned in Bug 381799, we need to fix this single line.
http://lxr.mozilla.org/mozilla1.8/source/browser/components/feeds/content/subscribe.xhtml#47

From my point of view this are two bugs, fixing the js console error and
fixing the user interface. I suggest to reopen 381799 for interface change and using this to resolve js console issues.

Comment 12

[Simon Bünzli]

Seems that we simply never reach the following line which would remove the style="display: none;" mentioned by comment #11:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/feeds/src/FeedWriter.js&rev=1.2.2.31&mark=366-370#343

Fixing the error from comment #0 might/should already make this happen.

Comment 13

[Robert Sayre]

This will need l10n strings, most likely. Assigning to beltzner or his delegate, so we can figure out what should happen for feeds we can't parse. IE7 and Safari both show "we give up" screens.

Comment 14

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

Blocking (and tackling!)

Comment 15

[Rene Didion]

When adding http://news.hereisthecity.com/news/business_news_rss091.xml Live Bookmark tells me the feed was empty.

Since the feed is working properly with other RSS-readers I was wondering whether it is part of this bug.

PS: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a7pre) Gecko/2007071711 Minefield/3.0a7pre

Comment 16

[Brian Polidoro]

(In reply to comment #15)
By the error I get in the console that looks like bug 381357.  

Comment 17

[Mike Connor [:mconnor]]

Alex, can you pull together some sort of UI for this failure state?  Not a feature, doesn't need to make M8

Comment 19

[Alex Faaborg [:faaborg] (Firefox UX)]

I'll work on the UI for this after M8

Comment 22

[Phil Ringnalda (:philor)]

Taking off the "this._reader has no properties" since it's no longer true, and aligning the milestone with the reality that we didn't choose a few words of text before freezing.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Any update on this bug? Alex?

Comment 25

[Alex Faaborg [:faaborg] (Firefox UX)]

This fell off my radar, not sure if we can fix it given the string freeze, but I'll get the text ready so we can fix this on trunk.

Comment 26

[Tanner M. Young [:tmyoung]]

Alex, what is the status on this?

Comment 27

[Alex Faaborg [:faaborg] (Firefox UX)]

in the middle of a unfortunately very large uiwanted queue.  Just put up an error icon (the red one with the X), and some temporary text, and we can clean the text up in ui-review.

Comment 28

[Marco Bonardo [:mak]]

The feed preview code has been removed by bug 1477669.