HTML API: Reduce skip_script_data length checks

[sirreal]

Trac ticket: Core-63738

Reduce the number of length checks in WP_HTML_Tag_Processor::skip_script_data().

---

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.

[github-actions]

Test using WordPress Playground

The 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

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[github-actions]

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props jonsurrell, dmsnell.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[dmsnell]

this patch is good, but before we merge, can we also note in this comment that we’re looking for other strings and they are covered here? I think the comment at the moment is slightly misleading, but we also check for <!-- for instance, and it’s currently incidental I think that terminating false is the same for all places within the SCRIPT.

imagine, however, that we wanted to signal something different if we failed to exit the SCRIPT because there was no end-tag vs. that we ended inside some escaped state? then, by returning early at this point we removed our ability to find the <!-- that transitions state.

I believe in review that the code is solid, but I want to make sure we leave this note for the future as a warning to reconsider the check if changing things. I don’t know that these are all currently caught in the tests (which of course would be another great way to cover this risk, by adding new tests for each of these cases, if we can properly think them up).

[sirreal]

Good point, I've updated the comment to clarify things.

I do have a number of tests to add to this. I can push them to this branch.

Given how complicated script tag parsing is, I'm tempted to extract them to their own test suite like wpHtmlTagProcessor-scriptParsing. Do you have thoughts on that?

[sirreal]

I pulled in your comment changes from #9397 and made some further tweaks. I'm pretty happy with it now.

[dmsnell]

your third example in the comment is perfect. I was probably lazy in not suggesting that in my comment.

[dmsnell]

extract them to their own test suite like wpHtmlTagProcessor-scriptParsing. Do you have thoughts on that?

not that I consider it a huge priority, but I think this is a great idea and would make it clearer for us to be more comprehensive in the tests. I think for every branch of the state machine in the HTML spec we could have data providers generating example inputs for them.

[sirreal]

I've updated the comment and added a number of tests for different script data states. @dmsnell I'd appreciate if you could take another quick look.

EDIT: Additional review happened in #9397.

[sirreal]

I've pulled the updated comment and moved the test adjacent to the other script parsing test, things which were noted in #9397.

[sirreal]

Merged in [60617].