Last Comment Bug 790554 - feed-parser.js raises an error with buggy RSS feeds; Error: linkElements is null
: feed-parser.js raises an error with buggy RSS feeds; Error: linkElements is null
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Feed Reader (show other bugs)
: 15
: All All
: -- normal (vote)
: Thunderbird 18.0
Assigned To: alta88
:
:
Mentors:
http://www.google.com/trends/hottrend...
: 792657 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-12 01:51 PDT by Yoshihisa Kawamoto
Modified: 2012-09-25 13:24 PDT (History)
7 users (show)
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
feed-parser.js.patch (339 bytes, patch)
2012-09-12 01:51 PDT, Yoshihisa Kawamoto
no flags Details | Diff | Splinter Review
patch (1.86 KB, patch)
2012-09-12 09:53 PDT, alta88
mconley: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Yoshihisa Kawamoto 2012-09-12 01:51:48 PDT
Created attachment 660355 [details] [diff] [review]
feed-parser.js.patch

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:15.0) Gecko/20100101 Firefox/15.0.1
Build ID: 20120905151427

Steps to reproduce:

Following feed URL does not work anymore. There is a RSS feed of the Google Hot Trends in my News Feeds subscription list:
http://www.google.com/trends/hottrends/atom/hourly
This feed URL is not legitimate, invalid, or buggy, but very useful. And this URL worked well up to Thunderbird 14.


Actual results:

When I update the feed (File -> Get New Messages for -> Blogs & News Feeds), a following error is raised in the Error Console:
Timestamp: 9/12/12 5:42:14 PM
Error: linkElements is null
Source File: chrome://messenger-newsblog/content/feed-parser.js
Line: 597

This may be caused by the buggy URL, but we should accept various feed URLs gently.


Expected results:

Hourly Google Hot Trends top 10 should come! This bug will be fixed by the following patch. Enjoy!
Comment 1 :aceman 2012-09-12 08:00:01 PDT
alta88 can you look at the patch what it is trying to accomplish? And it isn't in the proper format yet.

TB18 refuses to add this feed in the Subscription dialog.
Comment 2 Philip Chee 2012-09-12 09:24:24 PDT
Confirming on trunk:

Thu Sep 13 2012 00:22:12
Error: linkElements is null
Source file: chrome://messenger-newsblog/content/feed-parser.js
Line: 635
Comment 3 alta88 2012-09-12 09:53:00 PDT
Created attachment 660489 [details] [diff] [review]
patch


thanks kawamoto, but the patch isn't the right way to avoid the lack of check.  returning/checking for null is inherent to other parsing logic.

fix included.  it may be a year before it's released though.
Comment 4 Yoshihisa Kawamoto 2012-09-13 08:34:24 PDT
Hi alta88, thank you for the fix. It's OK for me about release date.

Thanks again,
Yoshihisa Kawamoto
Comment 5 :aceman 2012-09-13 10:34:14 PDT
Yoshihisa Kawamoto, please do not close the bug, the correct patch was not yet accepted and merged into Thunderbird code.
Comment 6 Yoshihisa Kawamoto 2012-09-13 17:26:34 PDT
Oh, I'm sorry for that.
Comment 7 Mike Conley (:mconley) - (needinfo me!) 2012-09-17 13:38:59 PDT
Comment on attachment 660489 [details] [diff] [review]
patch

Review of attachment 660489 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Thanks!
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-09-19 17:18:09 PDT
https://hg.mozilla.org/comm-central/rev/8fdbcb519211
Comment 9 :aceman 2012-09-20 00:01:32 PDT
See also 792657.

If this is really a regression starting at TB15, would it be worth it to push into TB16/17 quickly?
Comment 10 Ludovic Hirlimann [:Usul] 2012-09-20 00:17:31 PDT
*** Bug 792657 has been marked as a duplicate of this bug. ***
Comment 11 alta88 2012-09-20 07:14:03 PDT
Comment on attachment 660489 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: certain feed subscribes fail.
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky):  zero.

Note You need to log in before you can comment on or make changes to this bug.