HTML API: Fix A tag active formatting element loop missing break #7281
Conversation
|
@dmsnell for your review. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
A good test for this is this HTML: <i><a><table><td><a>The table creates a marker so the second This currently fails on nightly, but works correctly on this PR. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
A mistake in the original code handling opening A elements in the HTML Processor led to mistakes in parsing where the Processor would bail in situations when it could have proceeded. While this was errant behavior, it didn't violate the public contract since it would bail in these situations. This patch fixes the mistake, which was to only break out of the innermost loop instead of breaking from the containing loop, which resolves the issue. Developed in #7281 Discussed in https://core.trac.wordpress.org/ticket/61576 Follow-up to [56274]. Props jonsurrell. See #61576. git-svn-id: https://develop.svn.wordpress.org/trunk@58966 602fd350-edb4-49c9-b593-d223f7449a82
|
@sirreal I've folded this into Core-61576 even though it was a bug introduced with the original HTML Processor because the error behavior was to bail instead of return an invalid parse. If you think this is wrong we can re-classify, but it felt appropriate when I examined it. |
|
This leads me to wonder if some of the failure cases I've seen, which proliferate around |
A mistake in the original code handling opening A elements in the HTML Processor led to mistakes in parsing where the Processor would bail in situations when it could have proceeded. While this was errant behavior, it didn't violate the public contract since it would bail in these situations. This patch fixes the mistake, which was to only break out of the innermost loop instead of breaking from the containing loop, which resolves the issue. Developed in WordPress/wordpress-develop#7281 Discussed in https://core.trac.wordpress.org/ticket/61576 Follow-up to [56274]. Props jonsurrell. See #61576. Built from https://develop.svn.wordpress.org/trunk@58966 git-svn-id: http://core.svn.wordpress.org/trunk@58362 1a063a9b-81f0-0310-95a4-ce76da25c4cd
A mistake in the original code handling opening A elements in the HTML Processor led to mistakes in parsing where the Processor would bail in situations when it could have proceeded. While this was errant behavior, it didn't violate the public contract since it would bail in these situations. This patch fixes the mistake, which was to only break out of the innermost loop instead of breaking from the containing loop, which resolves the issue. Developed in WordPress/wordpress-develop#7281 Discussed in https://core.trac.wordpress.org/ticket/61576 Follow-up to [56274]. Props jonsurrell. See #61576. Built from https://develop.svn.wordpress.org/trunk@58966 git-svn-id: https://core.svn.wordpress.org/trunk@58362 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Yes, I believe some test failures around active format reconstruction were a result of this bug. I struggled with it when debugging some of the active format reconstruction and that was where I discovered this. |
A mistake in the original code handling opening A elements in the HTML Processor led to mistakes in parsing where the Processor would bail in situations when it could have proceeded. While this was errant behavior, it didn't violate the public contract since it would bail in these situations. This patch fixes the mistake, which was to only break out of the innermost loop instead of breaking from the containing loop, which resolves the issue. Developed in WordPress#7281 Discussed in https://core.trac.wordpress.org/ticket/61576 Follow-up to [56274]. Props jonsurrell. See #61576. git-svn-id: https://develop.svn.wordpress.org/trunk@58966 602fd350-edb4-49c9-b593-d223f7449a82
Fix a bug where a switch statement break should break the switch and the surrounding foreach loop to satisfy these conditions in the standard:
Markers and an
Aelement should break out of the loop that. As implemented now, the forEach will always continue up to the root.Trac ticket:
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.