Self-closing tag on void elements shouldn't be a parse error/warning in Dom\HTMLParser #17485

xPaw · 2025-01-16T15:01:02Z

Description

The following code:

<?php
$Data = "<!DOCTYPE HTML>\n<br />\n<input />";
$Document = \Dom\HTMLDocument::createFromString( $Data );
echo $Document->saveHTML();

Resulted in this output:

Warning: Dom\HTMLDocument::createFromString(): tree error non-void-html-element-start-tag-with-trailing-solidus in Entity, line: 2, column: 2-3 in test.php on line 3
Warning: Dom\HTMLDocument::createFromString(): tree error non-void-html-element-start-tag-with-trailing-solidus in Entity, line: 3, column: 2-6 in test.php on line 3

I believe the html standard specifically allows this on void elements: ref

Then, if the element is one of the void elements, or if the element is a foreign element, then there may be a single U+002F SOLIDUS character (/), which on foreign elements marks the start tag as self-closing. On void elements, it does not mark the start tag as self-closing but instead is unnecessary and has no effect of any kind. For such void elements, it should be used only with caution — especially since, if directly preceded by an unquoted attribute value, it becomes part of the attribute value rather than being discarded by the parser.

it doesn't warn for tags like <div /> and simply opens it as normal.

relevant code:

php-src/ext/dom/lexbor/lexbor/html/tree.h

Lines 334 to 339 in 5c39a46

    
           bool is_void = lxb_html_tag_is_void(token->tag_id); 
        
           if (is_void) { 
        
               lxb_html_tree_parse_error(tree, token, 
        
                                         LXB_HTML_RULES_ERROR_NOVOHTELSTTAWITRSO); 
        
           }

PHP Version

8.4.3

Operating System

No response

The text was updated successfully, but these errors were encountered:

xPaw · 2025-01-16T16:53:41Z

@lexborisov should I have opened this in your repo?

lexborisov · 2025-01-16T17:19:36Z

@xPaw

It looks like it to me.

This is where you have to look at the tree construction specification. It is written there:

https://html.spec.whatwg.org/multipage/parsing.html#acknowledge-self-closing-flag

When a start tag token is emitted with its self-closing flag set, if the flag is not acknowledged when it is processed by the tree construction stage, that is a non-void-html-element-start-tag-with-trailing-solidus parse error.

xPaw · 2025-01-16T17:22:44Z

Hmm, that seems confusing to me as to what's the correct implementation for a parser. Browsers obviously accept this.

Edit: actually that appears to be correct, since void elements should be acceptable with the slash? Even the error string implies non-void.

lexborisov · 2025-01-16T17:29:47Z

@xPaw

Hmm, that seems confusing to me as to what's the correct implementation for a parser. Browsers obviously accept this.

Edit: actually that appears to be correct, since void elements should be acceptable with the slash?

After reading the spec sheet again,
The problem seems to be solved by changing the condition in the code, to negation.
https://github.com/lexbor/lexbor/blob/58c88147db7342aefbc45d5e5621a974c210106a/source/lexbor/html/tree.h#L333

if (!is_void) {

xPaw · 2025-01-16T17:35:16Z

This is how I read it too, but assumed it was too simple of a fix.

nielsdos · 2025-01-16T18:28:45Z

I also think the check should probably be negated.

lexborisov · 2025-01-16T18:53:46Z

@xPaw @nielsdos

Yeah, it all adds up. I had doubts, because in the code the <param> tag used to be void, but then it was excluded and I didn't notice it, unfortunately.

I'll get the patch ready.

lexborisov · 2025-01-16T19:16:59Z

Fixed in lexbor/lexbor@e39083b and lexbor/lexbor@ae97abf

Per report from Pavel Djundik. This related #17485 issue on GitHub.

nielsdos · 2025-01-16T19:25:33Z

@lexborisov Thanks a lot! I'm not sure this is entirely correctly fixed however. After cherry-picking your commits into PHP I get a new warning for this line in this test:

php-src/ext/dom/tests/modern/html/interactions/HTMLDocument_getElementsByTagName.phpt

Line 18 in a4fa1e7

It's a foreign element (in SVG) and I believe it shouldn't warn here based on the spec comments.

xPaw · 2025-01-16T19:41:39Z

Just to add about svgs (before the fix at least), I've noticed that <path /> would turn into <path></path> when calling saveHtml.

nielsdos · 2025-01-16T19:44:12Z

@xPaw that is at least consistent with my browser. Try this JS:

html = `<!DOCTYPE html>
<html>
<body>
    <svg width="100" height="100">
        <path />
    </svg>
</body>
</html>`;
dom = (new DOMParser).parseFromString(html, 'text/html');
dom.body.innerHTML

lexborisov · 2025-01-16T19:46:51Z

@nielsdos

@lexborisov Thanks a lot! I'm not sure this is entirely correctly fixed however. After cherry-picking your commits into PHP I get a new warning for this line in this test:

php-src/ext/dom/tests/modern/html/interactions/HTMLDocument_getElementsByTagName.phpt

Line 18 in a4fa1e7
     <circle cx="0" cy="0" r="10"/> 
It's a foreign element (in SVG) and I believe it shouldn't warn here based on the spec comments.

I figured it out, void elements are only checked in the HTML namespace, in other cases it's always false.
But I didn't find any direct text about it. It just comes out of the references in the specification.

https://html.spec.whatwg.org/multipage/syntax.html#elements-2
https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inforeign "Any other start tag"

xPaw · 2025-01-16T19:50:13Z

@lexborisov Do you mean this?

Foreign elements must either have a start tag and an end tag, or a start tag that is marked as self-closing, in which case they must not have an end tag.

lexborisov · 2025-01-16T19:53:39Z

@lexborisov Do you mean this?

Foreign elements must either have a start tag and an end tag, or a start tag that is marked as self-closing, in which case they must not have an end tag.

But then it is not quite clear why the specification requires to call acknowledge-self-closing-flag for Foreign elements start tag.

Otherwise
Pop the current node off the stack of open elements and acknowledge the token's self-closing flag.

xPaw · 2025-01-16T20:04:31Z

Seems like acknowledge disables the error:

When a start tag token is emitted with its self-closing flag set, if the flag is not acknowledged when it is processed by the tree construction stage, that is a non-void-html-element-start-tag-with-trailing-solidus parse error.

lexborisov · 2025-01-16T20:10:05Z

Seems like acknowledge disables the error:

When a start tag token is emitted with its self-closing flag set, if the flag is not acknowledged when it is processed by the tree construction stage, that is a non-void-html-element-start-tag-with-trailing-solidus parse error.

It seems to be, but it would seem that it will always be false for Foreign elements. What's the point of calling it in then?
Ok, I'll reread the spec sheet again and if anything, I'll contact the folks at WATHWG.

xPaw · 2025-01-16T20:13:08Z

The spec is kind of all over the place, but this would imply that it gets set to true during tag name parse:

U+002F SOLIDUS (/)
Switch to the self-closing start tag state.

3.2.5.40 Self-closing start tag state
Consume the next input character:

U+003E GREATER-THAN SIGN (>)
Set the self-closing flag of the current tag token. Switch to the data state. Emit the current tag token.

xPaw added Bug Status: Needs Triage labels Jan 16, 2025

xPaw referenced this issue in lexbor/lexbor Jan 16, 2025

HTML: fixed error report on self-closing tag parsing.

ae97abf

Per report from Pavel Djundik. This related #17485 issue on GitHub.

nielsdos added Extension: dom Status: Verified and removed Status: Needs Triage labels Jan 16, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Self-closing tag on void elements shouldn't be a parse error/warning in Dom\HTMLParser #17485

Self-closing tag on void elements shouldn't be a parse error/warning in Dom\HTMLParser #17485

xPaw commented Jan 16, 2025 •

edited

Loading

xPaw commented Jan 16, 2025

lexborisov commented Jan 16, 2025

xPaw commented Jan 16, 2025 •

edited

Loading

lexborisov commented Jan 16, 2025

xPaw commented Jan 16, 2025

nielsdos commented Jan 16, 2025

lexborisov commented Jan 16, 2025

lexborisov commented Jan 16, 2025

nielsdos commented Jan 16, 2025

xPaw commented Jan 16, 2025

nielsdos commented Jan 16, 2025

lexborisov commented Jan 16, 2025 •

edited

Loading

xPaw commented Jan 16, 2025

lexborisov commented Jan 16, 2025

xPaw commented Jan 16, 2025

lexborisov commented Jan 16, 2025

xPaw commented Jan 16, 2025 •

edited

Loading

Self-closing tag on void elements shouldn't be a parse error/warning in Dom\HTMLParser #17485

Self-closing tag on void elements shouldn't be a parse error/warning in Dom\HTMLParser #17485

Comments

xPaw commented Jan 16, 2025 • edited Loading

Description

PHP Version

Operating System

xPaw commented Jan 16, 2025

lexborisov commented Jan 16, 2025

xPaw commented Jan 16, 2025 • edited Loading

lexborisov commented Jan 16, 2025

xPaw commented Jan 16, 2025

nielsdos commented Jan 16, 2025

lexborisov commented Jan 16, 2025

lexborisov commented Jan 16, 2025

nielsdos commented Jan 16, 2025

xPaw commented Jan 16, 2025

nielsdos commented Jan 16, 2025

lexborisov commented Jan 16, 2025 • edited Loading

xPaw commented Jan 16, 2025

lexborisov commented Jan 16, 2025

xPaw commented Jan 16, 2025

lexborisov commented Jan 16, 2025

xPaw commented Jan 16, 2025 • edited Loading

xPaw commented Jan 16, 2025 •

edited

Loading

xPaw commented Jan 16, 2025 •

edited

Loading

lexborisov commented Jan 16, 2025 •

edited

Loading

xPaw commented Jan 16, 2025 •

edited

Loading