New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
handle <option> tag without value option with tests #252
Conversation
Both commits are related and should be squashed IMHO. For example, someone doing a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not merging right away to let @hemberger comment if needed, but it all looks good to me (modulo the 1-commit vs 2-commits comment I made, but this can be fixed while merging).
1. The `Form.set_select` method will now select an <option> whose text matches the `data` argument if no matching value-attribute is found (even if the <option> it selects still has a value attribute). 2. When determining what form data to submit, selected <option> tags that do not have a value attribute will submit their text as the value instead. This change is made in accordance with the standard processing of <option> tags. See: https://www.w3schools.com/tags/att_option_value.asp > Note: If the value attribute is not specified, the content will > be passed as a value instead. Additionally, the `string` argument to bs4's `find` function requires beautifulsoup4 version 4.4 or greater. https://www.crummy.com/software/BeautifulSoup/bs4/doc/#the-string-argument
Thanks for the PR @tiboroche! I did a tiny bit of cleanup and then added a little more documentation in a fixup commit. I would suggest that we squash these commits together and use my commit message for the final version. |
Looks all good.
OK for me. I'd suggest keeping @tiboroche as author and adding a Co-authored-by: trailer. |
You are welcome. I am implementing MechanicalSoup at work for automated testing, and it is working great so far. I may contribute again soon depending on the bugs I encounter or the features I would like to add. |
Squashed and pushed as 5b16e00. |
<option> tags can omit the value option, in this case the text of the
option is used, cf :
https://www.w3schools.com/tags/att_option_value.asp