Bug #42114 for CGI: Needs Test, Patch: CGI::Pretty should not put line breaks after A elements

Sat Jan 03 03:54:14 2009 rrt [...] sc3d.org - Ticket created

Subject:

CGI::Pretty should not put line breaks after A elements

CGI::Pretty inserts line breaks after A elements, despite the man page claiming that it does not format them. This effectively inserts a space, breaking formatting. Although the version of CGI.pm on my system is 3.29, which is not the latest, CGI::Pretty is version 1.08, which does seem to be the latest. I got my CGI::Pretty with my Perl, which is 5.10.0.

Mon Jan 05 10:56:24 2009 LDS [...] cpan.org - Correspondence added

On Sat Jan 03 03:54:14 2009, rrt wrote: Show quoted text

> CGI::Pretty inserts line breaks after A elements, despite the man page > claiming that it does not format them. This effectively inserts a space, > breaking formatting. > > Although the version of CGI.pm on my system is 3.29, which is not the > latest, CGI::Pretty is version 1.08, which does seem to be the latest. I > got my CGI::Pretty with my Perl, which is 5.10.0.

I am unable to confirm this under the current version. Could you post a short script that demonstrates the problem?

Mon Jan 05 10:56:25 2009 The RT System itself - Status changed from 'new' to 'open'

Mon Jan 05 10:56:26 2009 LDS [...] cpan.org - Given to LDS

Mon Jan 05 14:35:14 2009 rrt [...] sc3d.org - Correspondence added

On Mon Jan 05 10:56:24 2009, LDS wrote: Show quoted text

> I am unable to confirm this under the current version. Could you post > a short script that demonstrates the problem?

Sure. The following script: #! /usr/bin/perl use CGI qw/:standard/; use CGI::Pretty; print header . start_html("hello_word"), "some text" . a({-href => "http://www.example.com/"}, "example link") . "some more text", end_html; produces the following output for me: Content-Type: text/html; charset=ISO-8859-1 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html xmlns="http://www.w3.org/1999/xhtml" lang="en-US" xml:lang="en-US"> <head> <title>hello_word</title> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" /> </head> <body> some text<a href="http://www.example.com/">example link</a> some more text </body> </html> Note the line break after </a>, which introduces a space when rendered.

Sat Jul 25 17:00:47 2009 MARKSTOS [...] cpan.org - Stolen from LDS

Sat Jul 25 17:01:04 2009 MARKSTOS [...] cpan.org - Severity Important changed to Wishlist

Sat Jul 25 17:01:26 2009 MARKSTOS [...] cpan.org - Subject changed from 'CGI::Pretty should not put line breaks after A elements' to 'Needs Test, Patch: CGI::Pretty should not put line breaks after A elements'

Sat Jul 25 17:06:03 2009 MARKSTOS [...] cpan.org - Correspondence added

On Mon Jan 05 14:35:14 2009, rrt wrote: Show quoted text

> On Mon Jan 05 10:56:24 2009, LDS wrote: >

> > I am unable to confirm this under the current version. Could you post > > a short script that demonstrates the problem?

> > Sure. The following script: > > #! /usr/bin/perl > > use CGI qw/:standard/; > use CGI::Pretty; > > print header . start_html("hello_word"), "some text" . a({-href => > "http://www.example.com/"}, "example link") . "some more text", end_html; > > produces the following output for me: > > Content-Type: text/html; charset=ISO-8859-1 > > <!DOCTYPE html > PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" > "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> > <html xmlns="http://www.w3.org/1999/xhtml" lang="en-US"

xml:lang="en-US"> Show quoted text

> <head> > <title>hello_word</title> > <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" /> > </head> > <body> > some text<a href="http://www.example.com/">example link</a> > some more text > </body> > </html> > > Note the line break after </a>, which introduces a space when rendered.

I'm confirmed that this demonstrates the issue with CGI.pm 3.43. To proceed, we'll need the script translated itno an automated test which confirms the issue, and then a patch to resolve it. Mark

Sat Aug 15 19:51:19 2009 BUBAFLUB [...] cpan.org - Correspondence added

Show quoted text

> > Note the line break after </a>, which introduces a space when rendered.

According to the tests #4 and #8 in t/pretty.t a linebreak after an <a> element is expected. Should those tests be changed? The block of code that generates the linebreak can be found on lines 116-119 in CGI::Pretty.pm version 3.44 (pasted and formatted below): if ( exists \$ASIS{ "\L$tagname\E" } ) { \@result = map { "\$tag\$_\$untag\$CGI::Pretty::LINEBREAK" } \@args; } Removing $CGI::Pretty::LINEBREAK from that statement causes test #12 in t/pretty.t to fail (relevant code snippet pasted below): not ok 12 - nested as-is array-reference # Failed test (t/pretty.t at line 98) # got: '<table> <tr> <td><table> <tr> <td>hi</td><td>there</td><td>frog</td> </tr> </table></td> </tr> </table> ' # expected: '<table> <tr> <td><table> <tr> <td>hi</td> <td>there</td> <td>frog</td> </tr> </table></td> </tr> </table> ' What should be done? 1) Should the man page / documentation be clarified that "not formatting" <a> really means that there is a linebreak after them? 2) Should we remove the offending linebreak from CGI::Pretty.pm and have the <td>'s all on one line? 3) Should we make an exception for <a>'s so that no linebreak is added after them?

Sat Aug 15 21:53:35 2009 MARKSTOS [...] cpan.org - Correspondence added

Show quoted text

> 1) Should the man page / documentation be > clarified that "not formatting" <a> really means that there is a > linebreak after them?

I decided to leave it vague for now and continue to say "unformatted". Show quoted text

> 2) Should we remove the offending linebreak from CGI::Pretty.pm and

have the <td>'s all on one line? I think so. The docs say that <td> is one of the tags we don't format. 3) Should we make Show quoted text

> an exception for <a>'s so that no linebreak is added after them?

I think it's reasonable to skip the linebreak for everything in the AS_IS array. With your help, I've now created a patch for this, with credit to you in the Changes file and pushed it in my public github repo. You can see what you think and possibly refine it further. I'm marking this as "patched" for now. Mark

Sat Aug 15 21:53:36 2009 MARKSTOS [...] cpan.org - Status changed from 'open' to 'patched'

Wed Sep 09 22:07:22 2009 MARKSTOS [...] cpan.org - Correspondence added

Subject:

Thanks, released

The patch for this ticket has now been released in CGI.pm 3.47, and this ticket is considered resolved. Thanks again for you help to improve CGI.pm! Mark

Wed Sep 09 22:07:22 2009 The RT System itself - Status changed from 'patched' to 'open'

Wed Sep 09 22:07:23 2009 MARKSTOS [...] cpan.org - Status changed from 'open' to 'resolved'

Thu Feb 17 04:31:02 2011 rrt [...] sc3d.org - Correspondence added

From:

rrt [...] sc3d.org

On Wed Sep 09 22:07:22 2009, MARKSTOS wrote: Show quoted text

> The patch for this ticket has now been released in CGI.pm 3.47, and

this Show quoted text

> ticket is considered resolved.

This is slightly misleading, as the version of CGI::Pretty in which the bug is fixed is smaller, at most 3.46. It would be most helpful if you could update the bug with the version of CGI::Pretty in which this bug was fixed. This is needed for programs which want to "use" the correct version.

Thu Feb 17 04:31:03 2011 The RT System itself - Status changed from 'resolved' to 'open'

Thu Feb 17 07:42:12 2011 MARKSTOS [...] cpan.org - Correspondence added

On Thu Feb 17 04:31:02 2011, rrt wrote: Show quoted text

> On Wed Sep 09 22:07:22 2009, MARKSTOS wrote:

> > The patch for this ticket has now been released in CGI.pm 3.47, and

> this

> > ticket is considered resolved.

> > This is slightly misleading, as the version of CGI::Pretty in which

the Show quoted text

> bug is fixed is smaller, at most 3.46. It would be most helpful if you > could update the bug with the version of CGI::Pretty in which this bug > was fixed. This is needed for programs which want to "use" the correct > version.

It sounds like you are suggesting that the ticket be updated to document that this was fixed in 3.46, not 3.47. In that case, you've just done that. If you'd like to further research the compatibility of this change, you are welcome to.

Thu Feb 17 07:42:13 2011 MARKSTOS [...] cpan.org - Status changed from 'open' to 'stalled'

Thu Feb 17 07:47:14 2011 rrt [...] sc3d.org - Correspondence added

From:

rrt [...] sc3d.org

On Thu Feb 17 07:42:12 2011, MARKSTOS wrote: Show quoted text

> On Thu Feb 17 04:31:02 2011, rrt wrote:

> > On Wed Sep 09 22:07:22 2009, MARKSTOS wrote:

> > > The patch for this ticket has now been released in CGI.pm 3.47,

and Show quoted text

> > this

> > > ticket is considered resolved.

> > > > This is slightly misleading, as the version of CGI::Pretty in which

> the

> > bug is fixed is smaller, at most 3.46. It would be most helpful if

you Show quoted text

> > could update the bug with the version of CGI::Pretty in which this

bug Show quoted text

> > was fixed. This is needed for programs which want to "use" the

correct Show quoted text

> > version.

> > It sounds like you are suggesting that the ticket be updated to

document Show quoted text

> that this was fixed in 3.46, not 3.47.

That is not correct. As you can see from the thread, the bug was fixed in CGI.pm version 3.47. What was unclear is to which version of CGI::Pretty that corresponds. I have now checked: it is CGI::Pretty 3.46.

Thu Feb 17 07:47:14 2011 The RT System itself - Status changed from 'stalled' to 'open'

Thu Feb 17 09:19:06 2011 MARKSTOS [...] cpan.org - Correspondence added

Show quoted text

> That is not correct. As you can see from the thread, the bug was fixed > in CGI.pm version 3.47. What was unclear is to which version of > CGI::Pretty that corresponds. I have now checked: it is CGI::Pretty > 3.46.

Given that you've documented here the detail you seem to be looking for, is there some change you are asking to be made in the CGI.pm distribution? Mark

Thu Feb 17 09:19:08 2011 MARKSTOS [...] cpan.org - Status changed from 'open' to 'stalled'

Thu Feb 17 09:19:55 2011 rrt [...] sc3d.org - Correspondence added

From:

rrt [...] sc3d.org

On Thu Feb 17 09:19:06 2011, MARKSTOS wrote: Show quoted text

>

> > That is not correct. As you can see from the thread, the bug was

fixed Show quoted text

> > in CGI.pm version 3.47. What was unclear is to which version of > > CGI::Pretty that corresponds. I have now checked: it is CGI::Pretty > > 3.46.

> > Given that you've documented here the detail you seem to be looking

for, is Show quoted text

> there some change you are asking to be made in the CGI.pm

distribution? No.

Thu Feb 17 09:19:56 2011 The RT System itself - Status changed from 'stalled' to 'open'

Thu Feb 17 09:41:33 2011 MARKSTOS [...] cpan.org - Status changed from 'open' to 'resolved'

Fri May 23 14:29:29 2014 The RT System itself - Queue changed from CGI.pm to CGI

Bug #42114 for CGI: Needs Test, Patch: CGI::Pretty should not put line breaks after A elements

Preferred bug tracker