Skip to content

Implement the latest select parsing changes#3317

Open
flavorjones wants to merge 4 commits intomainfrom
flavorjones-html5-relaxed-select
Open

Implement the latest select parsing changes#3317
flavorjones wants to merge 4 commits intomainfrom
flavorjones-html5-relaxed-select

Conversation

@flavorjones
Copy link
Member

@flavorjones flavorjones commented Oct 16, 2024 •
edited
Loading

What problem is this PR intended to solve?

In whatwg/html#10557 there are some changes being made to how <select> tags are parsed.

Have you included adequate test coverage?

Tests are under development on a branch at html5lib/html5lib-tests#178

Does this change affect the behavior of either the C or the Java implementations?

HTML5 is only supported by the C impl.

@flavorjones

This comment was marked as outdated.

Sign in to view
@flavorjones flavorjones mentioned this pull request Oct 16, 2024
Merged
@flavorjones flavorjones force-pushed the flavorjones-html5-relaxed-select branch from 3cefda2 to eb62613 Compare October 16, 2024 19:45
@flavorjones
Copy link
Member Author

flavorjones commented Oct 16, 2024

Ugh, sorry about that -- PEBKAC -- I just needed to force-recompile and that particular test is passing. There are other ones that I'll look into now.

@stevecheckoway
Copy link
Contributor

stevecheckoway commented Oct 16, 2024

I see that whatwg/html PR has had a bunch of changes since I last looked at it. I can take a look at this PR in more detail next week.

@flavorjones
Copy link
Member Author

flavorjones commented Oct 16, 2024 •
edited
Loading

@stevecheckoway Thanks. Actually, my changes look pretty good! Only two tests are failing for non-error-message reasons, and they're both very similar. (This PR is using a fixed-up version of html5lib/html5lib-tests#178 with appropriate error messages.)

Zooming in on the simpler of the two (CI failure here):

#data
<select><b><option><select><option></b></select>
#errors
#document
| <html>
|   <head>
|   <body>
|     <select>
|       <b>
|         <option>
|     <b>
|     <select>
|       <b>
|         <option>

This pr results in the following tree:

<body>
  <select>
    <b>
      <option>
  <b>
    <select>
      <option>

so I wanted to double check if I've missed something or if these tests are incorrect.

@flavorjones
Copy link
Member Author

flavorjones commented Oct 18, 2024

@stevecheckoway Before you dig in on this, please catch up on the chat I'm having upstream in html5lib/html5lib-tests#178, TLDR I think I'm right and the test (and chromium) are wrong.

@flavorjones
Copy link
Member Author

flavorjones commented Oct 30, 2024

Updated the gumbo tests.

Note that these changes break rails-html-sanitizer, I'll need to work a bit upstream before these changes are safe to merge and release.

@flavorjones flavorjones changed the title Implement the latest select parsing changes Implement the latest select parsing changes Nov 17, 2024
@flavorjones flavorjones force-pushed the flavorjones-html5-relaxed-select branch 2 times, most recently from 1442b66 to 4e0111b Compare November 23, 2024 14:47
@flavorjones flavorjones force-pushed the flavorjones-html5-relaxed-select branch from 4e0111b to 6b09d9c Compare December 19, 2024 23:51
flavorjones and others added 2 commits December 20, 2024 08:20
@flavorjones
feat: implement the new select parsing rules
9d56278 
whatwg/html#10557

This is green (modulo errors) up to whatwg/html@f8d14341 plus the
correction I suggested to the spec in
whatwg/html#10557 (comment)
@flavorjones
test: commenting out html5 error testing
e5e51e2 
because most implementations don't seem to be paying attention to
them?
@flavorjones flavorjones force-pushed the flavorjones-html5-relaxed-select branch from 6b09d9c to e5e51e2 Compare December 20, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@flavorjones @stevecheckoway