HTML API: P end tag does not generate P element

[jonsurrell]

According to the ​HTML Standard, a P end tag with no open P elements should generate an empty P element:

An end tag whose tag name is "p"
If the stack of open elements does not have a p element in button scope, then this > is a parse error; insert an HTML element for a "p" start tag token with no attributes.

Close a p element.

---

Behavior

Expected:

<?php
$p = WP_HTML_Processor::create_fragment( '</p>' );
assert( $p->next_tag() );
assert( 'P' === $p->get_tag() );
assert( ! $p->is_tag_closer() );

Actual:

<?php
$p = WP_HTML_Processor::create_fragment( '</p>' );

// The following line errors, no tags are found.
assert( $p->next_tag() );
assert( 'P' === $p->get_tag() );
assert( ! $p->is_tag_closer() );

---

​The relevant code block is here where we seem to follow the specification, but no P element is actually added.

WIP: Currently this only adds a failing test.

Trac ticket: https://core.trac.wordpress.org/ticket/60287

[jonsurrell]

I found this thanks to the html5lib-tests proposed in #60227 (test input <p><hr></p>).

[dmsnell]

thanks @jonsurrell. this so-far hasn't been considered an error, but it raises the question noted in the roadmap for 6.5, which is to ask what to do when a tag implies changes to the document before the cursor. we're aware that there's a P element here, but we don't have a proper way to represent it yet.

if it weren't mostly benign I would have aborted when reaching an unexpected P tag closer, but there's not P tag opener to stop at, and the HTML Processor doesn't stop at tag closers, so it's not wrong either what it's doing, from one sense. it's not possible to target this P element to change it, and CSS selectors may not find the right thing, but the breadcrumbs should remain accurate in the presence of these unexpected closers.

We can probably handle this in a similar way that we have discussed handling a closing </br>, and add special-casing to note it and modify the document if it's modified.

until then, if you think it's important enough we can abort processing. I'm reluctant to do that because we do properly handle this the way the spec reads, we just don't stop on the implicitly-created P element to let people modify it.

I'm not sure this test is valid. As we've discussed in Slack, there is not P tag to stop at, even though one is implicitly created. The <p> is implied, but only after we pass the location in the document where it's implied to exist.

In some cases, the place where a tag is implied could literally be hundreds of KBs prior to the current tag. We know we don't want to support arbitrary forward lookahead, so stopping on an implied tag opener probably isn't going to be in the "hot path" of next_tag(), but we might want to find a way to communicate "there are implied changes to the document before now" where we could let someone back up and review the parsed state of the document.

I'll close this PR and conversation can continue on the ticket. I agree that this strictly wrong. The parser is aware that there's a p tag, we just don't have a way to visit it at this time. Adding an API to visit a tag that's created retroactively is an enhancement we can consider in the future.

[Ankit K Gupta]

Hi everyone, we discussed this ticket in Test Team Triage session today and found connected PR with this ticket has been closed, so removing the keywords has-patch, has-unit-tests and adding has-dev-note.

[jonsurrell]

There was an error in my last comment on the PR. The current behavior is NOT strictly wrong. We don't support any interaction with generated elements at this time.

[dmsnell]

Closing this for now even though it relates to a bigger question the HTML API needs to answer, which is how to communicate that a document has changed after scanning to a later point in it. We will create a separate design discussion for that outside of this ticket.

Since the HTML Processor doesn't support stopping at the implicitly-created P tag but also maintains the necessary semantic rules, this isn't entirely a defect.