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

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