Skip to content

feat: add support for processing instructions - #790

Open
Delta-official wants to merge 13 commits into
servo:mainfrom
Delta-official:main
Open

Delta-official wants to merge 13 commits into
servo:mainfrom
Delta-official:main

Conversation

@Delta-official

Copy link
Copy Markdown

Adds support for processing instructions in html5ever and changes tests to support ProcessingInstruction tokens (see: html5lib/html5lib-tests#199)

Fixes #789

@github-actions github-actions Bot added V-breaking Breaking change and removed V-breaking Breaking change labels Sep 29, 2026
@github-actions github-actions Bot added V-breaking Breaking change and removed V-breaking Breaking change labels Sep 29, 2026
@TimvdLippe
TimvdLippe requested a review from mrobinson October 3, 2026 07:44

@mrobinson mrobinson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few comments here:

  • Did you run tests for this change?
  • Have you tested serialization? How does a processing instruction like <?one two?> serialize?
  • It looks like you are only adding support for processing instructions in two insertion modes? The specification lists that they should be handled in many more (for instance in tables: https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-intable).

Comment thread html5ever/src/tokenizer/mod.rs Outdated
Comment thread html5ever/src/tokenizer/mod.rs Outdated
Comment thread html5ever/src/tokenizer/mod.rs
Comment thread html5ever/src/tokenizer/states.rs Outdated
Comment thread html5ever/src/tokenizer/mod.rs Outdated
Comment thread html5ever/src/tokenizer/mod.rs Outdated
Comment thread html5ever/src/tokenizer/mod.rs Outdated
@Delta-official

Copy link
Copy Markdown
Author

A few comments here:

* Did you run tests for this change?

Yes. Some are failing (see below) but it's a result of html5lib-tests being outdated

Failing cases
failures:
tok: test3.test: <?
tok: test3.test: <? (exact errors)
tok: test3.test: <?A
tok: test3.test: <?B
tok: test3.test: <?B (exact errors)
tok: test3.test: <?Y (exact errors)
tok: test3.test: <?Z (exact errors)
tok: test3.test: <?A (exact errors)
tok: test3.test: <?a
tok: test3.test: <?a (exact errors)
tok: test3.test: <?Y
tok: test3.test: <?Z
tok: test3.test: <?z
tok: test3.test: <?z (exact errors)
tok: test3.test: <?b (exact errors)
tok: test3.test: <?y
tok: test3.test: <?b
tok: test3.test: <?y (exact errors)
tok: test2.test: Simili processing instruction
tok: test2.test: A bogus comment stops at >, even if preceded by two dashes
tok: test2.test: Simili processing instruction (exact errors)
tok: test2.test: A bogus comment stops at >, even if preceded by two dashes (exact errors)
* Have you tested serialization? How does a processing instruction like `<?one two?>` serialize?

Forgot about serialization, gonna add tests cases for that.

* It looks like you are only adding support for processing instructions in two insertion modes? The specification lists that they should be handled in many more (for instance in tables: https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-intable).

Missed those. Mainly went by warnings to see what was broken since this is my first time contributing and the other modes had default branches

@github-actions github-actions Bot added V-breaking Breaking change and removed V-breaking Breaking change labels Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

V-breaking Breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support processing instructions <?target data>

2 participants