The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Thunderbird 24.0

Status

MailNews Core
Feed Reader
--
major
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: M Prindle, Assigned: alta88)

Tracking

unspecified
Thunderbird 24.0

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

3.70 KB, patch
alta88
: review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
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
(Assignee)

Comment 1

5 years ago
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)

Comment 2

4 years ago
Created attachment 751366 [details] [diff] [review]
patch
Assignee: nobody → alta88
Attachment #751366 - Flags: review?(mkmelin+mozilla)

Comment 3

4 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)
(Assignee)

Comment 4

4 years ago
(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

4 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+
(Assignee)

Comment 6

4 years ago
Created attachment 753281 [details] [diff] [review]
updated


yes, better to be stricter.  converted to a function as well.
Attachment #751366 - Attachment is obsolete: true
Attachment #753281 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/de4183a854a0
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 24.0
(Reporter)

Comment 8

4 years ago
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.