Skip to content
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

fix html.SelectElement stripping whitespace from <option> values #228

Merged
merged 2 commits into from Feb 17, 2017
Merged

fix html.SelectElement stripping whitespace from <option> values #228

merged 2 commits into from Feb 17, 2017

Conversation

@ashkulz
Copy link
Contributor

@ashkulz ashkulz commented Feb 16, 2017

This is a fix for bug #1665241. I've tried to add tests, please let me know if any changes need to be made.

Ashish Kulkarni added 2 commits Feb 16, 2017
This is a fix for bug #1665241, as the original fix for #399249
in 0b14af5 should not apply to
option values, which are supposed to be passed through as-is.
@scoder
Copy link
Member

@scoder scoder commented Feb 17, 2017

Nice, thanks!

@scoder scoder merged commit 47741a8 into lxml:master Feb 17, 2017
@ashkulz
Copy link
Contributor Author

@ashkulz ashkulz commented Feb 17, 2017

Should I submit a PR for backporting it to the 3.7 branch? A 3.7.2 release is likely to get into the upcoming Debian release (either stretch or stretch-backports) 😄

Also, Travis CI doesn't seem to be working although it seems to be configured -- is it disabled for some reason?

@scoder
Copy link
Member

@scoder scoder commented Feb 17, 2017

While a bug-fix, this is also a change of long-standing behaviour that does not strictly belong into a 3.7.3.

@ashkulz
Copy link
Contributor Author

@ashkulz ashkulz commented Feb 17, 2017

Fair enough. I've submitted a PR to scrapy to document this, as I encountered the problem while using it.

nozayasu added a commit to nozayasu/scrapy that referenced this issue Mar 2, 2017
This can make the spider fail due to incorrect values being posted
server-side, which is extremely hard to debug because it is easy
to miss leading/trailing whitespace, even with a logging proxy.

The fix was merged for lxml 3.8 in lxml/lxml#228 so document that
as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants