HTMLEngine produces invalid HTML for self-closing tags

[sb8244]

Environment

• Elixir version (elixir -v): 1.12.2
• Phoenix version (mix deps): 1.5.12
• Phoenix LiveView version (mix deps): 0.16.1
• Operating system: OSX
• Browsers you attempted to reproduce this bug on (the more the merrier): Chrome
• Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: Yes

Actual behavior

When I have a tag like <div /> (with attributes, but doesn't matter here), the browser doesn't close the div tag. This causes elements to get put as children of the div, even though they are not defined as children.

Expected behavior

A self-closing HTML tag should either raise (invalid HTML) or close via the correct tag. So <div /> should turn into <div></div>.

If developers are expecting heex to feel like JSX (for better or worse), then the tag should become properly closed and not raised.

[chrismccord]

I wasn't aware react did self-closing for any existing HTML. Our current approach only self-closes function components, not HTML tags. I'm willing to be convinced but I'm unsure how I feel about stuff like <div />. @josevalim @msaraiva ?

[josevalim]

I agree we need to either raise or make it work. It seems a bit weird to allow HTML that is not really HTML on regular HTML nodes.

[msaraiva]

I feel fine about accepting <div/>. We already do it in Surface. I believe any self-closed HTML element that is not a void element (hr, br, etc.) should be translated to <element>...</element>. So we should have:

• <div/> -> <div></div>
• <hr/> -> <hr>

It make it easier for people to copy/paste code from XHTML or JSX based templates. In case we want to "impose" a specific style as the default later, I don't think it's up to the tokenizer/parser to do that. This could/should be done by the formatter instead.

[chrismccord]

@msaraiva sounds good. Is this something you have bandwidth to contribute?

[msaraiva]

Sure thing!

[sb8244]

Amazing, thanks @msaraiva