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

feat(parser/html): lex and parse unquoted attribute values #3951

Merged

Conversation

dyc3
Copy link
Contributor

@dyc3 dyc3 commented Sep 16, 2024

Summary

This PR lets the HTML parser correctly lex and parse unquoted attribute values.

Test Plan

Added a test. See also: https://html.spec.whatwg.org/#attributes-2

@github-actions github-actions bot added A-Parser Area: parser L-HTML Language: HTML labels Sep 16, 2024
Copy link

codspeed-hq bot commented Sep 16, 2024

CodSpeed Performance Report

Merging #3951 will degrade performances by 6.44%

Comparing 09-16-feat_parser_html_lex_and_parse_unquoted_attribute_values (4b03d2f) with main (2c8ff7f)

Summary

❌ 1 regressions
✅ 106 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main 09-16-feat_parser_html_lex_and_parse_unquoted_attribute_values Change
react.production.min_3378072959512366797.js[cached] 1.9 ms 2 ms -6.44%

@dyc3 dyc3 force-pushed the 09-16-feat_parser_html_lex_and_parse_unquoted_attribute_values branch from f991283 to fb69e6c Compare September 16, 2024 22:53
@dyc3 dyc3 marked this pull request as ready for review September 16, 2024 23:09
@dyc3 dyc3 requested review from a team September 16, 2024 23:09
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Looks to me, however we should:

  • Add more test cases, for example, cases where the attribute value has spaces (\n, \t) and </>, because they are part of the new logic.
  • Add an invalid test case that covers the new error you added

@dyc3 dyc3 force-pushed the 09-16-feat_parser_html_lex_and_parse_unquoted_attribute_values branch from fb69e6c to 5e73499 Compare September 17, 2024 12:08
@dyc3 dyc3 requested a review from ematipico September 17, 2024 12:11
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you, there's one case to cover and we can merge it

@dyc3 dyc3 force-pushed the 09-16-feat_parser_html_lex_and_parse_unquoted_attribute_values branch from 5e73499 to 4b03d2f Compare September 18, 2024 13:07
@dyc3 dyc3 merged commit a7b623a into main Sep 18, 2024
14 checks passed
@dyc3 dyc3 deleted the 09-16-feat_parser_html_lex_and_parse_unquoted_attribute_values branch September 18, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Parser Area: parser L-HTML Language: HTML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants