Ticket #2067 (new bug)

Opened 5 years ago

Last modified 5 years ago

failed to load feed from http://www.instedd.org/

Reported by: fredj Owned by: crschmidt
Priority: minor Milestone: 2.13 Release
Component: Layer.GeoRSS Version: SVN
Keywords: Cc:
State: Needs More Work

Description

georss from instedd.org throw an error when loaded with Layer.GeoRSS (in the feed title parsing part). Because the first title element is <title />.

The attached patch make the feed title parsing more robust.

georss example:  http://riff.instedd.org/GeoRSSFeed.aspx?social=Swineinfluenza&count=30

Attachments

2067.0.patch Download (4.6 KB) - added by fredj 5 years ago.
Don't crash when parsing feed title, with tests. Tests pass on FF 3.0.10
2067.1.patch Download (4.6 KB) - added by fredj 5 years ago.
Add a new test (count the features) to show the issue.

Change History

Changed 5 years ago by fredj

Don't crash when parsing feed title, with tests. Tests pass on FF 3.0.10

  Changed 5 years ago by fredj

  • state set to Review

Please review

follow-up: ↓ 3   Changed 5 years ago by bartvde

  • state changed from Review to Needs Discussion

fredj, your testcase does not fail without the change to Layer/GeoRRS, both in FF 3.5.5 as in Safari. Can you explain that? Was this browser specific (IE?). Or has something changed in trunk which makes this change unnecessary?

in reply to: ↑ 2   Changed 5 years ago by fredj

Replying to bartvde:

fredj, your testcase does not fail without the change to Layer/GeoRRS, both in FF 3.5.5 as in Safari. Can you explain that? Was this browser specific (IE?). Or has something changed in trunk which makes this change unnecessary?

You're right, the test pass without the change but there is an uncaught exception at line 147. This error is not reported and stops the execution of parseData(): the features are not loaded, etc ...

Changed 5 years ago by fredj

Add a new test (count the features) to show the issue.

  Changed 5 years ago by fredj

  • state changed from Needs Discussion to Review

  Changed 5 years ago by bartvde

  • state changed from Review to Needs More Work

Thanks for the update fredj. The second test fails for me in IE6 though.

fail features count is correct. eq:  values differ: got 0, but expected 3

Also, is it necessary to explicitly return null, since OpenLayers.Util.Try will already return null (If none executes successfully, returns null.).

  Changed 5 years ago by fredj

You're right about the null value, I'll change it.

The test also fails for IE7 but I suspect the georss to be invalid (or the IE parser buggy), the error is not related to the title part.

Note: See TracTickets for help on using tickets.