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

bpo-41748: Handles unquoted attributes with commas #24072

Merged
merged 9 commits into from
Feb 1, 2021

Conversation

karlcow
Copy link
Contributor

@karlcow karlcow commented Jan 3, 2021

data:text/html,<!doctype html><div class=bar,baz=asd>text</div>
serialized as <div class="bar,baz=asd">text</div>

but

data:text/html,<!doctype html><div class=bar ,baz=asd>text</div>
serialized as <div class="bar" ,baz="asd">text</div>

As discussed in https://bugs.python.org/issue41748

https://bugs.python.org/issue41748

Automerge-Triggered-By: GH:ezio-melotti

Lib/html/parser.py Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ezio-melotti ezio-melotti self-assigned this Jan 3, 2021
@karlcow
Copy link
Contributor Author

karlcow commented Jan 3, 2021

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

@ezio-melotti: please review the changes made to this pull request.

Lib/html/parser.py Outdated Show resolved Hide resolved
Lib/test/test_htmlparser.py Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@karlcow
Copy link
Contributor Author

karlcow commented Jan 4, 2021

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

@ezio-melotti: please review the changes made to this pull request.

Copy link
Member

@ezio-melotti ezio-melotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple minor comments, in addition to the comment I left in the main conversation.
Also remember to add a NEWS entry to the PR.

Lib/html/parser.py Outdated Show resolved Hide resolved
Lib/test/test_htmlparser.py Show resolved Hide resolved
@karlcow
Copy link
Contributor Author

karlcow commented Jan 6, 2021

@ezio-melotti done. I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

@ezio-melotti: please review the changes made to this pull request.

@ezio-melotti
Copy link
Member

Thanks for addressing all the comments, we only need a NEWS entry and then I can merge the PR.
You can use blurb or blurb-it to add one (see the devguide for more info).

@karlcow
Copy link
Contributor Author

karlcow commented Jan 6, 2021

@ezio-melotti NEWS.d entry added. Thanks for the review and the help along this PR.

@karlcow
Copy link
Contributor Author

karlcow commented Jan 12, 2021

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

@ezio-melotti: please review the changes made to this pull request.

@ezio-melotti ezio-melotti added 🤖 automerge needs backport to 3.8 only security fixes needs backport to 3.9 only security fixes type-bug An unexpected behavior, bug, or error labels Feb 1, 2021
@ezio-melotti ezio-melotti merged commit 9eb11a1 into python:master Feb 1, 2021
@miss-islington
Copy link
Contributor

Thanks @karlcow for the PR, and @ezio-melotti for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

@bedevere-bot
Copy link

@ezio-melotti: Please replace # with GH- in the commit message next time. Thanks!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 1, 2021
* bpo-41748: Adds tests for unquoted attributes with comma

* bpo-41748: Handles unquoted attributes with comma

* bpo-41748: Addresses review comments

* bpo-41748: Addresses review comments

* Adds more test cases
* Simplifies the regex for handling spaces

* bpo-41748: Moves attributes tests under the right class

* bpo-41748: Addresses review about duplicate attributes

* bpo-41748: Adds NEWS.d entry for this patch
(cherry picked from commit 9eb11a1)

Co-authored-by: Karl Dubost <karl+github@la-grange.net>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Feb 1, 2021
@bedevere-bot
Copy link

GH-24415 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 1, 2021
* bpo-41748: Adds tests for unquoted attributes with comma

* bpo-41748: Handles unquoted attributes with comma

* bpo-41748: Addresses review comments

* bpo-41748: Addresses review comments

* Adds more test cases
* Simplifies the regex for handling spaces

* bpo-41748: Moves attributes tests under the right class

* bpo-41748: Addresses review about duplicate attributes

* bpo-41748: Adds NEWS.d entry for this patch
(cherry picked from commit 9eb11a1)

Co-authored-by: Karl Dubost <karl+github@la-grange.net>
@bedevere-bot
Copy link

GH-24416 is a backport of this pull request to the 3.8 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.8 only security fixes label Feb 1, 2021
miss-islington added a commit that referenced this pull request Feb 1, 2021
* bpo-41748: Adds tests for unquoted attributes with comma

* bpo-41748: Handles unquoted attributes with comma

* bpo-41748: Addresses review comments

* bpo-41748: Addresses review comments

* Adds more test cases
* Simplifies the regex for handling spaces

* bpo-41748: Moves attributes tests under the right class

* bpo-41748: Addresses review about duplicate attributes

* bpo-41748: Adds NEWS.d entry for this patch
(cherry picked from commit 9eb11a1)

Co-authored-by: Karl Dubost <karl+github@la-grange.net>
miss-islington added a commit that referenced this pull request Feb 1, 2021
* bpo-41748: Adds tests for unquoted attributes with comma

* bpo-41748: Handles unquoted attributes with comma

* bpo-41748: Addresses review comments

* bpo-41748: Addresses review comments

* Adds more test cases
* Simplifies the regex for handling spaces

* bpo-41748: Moves attributes tests under the right class

* bpo-41748: Addresses review about duplicate attributes

* bpo-41748: Adds NEWS.d entry for this patch
(cherry picked from commit 9eb11a1)

Co-authored-by: Karl Dubost <karl+github@la-grange.net>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
* bpo-41748: Adds tests for unquoted attributes with comma

* bpo-41748: Handles unquoted attributes with comma

* bpo-41748: Addresses review comments

* bpo-41748: Addresses review comments

* Adds more test cases
* Simplifies the regex for handling spaces

* bpo-41748: Moves attributes tests under the right class

* bpo-41748: Addresses review about duplicate attributes

* bpo-41748: Adds NEWS.d entry for this patch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants