Skip Menu |
 

This queue is for tickets about the libwww-perl CPAN distribution.

Report information
The Basics
Id: 18993
Status: resolved
Priority: 0/
Queue: libwww-perl

People
Owner: Nobody in particular
Requestors: gpeters [...] deepsky.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 5.805
Fixed in: (no value)



Subject: HTML::Form does not recognise multiple select items in the same form
Download (untitled) / with headers
text/plain 380b
It is legal HTML to have multiple fields in the same form with the same name. HTML::Form even recognises this. Sadly, <select> form elements are treated as if they're open. See the attached HTML in any web browser, notice three select boxes? Now, parse it with HTML::Form, see the pain? I'm working on fixing this now, so probably I'll have a patch in a few hours to attach.
Subject: simple.html
Download simple.html
text/html 286b
hi mom hi mom hi mom
From: Gavin Peters <gpeters [...] deepsky.com
Download (untitled) / with headers
text/plain 372b
I have now fixed this bug, in the case of select lists. It was a simple fix, and I haven't bothered going to see how many other HTML elements in forms were being left open when they should have been closed. Incidentally: I needed this to handle WWW::Mechanize accessing the MTA FastLane system, a website for managing your automatic toll paying aparatus from the MTA.
Download perl-fix-diff.diff
text/x-diff 1.2k
diff -ru libwww-perl-5.805.orig/lib/HTML/Form.pm libwww-perl-5.805/lib/HTML/Form.pm --- libwww-perl-5.805.orig/lib/HTML/Form.pm 2005-12-07 09:32:27.000000000 -0500 +++ libwww-perl-5.805/lib/HTML/Form.pm 2006-04-30 18:27:19.000000000 -0400 @@ -139,6 +139,8 @@ my @forms; my $f; # current form + my %openselect; # index to the open instance of a select + while (my $t = $p->get_tag) { my($tag,$attr) = @$t; if ($tag eq "form") { @@ -196,6 +198,9 @@ $attr->{"select_$_"} = delete $attr->{$_} if exists $attr->{$_}; } + # count this new select option separately + $openselect{$attr->{name}}++; + while ($t = $p->get_tag) { my $tag = shift @$t; last if $tag eq "/select"; @@ -214,6 +219,7 @@ $a{value_name} = $p->get_trimmed_text; $a{value} = delete $a{value_name} unless defined $a{value}; + $a{idx} = $openselect{$attr->{name}}; $f->push_input("option", \%a); } else { @@ -1043,7 +1049,7 @@ my $m = $self->{menu}[0]; $m->{disabled}++ if delete $self->{option_disabled}; - my $prev = $form->find_input($self->{name}, $self->{type}); + my $prev = $form->find_input($self->{name}, $self->{type}, $self->{idx}); return $self->SUPER::add_to_form($form) unless $prev; # merge menues
It took a while :( I've now applied this patch. Thanks!
Download (untitled) / with headers
text/plain 239b
The patch for this introduced a new bug, which is filed here: http://rt.cpan.org/Ticket/Display.html?id=35607 It would be good to double-check my refinement of the patch submitted in that ticket to confirm it doesn't re-open this defect.
Subject: HTML::Form does not recognise multiple select items (recommend reverting patch)
From: MARKSTOS [...] cpan.org
Download (untitled) / with headers
text/plain 1.9k
On Mon May 05 11:13:16 2008, MARKSTOS wrote: Show quoted text
> The patch for this introduced a new bug, which is filed here: > http://rt.cpan.org/Ticket/Display.html?id=35607 > > It would be good to double-check my refinement of the patch submitted in > that ticket to confirm it doesn't re-open this defect.
After spending hours working with the effects of this patch, I have reverted it in my copy and have come to the conclusion that it should be reverted upstream as well, for two reasons. 1. Clearly, it introduced at least one other bug, as I tried to address in RT#35607. This patch should be re-introduced with at least one automated test to confirm it works as expected. 2. The patch itself introduces a regression of sorts (separate from RT#35607, I think). Before if there were multiple select forms with the same name, it worked to just say "fill in that select form with this value". I suppose since all the <selects> were accidentally lumped together, it just worked. With the patch applied, code that targeted a form that was not first now breaks with an "illegal value" error. I'm not opposed to telling people "You need to update your code because we started working more correctly." However, for this issue there is no easy path forward unless you know exactly by number which <select> form you want to submit to, which I didn't in my application. One possible solution to further extend "find_input" so it can find an input where a value is a possible input. Without something this, other people who upgrade LWP will be stuck like I was when their code starts breaking and there's not a clear way to fix it. In short, we should able to express the following: "Find the <select> tag with this name which accepts this value" I've also added a "wish" in Mechanize to support a fix at that level, but having a smarter find_input() would certainly be helpful. ( http://code.google.com/p/www-mechanize/issues/detail?id=51 ) In the meantime, I do advocate reverting this patch. Mark
Download (untitled) / with headers
text/plain 306b
Mark, Regarding your 2) concern; if you use the $from->param($name, $value) interface to set the value it should just do the right thing, it will search for inputs that accept the value you provide. The issue only occurs for the $form->value($name, $value) interface. I'm not sure that's worth fixing.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.