Skip to content

Parse XML-style <?target data?> processing instructions#12118

Merged
annevk merged 29 commits into
mainfrom
foolip/pi-parsing
Jun 25, 2026
Merged

Parse XML-style <?target data?> processing instructions#12118
annevk merged 29 commits into
mainfrom
foolip/pi-parsing

Conversation

@foolip

@foolip foolip commented Jan 31, 2026

Copy link
Copy Markdown
Member

Notably behavior and their rationale:

  • While <? opens a PI, a > always closes it. This to match the bogus
    comment tokenizer behavior, so that exactly the same characters are
    consumed as bogus comments in existing parsers. Any deviation would
    make this a much riskier parser change.
  • The target must start with ASCII alpha, but after that almost anything
    goes, matching tag names. This doesn't have to be this way, but
    there's no obvious other precedent to follow.
  • The target is ASCII-lowercased by the tokenizer, just like tag and
    attribute names. This is for consistency, and there's precedent from
    the SVG-in-HTML parser behavior.
  • If target is a case-insensitive match for "xml" or "xml-stylesheet",
    it's instead treated as a bogus comment. <?xml?> is not a valid PI
    in XML, and <?xml-stylesheet href="style.css"?> would otherwise
    start loading stylesheets in HTML documents, which we don't want.
  • A ? that's not followed by a > becomes part of the data (never
    the target). This is to match XML for a <?t ???> where data is "??".
  • For <?t?d?> (invalid XML) the target is "t" and data is "?d". This
    behavior results from handling ? the same way wherever it's
    encountered. The example would serialize back as <?t ?d?>.
  • <?> and <? followed by EOF are treated as bogus comments, because
    this is the most conservative choice, and also avoids empty target.

(See WHATWG Working Mode: Changes for more details.)


/index.html ( diff )
/infrastructure.html ( diff )
/parsing.html ( diff )
/syntax.html ( diff )

Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source
@foolip foolip force-pushed the foolip/pi-parsing branch from f806ae3 to a26fe99 Compare January 31, 2026 08:09
Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source
Comment thread source Outdated
@foolip foolip marked this pull request as ready for review January 31, 2026 21:15
@foolip

foolip commented Jan 31, 2026

Copy link
Copy Markdown
Member Author

I've polished some more, this is close to ready now. Needs impl interest and tests, of course.

@noamr

noamr commented Jan 31, 2026

Copy link
Copy Markdown
Contributor

I've polished some more, this is close to ready now. Needs impl interest and tests, of course.

Nice! I can have tests ready within a few days.

@hsivonen hsivonen 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.

LGTM. Thanks.

Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
@zcorpan

zcorpan commented Feb 2, 2026

Copy link
Copy Markdown
Member
  • For <?t?d?> (invalid XML) the target is "t" and data is "?d". This
    behavior results from handling ? the same way wherever it's
    encountered. The example would serialize back as <?t ?d?>.

Nit: As specified in https://html.spec.whatwg.org/#serialising-html-fragments it would serialize as <?t ?d> (without trailing ?)

@foolip

foolip commented Feb 3, 2026

Copy link
Copy Markdown
Member Author

@zcorpan you're absolutely right, I was testing in Chrome and didn't realize the spec says otherwise.

@foolip

foolip commented Feb 3, 2026

Copy link
Copy Markdown
Member Author

Test for serialization: web-platform-tests/wpt#57486

@zcorpan

zcorpan commented Feb 3, 2026

Copy link
Copy Markdown
Member

Ah, I did not know Chromium and WebKit serialize with ?>... After discussing with @hsivonen a bit, we think it makes sense to change Gecko to serialize with ?> also.

This opens up for making the ? required for document conformance.

