Closed Bug 562227 Opened 15 years ago Closed 12 years ago

When using the URL referenced the feed gets cut into two pieces making the feed invalid

Categories

(MailNews Core :: Feed Reader, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 24.0

People

(Reporter: mprindle, Assigned: alta88)

References

()

Details

Attachments

(1 file, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.9) Gecko/20100317 Lightning/1.0b1 Thunderbird/3.0.4 When I add this url "http://houston.craigslist.org/search/sss?query=google+nexus+(N1|one)+-(broken)+-(NOT+CHINA+KNOCK+OFF)+-(832-531-5184)+-(Dr+Cell+Phone)&format=rss" into my RSS feed TB will check the feed once, but when I close and reopen TB the feed is cut into two entries. The first entry is "http://houston.craigslist.org/search/sss?query=google+nexus+(N1" and the 2nd entry is "one)+-(broken)+-(NOT+CHINA+KNOCK+OFF)+-(832-531-5184)+-(Dr+Cell+Phone)&format=rss". It appears the | in the feed is breaking the url in TB. On craigslist the | is use to OR a statement together. The feeds are structured like this Craigslist (Folder) - Feed 1 - Feed 2 - Feed 3 Also, as a part of this bug once the feed is broken in to two pieces I can no longer delete the feed. I have to go to "C:\Users\username\AppData\Roaming\Thunderbird\Profiles\ve5l8jty.user\Mail\News & Blogs" and delete everything in the folder. I'm sure there's a way to figure out what file to delete, but I'm unable to figure it out. Reproducible: Always Steps to Reproduce: 1.Subscribe to the feed referenced above 2.Close TB 3.Open TB and the feed is broke Actual Results: 1.Feed subscribes and updates once 2. After open & close the feed is broke Expected Results: 1.Subscribe the feed 2.Open TB and the feed updates
yes, Tb uses | as a delimiter for multiple feeds subscribed to a folder.
Status: UNCONFIRMED → NEW
Component: Folder and Message Lists → Feed Reader
Ever confirmed: true
OS: Windows 7 → All
Product: Thunderbird → MailNews Core
QA Contact: folders-message-lists → feed.reader
Hardware: x86 → All
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → alta88
Attachment #751366 - Flags: review?(mkmelin+mozilla)
Comment on attachment 751366 [details] [diff] [review] patch Review of attachment 751366 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/extensions/newsblog/content/utils.js @@ +270,5 @@ > let feedurls = aFolder.getStringProperty("feedUrl"); > + if (feedurls) { > + feedurls = feedurls.replace(this.kFeedUrlDelimiter + "http", > + "\x01http", "g"); > + feedurls = feedurls.replace(this.kFeedUrlDelimiter + "file://", we don't allow file:// feeds, at least yet. so you should be able to simply split on "|http", no? that should also be forward-compatible (for downgrade scenarios) @@ +395,5 @@ > + "\x01file://", "g"); > + curFeedUrls = curFeedUrls.split("\x01"); > + } > + else > + curFeedUrls = []; if one if-else has braces, they all should is the style rule. (though if yo split on "|http" you don't need an if-else at all)
(In reply to Magnus Melin from comment #3) > Comment on attachment 751366 [details] [diff] [review] > patch > > Review of attachment 751366 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mailnews/extensions/newsblog/content/utils.js > @@ +270,5 @@ > > let feedurls = aFolder.getStringProperty("feedUrl"); > > + if (feedurls) { > > + feedurls = feedurls.replace(this.kFeedUrlDelimiter + "http", > > + "\x01http", "g"); > > + feedurls = feedurls.replace(this.kFeedUrlDelimiter + "file://", > > we don't allow file:// feeds, at least yet. > so you should be able to simply split on "|http", no? that should also be > forward-compatible (for downgrade scenarios) > yes, but it is possible to override the isValidScheme function, which i do locally and it's pretty important for testing, so i'd like to have this handled up front. > @@ +395,5 @@ > > + "\x01file://", "g"); > > + curFeedUrls = curFeedUrls.split("\x01"); > > + } > > + else > > + curFeedUrls = []; > > if one if-else has braces, they all should is the style rule. > > (though if yo split on "|http" you don't need an if-else at all) ok. (that's how i had it initially but needed to include file://.)
Comment on attachment 751366 [details] [diff] [review] patch Review of attachment 751366 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/extensions/newsblog/content/utils.js @@ +389,5 @@ > > let curFeedUrls = aFolder.getStringProperty("feedUrl"); > + if (curFeedUrls) { > + curFeedUrls = curFeedUrls.replace(this.kFeedUrlDelimiter + "http", > + "\x01http", "g"); Maybe do this for http:// and https:// cases separately? For edge cases.
Attachment #751366 - Flags: review?(mkmelin+mozilla) → review+
Attached patch updatedSplinter Review
yes, better to be stricter. converted to a function as well.
Attachment #751366 - Attachment is obsolete: true
Attachment #753281 - Flags: review+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 24.0
Thanks for getting this bug fixed. I can now go back to using the RSS reader in TB.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: