yahoo rss newsfeed (de.news.yahoo.com) is not previewed properly

VERIFIED FIXED in Firefox 3

Status

()

Firefox
RSS Discovery and Preview
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: xtc4uall, Assigned: Will Guaraldi)

Tracking

({regression})

Trunk
Firefox 3
x86
Windows XP
regression
Points:
---
Bug Flags:
blocking-firefox3 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
STRs:
- open feed url
- notice that <title>, <description> & <image> are shown, but not the feed <item>s.

for another yahoo feed http://de.news.yahoo.com/politik/ausland.html.xml only the first three items are previewed.

in both cases adding the feed as a live bookmark makes loading all feed items properly though.

fails in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041406 Firefox/3.0.0 ID:2008041406 (new profile, no extensions etc.)

works in Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9b5) Gecko/2008032620 Firefox/3.0b5 ID:2008032620 (new profile, no extensions etc.)

w3c validator (just) has 2 recommendations:

# line 13, column 34: Image title doesn't match channel title
 <title>Yahoo! Nachrichten DE</title>
                             ^
# line 133, column 2: Missing atom:link with rel="self"
 </channel>
 ^

help with narrowing down the regression window is appreciated.
With that range and the error console complaining about:

Error: uncaught exception: [Exception... "'[JavaScript Error: "url is null" {file: "file:///C:/Program%20Files/Minefield/components/FeedWriter.js" line: 509}]' when calling method: [nsIFeedWriter::writeContent]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://browser/content/feeds/subscribe.js :: SH_writeContent :: line 51"  data: yes]

and the feed starting the first four entries with "<media:content url="" height="" width=""></media:content>" and bug 426223 changing the way we look at null media:content url attributes (from !getProperty() to !bagHasKey(), which may or may not be the problem, I don't actually know), I'm betting your regression range will be right around 2008-04-04 02:13.
Blocks: 426223
Flags: blocking-firefox3?
(Assignee)

Comment 2

10 years ago
The FeedProcessor shouldn't be letting any enclosures that lack a url through because it's not useful to show things like that.  It sounds like either the media:content items aren't getting checked or my check sucks.

I'll add a test and try to get it fixed by tomorrow.
Status: NEW → ASSIGNED
(Assignee)

Updated

10 years ago
Assignee: nobody → will.guaraldi
Status: ASSIGNED → NEW
(Assignee)

Comment 3

10 years ago
Created attachment 315688 [details] [diff] [review]
patch to fix 429049

This is a one-line change that forces enclosures to have a non-empty url in order to add them to the enclosures array which the FeedWriter displays to the user.  I tossed in a comment regarding the restriction.

The example feed works properly now displaying enclosed media items where media:content tags exist.

Also included is a test case.  All existing tests continue to pass.

Asking sayrer to review since it's a change in the FeedProcessor.
Attachment #315688 - Flags: review?(sayrer)
Flags: blocking-firefox3? → blocking-firefox3+

Updated

10 years ago
Whiteboard: [has patch][needs review sayrer]
(Reporter)

Comment 4

10 years ago
for what it's worth: i manually edited my local FeedProcessor.js with the patch' change and above mentioned feeds are previewed correctly now. no regressions seen with my other ~50 feeds.
Comment on attachment 315688 [details] [diff] [review]
patch to fix 429049

sayrer seems to be overlooking this, but it looks good to me, and the test seems to be correct.
Attachment #315688 - Flags: review?(sayrer)
Attachment #315688 - Flags: review+
Attachment #315688 - Flags: approval1.9+

Updated

10 years ago
Keywords: checkin-needed
Whiteboard: [has patch][needs review sayrer] → [has patch][has reviews][has approval]
mozilla/toolkit/components/feeds/src/FeedProcessor.js 	1.37
mozilla/toolkit/components/feeds/test/xml/rss2/mrss_content_429049.xml 	1.1 
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews][has approval]
Target Milestone: --- → Firefox 3
(Reporter)

Comment 7

10 years ago
VERIFIED FIXED with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042705 Firefox/3.0.0 ID:2008042705

thanks Will :)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.