-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Stop reporting redundant fragment parser error in foreign content #9809
base: main
Are you sure you want to change the base?
Stop reporting redundant fragment parser error in foreign content #9809
Conversation
d119c56
to
d04ee3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hsivonen can you take a look please?
|
||
<li><p>Otherwise, process the token according to the rules given in the section corresponding | ||
to the current <span>insertion mode</span> in HTML content.</li> | ||
<li><p>Return to the step labeled <i>loop</i>.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Rewriting this set of steps as a https://infra.spec.whatwg.org/#iteration-while "While true" could be nice.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I don't really find "While true" any better than "loop" ... it's just as undescriptive regarding when the loop will terminate. In any case I think such a change should be done in a followup PR ... so that the diff of this clarification is minimal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While true with nested steps is much better than goto imo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes that's true ... there however aren't any nested steps here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, @annevk suggests switching from goto without nested steps to while true with nested steps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right ... I think Anne meant that the real advantage of while is if you have multiple nested loops that is a loop within a loop, which as I pointed out isn't the case here.
In any case as I said I'd like to keep the changes in this commit minimal (since it fixes something that's wrong) and keep editorial changes to future PRs.
I have tested this change against the test cases from html5lib-tests. In document parsing this doesn't change any test results. In fragment parsing this fixes 7 test cases by reporting one parser error less. (That is all existing HTML parsers that pass html5lib-tests must have already implemented these steps in the order they are changed to by this commit.) The test cases where one error is reported less fall in two categories: * fragment parsing e.g. `<svg></table>` with a `tbody` HTML element as the context element => one "unexpected end tag" error is reported less (previously it was reported twice) * fragment parsing e.g. `<g></path>` with a `path` SVG element as the context element => one "found special tag while closing generic tag" error is no longer reported Fixes html5lib/html5lib-tests#173.
d04ee3d
to
1d479e3
Compare
I have tested this change against the test cases from html5lib-tests.
In document parsing this doesn't change any test results.
In fragment parsing this fixes 7 test cases by reporting one parser error less. (That is all existing HTML parsers that pass html5lib-tests must have already implemented these steps in the order they are changed to by this commit.)
The test cases where one error is reported less fall in two categories:
fragment parsing e.g.
<svg></table>
with atbody
HTML element as the context element => one "unexpected end tag" error is reported lessfragment parsing e.g.
<g></path>
with apath
SVG element as the context element => one "found special tag while closing generic tag" error is no longer reportedFixes html5lib/html5lib-tests#173.
/parsing.html ( diff )