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)
MailNews Core
Feed Reader
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 24.0
People
(Reporter: mprindle, Assigned: alta88)
References
()
Details
Attachments
(1 file, 1 obsolete file)
3.70 KB,
patch
|
alta88
:
review+
|
Details | Diff | Splinter Review |
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
Assignee: nobody → alta88
Attachment #751366 -
Flags: review?(mkmelin+mozilla)
Comment 3•12 years ago
|
||
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 5•12 years ago
|
||
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+
yes, better to be stricter. converted to a function as well.
Attachment #751366 -
Attachment is obsolete: true
Attachment #753281 -
Flags: review+
Keywords: checkin-needed
Comment 7•12 years ago
|
||
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.
Description
•