Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"p" tag can't have close tag? #5049

Closed
canadaduane opened this issue Jun 21, 2020 · 6 comments · Fixed by #5060
Closed

"p" tag can't have close tag? #5049

canadaduane opened this issue Jun 21, 2020 · 6 comments · Fixed by #5060
Labels
awaiting submitter needs a reproduction, or clarification

Comments

@canadaduane
Copy link

canadaduane commented Jun 21, 2020

Describe the bug

The </p> tag is not parsed correctly:

<script>
	let name = 'world';
</script>

<p>
	<pre>{name}</pre>
</p>

The result is: </p> attempted to close an element that was not open (7:0)

Information about your Svelte project:

  • First-time user, just trying Svelte out
  • used the REPL

Severity

  • "Papercut"

Additional context

  • This was a pretty surprising result for me as a first-time user--I'm converting some HTML to Svelte and didn't understand why it wouldn't parse.
@canadaduane canadaduane changed the title "pre" tag can't be used? "p" tag can't have close tag? Jun 21, 2020
@PatrickG
Copy link
Member

PatrickG commented Jun 21, 2020

The p element is not allowed to have a block element child.
See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/p

That means, p is auto-closed right before the pre element.

That would work:

<script>
	let name = 'world';
</script>

<p>
<pre>{name}</pre>

@pngwn
Copy link
Member

pngwn commented Jun 21, 2020

Yes. This is actually invalid HTML but browsers will do their best with it regardless. Svelte is much stricter and won’t parse invalid HTML. The auto closing behaviour is a little confusing, however.

@Conduitry
Copy link
Member

I'm not sure what preferred behavior would be here. Browsers really do autoclose the <p> upon reaching a <pre>. Are you just thinking more specific compiler error messages? Like if the browser sees an erroneous closing tag for something that it had previously automatically closed, it adjust the error message?

@pngwn
Copy link
Member

pngwn commented Jun 21, 2020

Yeah, I think so.

Browsers do autoclose paragraphs, amongst many other things they do to recover from invalid code but they also don't throw parsing errors. I don't think many of the 'lets try to make it work' features of browsers are widely known. With the above example, not only does chrome autoclose the first p but it autoopens the second.

Ideally when seeing a close tag without an open tag, after autoclosing a tag of the same type, we could give a more descriptive error. However, I don't know if the parser is setup to handle this kind of error reporting. We would need to make sure we retained that information. There are possibly a bunch of edge cases that could make this doubly confusing.

@Conduitry
Copy link
Member

I'm removing the dev warning label because my understanding of that one is that it's for additional runtime checks that are put in the generated code only when compiling in dev mode. This issue is for an adjustment to the message that is given in a compile-time error that is unrelated to dev mode.

@Conduitry
Copy link
Member

In 3.24.1 there's now a more helpful error message if the parser encounters a closed tag that previously had been autoclosed by opening another tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants