search

modolabs / Kurogo-Mobile-Web

Kurogo is a PHP framework for delivering high quality, data driven customizable content to a wide range of mobile devices. Its strengths lie in the customizable system that allows you to adapt content from a variety of sources and easily present that to mobile devices from feature phones, to early generation smart phones, to modern devices and tablets
http://kurogo.org
GNU Lesser General Public License v2.1
198 stars 99 forks source link

New module issues in 1.2 #24

Closed bpayst closed 13 years ago

bpayst commented 13 years ago

I just updated to 1.2 and several feeds that worked in 1.1 no longer work.

This feed: http://global.unc.edu/rss_newscenter.php

returns:

XML error: Undeclared entity error at line 14

Fatal error: Uncaught exception 'Exception' with message 'XML error: Undeclared entity error at line 14' in /var/www/mobile/lib/XMLDataParser.php:68 Stack trace:

0 /var/www/mobile/lib/DataController.php(335): parseData(string)

1 /var/www/mobile/lib/DataController.php(371): parseData(string, object)

2 /var/www/mobile/lib/RSSDataController.php(58): getParsedData()

3 /var/www/mobile/app/modules/news/NewsWebModule.php(301): items(integer, string)

4 /var/www/mobile/lib/WebModule.php(1201): initializeForPage()

5 /var/www/mobile/lib/WebModule.php(1292): setPageVariables()

6 /var/www/mobile/www/index.php(271): displayPage()

7 {main}

thrown in /var/www/mobile/lib/XMLDataParser.php on line 68

These feeds do not load in the display pane (stories list does load) in the tablet UI, they do work in the iPhone and desktop views:

http://uncnews.unc.edu/component/option,com_rss/feed,RSS2.0/no_html,1/ http://www.med.unc.edu/www/news/aggregator/RSS

akinspe commented 13 years ago

This issue is not a 1.2 issue. It is present in 1.1 as well. It was just exposed by a recent change to your feed. The tablet pane only loads the primary feed at this time. This is a known limitation. Since it's an independent issue, could you please crete a a new issue about that. I will update you on the progress of detecting, and dealing with the XML parsing error.

akinspe commented 13 years ago

The problem is that your feeds are producing invalid XML. Those description/title tags should be wrapped in <![CDATA[ ]]> tags. We can look to see what we can do to mitigate that, but there are limits to dealing with data that violates specs.

bpayst commented 13 years ago

well, they're not technically my feeds (all mine are valid). Still, they do validate as RSS using the W3 validator:

http://feed1.w3.org/check.cgi?url=http%3A//uncnews.unc.edu/component/option%2Ccom_rss/feed%2CRSS2.0/no_html%2C1/

http://feed2.w3.org/check.cgi?url=http%3A//www.med.unc.edu/www/news/aggregator/RSS

Other feed readers handle them fine. I understand the limits of data that violates specs, but these don't really seem to in this particular use case.

akinspe commented 13 years ago

Sorry. We're conflating 2 distinct issues:

1 This feed: http://global.unc.edu/rss_newscenter.php is not valid. At this point it seems to be the only invalid one. My apologies for using plural.

http://feed1.w3.org/check.cgi?url=http%3A%2F%2Fglobal.unc.edu%2Frss_newscenter.php It will take some interesting heuristics to deal with include HTML entities not wrapped in CDATA in a performant way.

2 The tablet interface only shows the first feed on the home screen. There is not currently a way to show the other feeds. Hopefully this will be addressed in the future.

bpayst commented 13 years ago

My fault should have put them in as different issues. Yes, that global feed is crap at the moment. You can ignore that. It's invalid and should error out. I'll just pull the feed.

The tablet interface issue is not on the home screen. It's on the page for that feed. For example http://m.unc.edu/news/?section=8 on a tablet shows stories, but the full detail pane on the right just spins and never loads.

akinspe commented 13 years ago

OK. I know what's happening. We do not handle cases where there is no GUID in the feed very well.

Short story: the url that's given is not a Kurogo url, but rather the direct link to the article. This is bad (and Kurogo's fault)

Longer story: The AJAX code attempts to load that URL in the detail page (instead of the url that shows the story detail) and of course that doesn't work because of cross domain policy.

This will require a fix. Probably a way to generate GUIDs when they aren't present in the feed. It isn't an issue for non-tablets since they just open the url anyway (no ajax).

bpayst commented 13 years ago

Yes, that would certainly explain it. Having worked parsing feeds a bit myself in the past I know the exceptions tend to be much greater than those that follow the rules.

Happy to test a fix when it is available.