Pros to making it required:

  • It's a clear deliberate ending of the PI, so that an accidental > can be highlighted as a syntax error.
  • Matches XML (except HTML PI can't contain >).

Cons:

  • It's an extra character that's not strictly necessary.

@zcorpan

zcorpan commented Feb 3, 2026

Copy link
Copy Markdown
Member

Also, maybe we can just support xml-stylesheet in HTML (but do nothing for XSLT)? It's already supported in the DOM: https://software.hixie.ch/utilities/js/live-dom-viewer/saved/14486

I checked these 6 pages https://docs.google.com/spreadsheets/d/1o04eP_BwH1u7X8CyyLUvxOsntZNmfrDalfhU7ldVqlU/edit?gid=233477192#gid=233477192&range=C12 , only one has a PI pointing to external CSS but that file is empty.

@noamr

noamr commented Feb 3, 2026

Copy link
Copy Markdown
Contributor

Also, maybe we can just support xml-stylesheet in HTML (but do nothing for XSLT)? It's already supported in the DOM: https://software.hixie.ch/utilities/js/live-dom-viewer/saved/14486

I checked these 6 pages https://docs.google.com/spreadsheets/d/1o04eP_BwH1u7X8CyyLUvxOsntZNmfrDalfhU7ldVqlU/edit?gid=233477192#gid=233477192&range=C12 , only one has a PI pointing to external CSS but that file is empty.

Yea we could probably also parse that PI for <?xml> without that doing too much damage as nobody is looking at it.

@zcorpan

zcorpan commented Feb 3, 2026

Copy link
Copy Markdown
Member

Yea we could probably also parse that PI for <?xml> without that doing too much damage as nobody is looking at it.

Hmm, it would break roundtripping to XML. I'd prefer a comment node or dropping it.

@foolip

foolip commented Feb 4, 2026

Copy link
Copy Markdown
Member Author

On serializing PIs with just >, the current behavior was spec'd in 5319a9a and https://lists.whatwg.org/pipermail/whatwg-whatwg.org/2008-February/056354.html (same day) doesn't give any more details.

@zcorpan, changing it to ?> and updating web-platform-tests/wpt#57486 would be OK, but I already prepared https://chromium-review.googlesource.com/c/chromium/src/+/7531667, so that door is open.

Regarding conformance, do you mean that a case like <?t name="a>b"?> can be highlighted as a syntax error? That's a good point, so even if you assume that you can put > inside of PIs without escaping, it will be an error. Note that <?t name="a>b"> could also be a syntax error however. It's only with > in an unquoted attribute value like <?t name=a>b> that we can't make it an error, but that case looks like regular element syntax. Am I misunderstanding the argument?

@justinfagnani

Copy link
Copy Markdown

On the DOM side, does this still create Comment nodes? Otherwise this is certainly not web compatible.

@noamr

noamr commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

On the DOM side, does this still create Comment nodes? Otherwise this is certainly not web compatible.

It creates ProcessingInstruction nodes which already exist in DOM. What specifically makes this web incompatible in your view? HTTP archive shows very little use of PI-like syntax and we can exclude problematic targets if we need to.

@justinfagnani

Copy link
Copy Markdown

In lit-html we use PI syntax to create comments that are parsed inside templates: https://github.com/lit/lit/blob/f243134b226735320b61466cebdaf0c1e574bfa7/packages/lit-html/src/lit-html.ts#L354

These comments are then retrieved with a TreeWalker that shows Comment nodes.

The targets aren't fixed, but have the form lit$$xxxxxxxxx$ where xxxxxxxxx is a 9-digit random number.

@noamr

noamr commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

In lit-html we use PI syntax to create comments that are parsed inside templates: https://github.com/lit/lit/blob/f243134b226735320b61466cebdaf0c1e574bfa7/packages/lit-html/src/lit-html.ts#L354

These comments are then retrieved with a TreeWalker that shows Comment nodes.

The targets aren't fixed, but have the form lit$$xxxxxxxxx$ where xxxxxxxxx is a 9-digit random number.

We can easily exclude targets that start with ’lit$’ from this, or any other string that we find to be problematic in terms of compat.

@noamr

noamr commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

In lit-html we use PI syntax to create comments that are parsed inside templates: https://github.com/lit/lit/blob/f243134b226735320b61466cebdaf0c1e574bfa7/packages/lit-html/src/lit-html.ts#L354

These comments are then retrieved with a TreeWalker that shows Comment nodes.

The targets aren't fixed, but have the form lit$$xxxxxxxxx$ where xxxxxxxxx is a 9-digit random number.

@foolip suggested that we constrain the syntax instead.
Perhaps something like this would work:

^[a-zA-Z_][\w_\.\-]*$

I think treating invalid targets as bogus comments would work better than block-listing lit$ and would fix this issue.

@noamr

noamr commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

In lit-html we use PI syntax to create comments that are parsed inside templates: https://github.com/lit/lit/blob/f243134b226735320b61466cebdaf0c1e574bfa7/packages/lit-html/src/lit-html.ts#L354
These comments are then retrieved with a TreeWalker that shows Comment nodes.
The targets aren't fixed, but have the form lit$$xxxxxxxxx$ where xxxxxxxxx is a 9-digit random number.

@foolip suggested that we constrain the syntax instead. Perhaps something like this would work:

^[a-zA-Z_][\w_\.\-]*$

I think treating invalid targets as bogus comments would work better than block-listing lit$ and would fix this issue.

Or even pithier, /^\p{ID_Start}[\p{ID_Continue}-]*$/uv - allow the unicode range for ID_start, followed by zero or more "ID_Continue or dash" (see https://www.unicode.org/reports/tr31/)

@zcorpan

zcorpan commented Feb 4, 2026

Copy link
Copy Markdown
Member

Or even pithier, /^\p{ID_Start}[\p{ID_Continue}-]*$/uv - allow the unicode range for ID_start, followed by zero or more "ID_Continue or dash" (see https://www.unicode.org/reports/tr31/)

That includes non-ASCII, right? I think the HTML parser intentionally doesn't branch for non-ASCII anywhere.

@noamr

noamr commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

Or even pithier, /^\p{ID_Start}[\p{ID_Continue}-]*$/uv - allow the unicode range for ID_start, followed by zero or more "ID_Continue or dash" (see https://www.unicode.org/reports/tr31/)

That includes non-ASCII, right? I think the HTML parser intentionally doesn't branch for non-ASCII anywhere.

Good point, it can be something like /^[A-Za-z][-A-Za-z0-9]*$/.
We can also allow . and/or _ if we really want to. But perhaps less is more.

@noamr noamr force-pushed the foolip/pi-parsing branch from 930868e to 070acbc Compare June 23, 2026 14:29

@jmdyck jmdyck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few minor suggestions. (Sorry if they're not in file order.)

Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated
Comment thread source Outdated

@hsivonen hsivonen 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.

LGTM. Thanks.

@zcorpan zcorpan 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.

LGTM % @jmdyck 's comments

@noamr

noamr commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

A few minor suggestions. (Sorry if they're not in file order.)

Thanks! All addressed.

@jmdyck jmdyck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two more nits after previous fixes.

Comment thread source Outdated
Comment thread source Outdated
@noamr

noamr commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Two more nits after previous fixes.

Thanks, fixed both.

@annevk annevk merged commit 320c05f into main Jun 25, 2026
2 checks passed
@annevk annevk deleted the foolip/pi-parsing branch June 25, 2026 09:07
annevk pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 26, 2026
lando-worker Bot pushed a commit to mozilla-firefox/firefox that referenced this pull request Jul 10, 2026
…ruction parsing tests, a=testonly

Automatic update from web-platform-tests
Untentative and fix HTML processing instruction parsing tests

For whatwg/html#12118.
--

wpt-commits: cf0642a4d2a851574afd696f566987f84f92849f
wpt-pr: 60828
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

7 participants