Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfixes #644

Merged
merged 9 commits into from May 15, 2016
Merged

Bugfixes #644

merged 9 commits into from May 15, 2016

Conversation

barijaona
Copy link
Member

The change of row selection is already calling the -tableViewSelectionDidChange: delegate, which performs its own update.
This fixes an update problem when skipping from a folder which loads full HTML pages to another one which doesn't.
- solve an issue with URL strings provided by Flickr
- add an assertion
- fix comments
@@ -202,6 +202,7 @@ -(IBAction)doSubscribe:(id)sender
// Validate the subscription, possibly replacing the feedURLString with a real one if
// it originally pointed to a web page.
rssFeedURL = [subscriptionModel verifiedFeedURLFromURL:rssFeedURL];
NSAssert(rssFeedURL != nil, @"No valid URL verified to attempt subscription !");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This always causes an assertion failure as rssFeedURL is basically always != nil.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures rssFeedURL is not nil.
It is aimed at detecting problems in SubscriptionModel’s -verifiedFeedURLFromURL:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it always crashes the program whether or not a valid URL has been entered.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not encountered that problem.
According to the doc, NSAssert(condition, desc, ...) will fail if condition evaluates to NO.
A similar use case here : https://github.com/ViennaRSS/vienna-rss/blob/master/src/ArticleController.m#L336

Copy link
Member

@josh64x2 josh64x2 May 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try this valid feed: https://feeds.feedburner.com/UnifiedArsTechnicaFeed?format=xml
It's always set to nil for me and therefore triggers the assertion failure.

EDIT: OK I see now the issue is in SubscriptionModel, as this feed url works: http://www.abc.net.au/news/feed/51120/rss.xml

}
return rssFeedURL.absoluteURL;
return myURL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems to be what is causing the "new" issue. If myURL isn't set because extractFeeds returns NO, then we get nil returned even if the original URL was OK.

@barijaona
Copy link
Member Author

Commit added.

@josh64x2
Copy link
Member

great, thanks!

@josh64x2 josh64x2 merged commit 5747847 into ViennaRSS:master May 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants