Skip Menu |

This queue is for tickets about the XML-RSS CPAN distribution.

Report information
The Basics
Id: 11415
Status: resolved
Priority: 0/
Queue: XML-RSS

People
Owner: Nobody in particular
Requestors:
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 1.05
Fixed in: 1.20



Subject: Can't create "0" values in optional items
Optional items can't be added to RSS/RDF files if the values are 0. For examples, "$self->{channel}->{'dc'}->{'creator'}" at line 697 in RSS.pm(XML-RSS-1.05), if the value is 0, it can't be added to RSS/RDF. I've tried making the patch to escape the problems, and attached it.

Message body is not shown because it is too large.

[guest - Mon Feb 7 23:20:22 2005]: Show quoted text
> I've tried making the patch to escape the problems, and attached it.
I updated the patch to correct a typo.

Message body is not shown because it is too large.

On Tue Feb 08 00:12:21 2005, guest wrote: Show quoted text
> [guest - Mon Feb 7 23:20:22 2005]:
> > I've tried making the patch to escape the problems, and attached it.
> > I updated the patch to correct a typo.
Hi, Could you make a test case or two for this problem? In the patch, instead of using 'ne ""' you should test for "defined $...." or "exists ....". - ask
Subject: Started reworked version of the patch
On Sat Mar 11 18:21:42 2006, ABH wrote: Show quoted text
> On Tue Feb 08 00:12:21 2005, guest wrote:
> > [guest - Mon Feb 7 23:20:22 2005]:
> > > I've tried making the patch to escape the problems, and attached
> it.
> > > > I updated the patch to correct a typo.
> > Hi, > > Could you make a test case or two for this problem? > > In the patch, instead of using 'ne ""' you should test for "defined > $...." or "exists ....". >
Hi! I started the reworked version of the patch by using defined() instead of qq{ne ""} while adding regression tests before fixing the code. The little I have so far can be found in the attached patch. I'll continue to work on it. Regards, Shlomi Fish Show quoted text
> - ask
=== t/items-are-0.t ================================================================== --- t/items-are-0.t (/local/XML-RSS/tags/before-work-on-items-that-are-0) (revision 346) +++ t/items-are-0.t (/local/XML-RSS/local-trunk) (revision 346) @@ -0,0 +1,64 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use Test::More tests => 4; + +use XML::RSS; + +sub create_rss_1 +{ + my $args = shift; + # my $rss = new XML::RSS (version => '0.9'); + my $rss = new XML::RSS (version => $args->{version}); + + $rss->channel( + title => "freshmeat.net", + link => "http://freshmeat.net", + description => "the one-stop-shop for all your Linux software needs", + ); + + $rss->image( + title => "freshmeat.net", + url => "0", + link => "http://freshmeat.net/" + ); + + $rss->add_item( + title => "GTKeyboard 0.85", + link => "http://freshmeat.net/news/1999/06/21/930003829.html" + ); + + return $rss; +} + +{ + my $rss = create_rss_1({version => "0.9"}); + # TEST + like ($rss->as_string, qr{<image>.*?<title>freshmeat.net</title>.*?<url>0</url>.*?<link>http://freshmeat.net/</link>.*?</image>}s, + "Checking for image in RSS 0.9"); +} + +{ + my $rss = create_rss_1({version => "0.91"}); + # TEST + like ($rss->as_string, qr{<image>.*?<title>freshmeat.net</title>.*?<url>0</url>.*?<link>http://freshmeat.net/</link>.*?</image>}s, + "Checking for image in RSS 0.9.1"); +} + +# TODO : +# Test other elements within the <image> tag for zero-ness. +{ + my $rss = create_rss_1({version => "1.0"}); + # TEST + like ($rss->as_string, qr{<image rdf:about="0">.*?<title>freshmeat.net</title>.*?<url>0</url>.*?<link>http://freshmeat.net/</link>.*?</image>}s, + "Checking for image in RSS 1.0"); +} + +{ + my $rss = create_rss_1({version => "2.0"}); + # TEST + like ($rss->as_string, qr{<image>.*?<title>freshmeat.net</title>.*?<url>0</url>.*?<link>http://freshmeat.net/</link>.*?</image>}s, + "Checking for image in RSS 2.0"); +} === t/test_manifest ================================================================== --- t/test_manifest (/local/XML-RSS/tags/before-work-on-items-that-are-0) (revision 346) +++ t/test_manifest (/local/XML-RSS/local-trunk) (revision 346) @@ -12,4 +12,5 @@ encode-output.t enclosures.t auto_add_modules.t +items-are-0.t rss2-gt-encoding.t === MANIFEST ================================================================== --- MANIFEST (/local/XML-RSS/tags/before-work-on-items-that-are-0) (revision 346) +++ MANIFEST (/local/XML-RSS/local-trunk) (revision 346) @@ -44,6 +44,7 @@ t/enclosures.t t/encode-output.t t/encoding.t +t/items-are-0.t t/load.t t/pod.t t/rss2-gt-encoding.t === lib/XML/RSS.pm ================================================================== --- lib/XML/RSS.pm (/local/XML-RSS/tags/before-work-on-items-that-are-0) (revision 346) +++ lib/XML/RSS.pm (/local/XML-RSS/local-trunk) (revision 346) @@ -632,7 +632,7 @@ ################# # image element # ################# - if ($self->{image}->{url}) { + if (defined($self->{image}->{url})) { $output .= '<image>'."\n"; # title @@ -757,7 +757,7 @@ ################# # image element # ################# - if ($self->{image}->{url}) { + if (defined($self->{image}->{url})) { $output .= '<image>'."\n"; # title @@ -953,9 +953,9 @@ if ( exists( $rdf_resource_fields{ $url } ) and exists( $rdf_resource_fields{ $url }{ $el }) ) { - $output .= qq!<$prefix:$el rdf:resource="! . + $output .= qq{<$prefix:$el rdf:resource="} . $self->encode($value) . - qq!" />\n!; + qq{" />\n}; } else { $output .= "<$prefix:$el>". $self->encode($value) ."</$prefix:$el>\n"; @@ -985,7 +985,7 @@ ################# # image element # ################# - if ($self->{image}->{url}) { + if (defined($self->{image}->{url})) { $output .= '<image rdf:about="'. $self->encode($self->{image}->{url}) .'">'."\n"; # title @@ -1023,9 +1023,9 @@ if ( exists( $rdf_resource_fields{ $url } ) and exists( $rdf_resource_fields{ $url }{ $el }) ) { - $output .= qq!<$prefix:$el rdf:resource="! . + $output .= qq{<$prefix:$el rdf:resource="} . $self->encode($value) . - qq!" />\n!; + qq{" />\n}; } else { $output .= "<$prefix:$el>". $self->encode($value) ."</$prefix:$el>\n"; @@ -1212,7 +1212,7 @@ ################# # image element # ################# - if ($self->{image}->{url}) { + if (defined($self->{image}->{url})) { $output .= '<image>'."\n"; # title
From: SHLOMIF [...] cpan.org
On Fri Oct 20 12:58:02 2006, SHLOMIF wrote: Show quoted text
> On Sat Mar 11 18:21:42 2006, ABH wrote:
> > On Tue Feb 08 00:12:21 2005, guest wrote:
> > > [guest - Mon Feb 7 23:20:22 2005]:
> > > > I've tried making the patch to escape the problems, and
attached Show quoted text
> > it.
> > > > > > I updated the patch to correct a typo.
> > > > Hi, > > > > Could you make a test case or two for this problem? > > > > In the patch, instead of using 'ne ""' you should test
for "defined Show quoted text
> > $...." or "exists ....". > >
> > Hi! > > I started the reworked version of the patch by using defined()
instead Show quoted text
> of qq{ne ""} while adding regression tests before fixing the code.
The Show quoted text
> little I have so far can be found in the attached patch. I'll
continue Show quoted text
> to work on it. >
This is the second revision of this patch which is more complete. It covers the <image> and <item> tags except for some dc: and other modules sub-elements. Some other necessary changes: 1. I refactored the code to some extent. 2. I added a "runtest" and "distruntest" targets to the makefile to run the tests using Test::Run, so I could get the summary line coloured depending whether it's successful or not. It doesn't disturb the rest of the makefile, and you can omit the Makefile.PL changes from the patch. Regards, Shlomi Fish Show quoted text
> Regards, > > Shlomi Fish

Message body is not shown because it is too large.

From: SHLOMIF [...] cpan.org
On Sun Oct 22 15:52:44 2006, SHLOMIF wrote: Show quoted text
> On Fri Oct 20 12:58:02 2006, SHLOMIF wrote:
> > On Sat Mar 11 18:21:42 2006, ABH wrote:
> > > On Tue Feb 08 00:12:21 2005, guest wrote:
> > > > [guest - Mon Feb 7 23:20:22 2005]:
> > > > > I've tried making the patch to escape the problems, and
> attached
> > > it.
> > > > > > > > I updated the patch to correct a typo.
> > > > > > Hi, > > > > > > Could you make a test case or two for this problem? > > > > > > In the patch, instead of using 'ne ""' you should test
> for "defined
> > > $...." or "exists ....". > > >
> > > > Hi! > > > > I started the reworked version of the patch by using defined()
> instead
> > of qq{ne ""} while adding regression tests before fixing the code.
> The
> > little I have so far can be found in the attached patch. I'll
> continue
> > to work on it. > >
> > This is the second revision of this patch which is more complete. It > covers the <image> and <item> tags except for some dc: and other > modules sub-elements. > > Some other necessary changes: > > 1. I refactored the code to some extent. > > 2. I added a "runtest" and "distruntest" targets to the makefile to > run the tests using Test::Run, so I could get the summary line > coloured depending whether it's successful or not. It doesn't
disturb Show quoted text
> the rest of the makefile, and you can omit the Makefile.PL changes > from the patch. >
The attached patch fixes all the cases of this. It was a labour of love. :-) The only omission are the date bugs, because date conversion is more complicated. Regards, Shlomi Fish

Message body is not shown because it is too large.