Skip to content

Commit 89da3b8

Browse files
authored
parse select inputs that have no name #2 (#40)
In order to do this, I had to allow find_input to find inputs that have no name. Before, we treated an undef in the name (actually, the 'selector') as an instruction not to match the name. To put option elements into the correct surrounding select, we have to match them by their name, including when there is no name. Two of the tests I have added don't produce what I would expect. I think this is another bug. It seems we include unnamed fields that have a value in the query string, as long as they have a name attribute. This leads to query strings such as 'name=value&=value'. The W3 specification explicitly says that only element that have names should be submitted, so I believe this behaviour is wrong.
1 parent e567495 commit 89da3b8

3 files changed

Lines changed: 105 additions & 13 deletions

File tree

Changes

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ Change history for HTML-Form
44
- Use "croak" instead of "die" to show errors from the perspective of the
55
caller [RT#20499] (GH#29) (Julien Fiegehenn)
66
- Remove the executable bit from a couple of tests (GH#41) (James Raspass)
7+
- <option>s within select fields without a name no longer get merged into
8+
the previous select field (GH#2) (Julien Fiegehenn)
9+
- find_input() can now take a reference to undef to explicitly find inputs
10+
that have no name (GH#2) (Julien Fiegehenn)
711

812
6.09 2022-08-14 22:16:37Z
913

lib/HTML/Form.pm

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -488,24 +488,35 @@ This method is used to locate specific inputs within the form. All
488488
inputs that match the arguments given are returned. In scalar context
489489
only the first is returned, or C<undef> if none match.
490490
491-
If $selector is not C<undef>, then the input's name, id, class attribute must
492-
match. A selector prefixed with '#' must match the id attribute of the input.
493-
A selector prefixed with '.' matches the class attribute. A selector prefixed
494-
with '^' or with no prefix matches the name attribute.
491+
If C<$selector> is not C<undef>, then the input's I<name>, I<id> or I<class>
492+
attribute must match.
493+
A selector prefixed with '#' must match the I<id> attribute of the input.
494+
A selector prefixed with '.' matches the I<class> attribute. A selector prefixed
495+
with '^' or with no prefix matches the I<name> attribute.
495496
496-
If $type is not C<undef>, then the input must have the specified type.
497+
my @by_id = $form->find_input( '#some-id' );
498+
my @by_class = $form->find_input( '.some-class' );
499+
my @by_name = $form->find_input( '^some-name' );
500+
my @also_by_name = $form->find_input( 'some-name' );
501+
502+
If you want to find an input that has no I<name> at all, pass in a reference
503+
to C<undef>.
504+
505+
my @nameless_inputs = $form->find_input( \undef );
506+
507+
If C<$type> is not C<undef>, then the input must have the specified type.
497508
The following type names are used: "text", "password", "hidden",
498509
"textarea", "file", "image", "submit", "radio", "checkbox" and "option".
499510
500-
The $index is the sequence number of the input matched where 1 is the
501-
first. If combined with $name and/or $type, then it selects the I<n>th
502-
input with the given name and/or type.
511+
The C<$index> is the sequence number of the input matched where 1 is the
512+
first. If combined with C<$selector> and/or C<$type>, then it selects the
513+
I<n>th input with the given I<name> and/or type.
503514
504515
=cut
505516

506517
sub find_input
507518
{
508-
my($self, $name, $type, $no) = @_;
519+
my($self, $selector, $type, $no) = @_;
509520
Carp::croak "Invalid index $no"
510521
if defined $no && $no < 1;
511522
if (wantarray) {
@@ -514,7 +525,20 @@ sub find_input
514525
my @res;
515526
my $c;
516527
for (@{$self->{'inputs'}}) {
517-
next if defined($name) && !$_->selected($name);
528+
if ( defined($selector) ) {
529+
530+
# an input that explicitly has no name
531+
if ( ref($selector) eq 'SCALAR' ) {
532+
next
533+
if !defined($$selector) && $_->{name};
534+
}
535+
536+
# an input that does not fit this selector
537+
else {
538+
next
539+
if !$_->selected($selector);
540+
}
541+
}
518542
next if $type && $type ne $_->{type};
519543
$c++;
520544
next if $no && $no != $c;
@@ -526,7 +550,20 @@ sub find_input
526550
else {
527551
$no ||= 1;
528552
for (@{$self->{'inputs'}}) {
529-
next if defined($name) && !$_->selected($name);
553+
if ( defined($selector) ) {
554+
555+
# an input that explicitly has no name
556+
if ( ref($selector) eq 'SCALAR' ) {
557+
next
558+
if !defined($$selector) && $_->{name};
559+
}
560+
561+
# an input that does not fit this selector
562+
else {
563+
next
564+
if !$_->selected($selector);
565+
}
566+
}
530567
next if $type && $type ne $_->{type};
531568
next if --$no;
532569
return $_;
@@ -1235,7 +1272,9 @@ sub add_to_form
12351272
my $m = $self->{menu}[0];
12361273
$m->{disabled}++ if delete $self->{option_disabled};
12371274

1238-
my $prev = $form->find_input($self->{name}, $self->{type}, $self->{idx});
1275+
# if there was no name we have to search for an input that explicitly has
1276+
# no name either, because otherwise the name attribute would be ignored
1277+
my $prev = $form->find_input($self->{name} || \undef, $self->{type}, $self->{idx});
12391278
return $self->SUPER::add_to_form($form) unless $prev;
12401279

12411280
# merge menus

t/form.t

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use strict;
44
use warnings;
55

6-
use Test::More tests => 129;
6+
use Test::More;
77
use HTML::Form;
88

99
my @warn;
@@ -623,3 +623,52 @@ Content-Type: application/x-www-form-urlencoded
623623
624624
submit=
625625
EOT
626+
627+
# select with a name followed by select without a name GH#2
628+
$f = HTML::Form->parse(<<EOT, "http://localhost/");
629+
<form action="target.html" method="get">
630+
<select>
631+
<option selected>option in unnamed before</option>
632+
</select>
633+
<select name="foo">
634+
<option selected>option in named</option>
635+
</select>
636+
<select>
637+
<option selected>option in unnamed after 1</option>
638+
</select>
639+
<select name="">
640+
<option selected>option in empty string name</option>
641+
</select>
642+
EOT
643+
644+
TODO: {
645+
local $TODO = 'input with empty name should not be included';
646+
is(
647+
join( "|", $f->form ),
648+
"foo|option in named",
649+
"options in unnamed selects are ignored"
650+
);
651+
}
652+
653+
# explicitly selecting an input that has no name
654+
my @nameless_inputs = $f->find_input( \undef );
655+
is( scalar @nameless_inputs,
656+
3, 'find_input with ref to undef finds three forms' );
657+
ok(
658+
( !grep { $_->{name} } @nameless_inputs ),
659+
'... and none of them has a name'
660+
);
661+
662+
ok(
663+
!( scalar $f->find_input( \undef ) )->{name},
664+
'find_input with ref to undef in scalar context'
665+
);
666+
TODO: {
667+
local $TODO = 'input with empty name should not be included';
668+
is($f->click->as_string, <<"EOT");
669+
GET http://localhost/target.html?foo=option+in+named
670+
671+
EOT
672+
}
673+
674+
done_testing;

0 commit comments

Comments
 (0)