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.

Sign up for GitHub

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

Jump to bottom

Add test for </button> closing <p> #146

Merged
merged 2 commits into from
May 9, 2022
Merged

Add test for </button> closing <p> #146

merged 2 commits into from
May 9, 2022

Conversation

fb55
Copy link
Contributor

@fb55 fb55 commented May 8, 2022

Fixes #143

fb55 added a commit to inikulin/parse5 that referenced this pull request May 8, 2022
@fb55
fix(parser): Have </button> close <p>
7c32ab6 
Fixes #533

A PR with a test was opened at html5lib/html5lib-tests#146
@fb55 fb55 mentioned this pull request May 8, 2022
Merged
@Ms2ger Ms2ger merged commit 9f14a9a into html5lib:master May 9, 2022
@fb55 fb55 deleted the button-p branch May 9, 2022 07:49
@fb55 fb55 mentioned this pull request May 23, 2022
Open
@stevecheckoway
Copy link
Contributor

I don't believe <!doctype html><button><p></button>x has any errors and thus, I think the added test is incorrect.

My read of the standard is

  1. The parser starts in the "initial" insertion mode and reads a DOCTYPE token whose name is html. This switches to "before html";
  2. In the "before html" insertion mode, the <button> token is handled by "Anything else" which switches to "before head";
  3. In the "before head" insertion mode, the <button> token is handled by "Anything else" which switches to "in head";
  4. In the "in head" insertion mode, the <button> token is handled by "Anything else" which switches to "after head";
  5. In the "after head" insertion mode, the <button> token is handled by "Anything else" which switches to "in body";
  6. In the "in body" insertion mode, the <button> token is handled by "A start tag whose tag name is 'button'" which causes no errors because there is not a button element in scope;
  7. In the "in body" insertion mode, the <p> token is handled by "A start tag whose tag name is one of: … 'p'…";
  8. In the "in body" insertion mode, the </button> token is handled by "An end tag whose tag name is one of: … 'button' …" which generates implied end tags—this pops the p element off the stack of open elements—leaving the button element as the current node and thus not causing an error;
  9. In the "in body" insertion mode, the x token is handled by "Any other character token"; and
  10. In the "in body" insertion mode, the end-of-file token is handled by "An end-of-file token" which does not cause an error because the stack of open elements only contains html and body and the parsing ends here.

Was this a copy and paste error or have I missed something in the standard that causes these three errors?

@stevecheckoway stevecheckoway mentioned this pull request Jun 9, 2022
Merged
@alexander-akait
Copy link
Contributor

@stevecheckoway Yes, you are right, there are no errors in this test case

@fb55
Copy link
Contributor Author

fb55 commented Jun 26, 2022

Yeah, this was an oversight on my part. Sorry for that!

@flavorjones
Copy link
Contributor

@alexander-akait @fb55 Do you think it's possible to merge @stevecheckoway's PR at #149 if his analysis is correct?

@alexander-akait
Copy link
Contributor

@flavorjones Sorry, I don't have permissions to merge 😞

@fb55
Copy link
Contributor Author

fb55 commented Jul 10, 2022

I don't have permissions either. @Ms2ger how do you feel about adding additional collaborators to this repo?

@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 11, 2022

I don't have permissions to add anyone else, maybe @jgraham?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ms2ger Ms2ger Ms2ger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing coverage for <button><p></button>x
5 participants
@fb55 @stevecheckoway @alexander-akait @flavorjones @Ms2ger