Re: Bug in HTML::Form <label> support

Dan Kubb writes:

> Anyway, the patch is attached. I'd appreciate comments,

[...]

> diff -ru old/lib/HTML/Form.pm new/lib/HTML/Form.pm
> --- old/lib/HTML/Form.pm Tue Nov 30 03:25:00 2004
> +++ new/lib/HTML/Form.pm Fri Dec 3 00:44:02 2004
> @@ -96,7 +96,7 @@
> my $p = HTML::TokeParser->new(ref($html) ? $html->decoded_content(ref => 1) : \$html);
> eval {
> # optimization
> - $p->report_tags(qw(form input textarea select optgroup option keygen));
> + $p->report_tags(qw(form input textarea select optgroup option keygen label));
> };
>
> unless (defined $base_uri) {
> @@ -122,12 +122,34 @@
> $attr->{'enctype'});
> $f->{attr} = $attr;
> push(@forms, $f);
> + my(%labels, $current_label);
> while (my $t = $p->get_tag) {
> my($tag, $attr) = @$t;
> last if $tag eq "/form";
> - if ($tag eq "input") {
> + if ($tag eq "label") {
> + $current_label = $p->get_phrase;
> + $labels{ $attr->{for} } = $current_label
> + if exists $attr->{for};
> + }
> + elsif ($tag eq "/label") {
> + $current_label = undef;
> + }
> + elsif ($tag eq "input") {

Are there other form elements than that might take labels?

> my $type = delete $attr->{type} || "text";
> - $attr->{value_name} = $p->get_phrase;
> +
> + if(exists $attr->{id} && exists $labels{$attr->{id}}) {
> + $attr->{value_name} = $labels{$attr->{id}};

Indentation is not consistent with the rest of the code.

> + }
> + else {
> + $attr->{value_name} = defined $current_label
> + ? $current_label . " " . $p->get_phrase
> + : $p->get_phrase;
> +
> + $attr->{value_name} =~ s/\A\s+//;
> + 1 while $attr->{value_name} =~ s/\s\z//;

why not '$attr->{value_name} =~ s/\s+\z//;'

> + $attr->{value_name} =~ s/\s+/ /;

There can't really be multispace anywhere since get_phrase will trim
the text. This would always be a noop.

> + }
> +
> $f->push_input($type, $attr);
> }
> elsif ($tag eq "textarea") {
> diff -ru old/t/html/form.t new/t/html/form.t
> --- old/t/html/form.t Mon Nov 15 06:45:42 2004
> +++ new/t/html/form.t Fri Dec 3 01:07:09 2004
> @@ -3,7 +3,7 @@
> use strict;
> use Test qw(plan ok);
>
> -plan tests => 98;
> +plan tests => 102;
>
> use HTML::Form;
>
> @@ -447,3 +447,27 @@
> ok(@f, 2);
> ok($f[0]->find_input("s"));
> ok($f[1]->find_input("t"));
> +
> +$f = HTML::Form->parse(< > +