search

libwww-perl / WWW-Mechanize

Handy web browsing in a Perl object
https://metacpan.org/pod/WWW::Mechanize
Other
68 stars 52 forks source link

HTML::Form::find_input() has a 1-based index #293

Closed oalders closed 4 years ago

oalders commented 4 years ago

Fixes https://github.com/libwww-perl/HTML-Form/issues/22

petdance commented 4 years ago

I think this needs a change to the HTML::Form requirement for the module, too.

It looks to me like this change requires HTML::Form 6.06, no? If someone doesn't have 6.06 and they use this, Mech isn't going to work, right?

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 305

Totals Coverage Status
Change from base Build 303: 0.0%
Covered Lines: 729
Relevant Lines: 782
💛 - Coveralls
oalders commented 4 years ago

@petdance thanks for reviewing this. It won't break older versions of WWW::Mechanize. This is just drawing attention to an incorrect use of find_input where an index < 1 gets passed to find_input.

HTML::Form does fix up 0 to 1 here: https://github.com/choroba/HTML-Form/blob/0db2697f3fcdd385dc67cac4ee31fb4958093220/lib/HTML/Form.pm#L525 but that should probably be a defined check.

That sub was really confusing. It was using a 1-based list but allowing a 0 to be passed as an index value and then subtly changing that to 1 later on, so you could pass 0 or 1 and still get the same value back. :(

oalders commented 4 years ago

Since this dist is currently broken, I'm going to go ahead and merge + release.