HTML API: Add test case to demonstrate issue with seeking after modifying #4355
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dmsnell
added a commit
to dmsnell/wordpress-develop
that referenced
this pull request
Apr 23, 2023
While working on WordPress/block-interactivity-experiments#141 @ockham discovered that the Tag Processor was making a small mistake. After making some updates the parser wasn't returning to the start of the affected tag. A test case was created in WordPress#4355. In few words, the Tag Processor has not been treating its own internal pointer `bytes_already_parsed` the same way it treats its bookmarks. That is, when updates are applied to the input document and then `get_updated_html()` is called, the internal pointer transfers to the newly-updated content as if no updates had been applied since the previous call to `get_updated_html()`. In this patch we're creating a new "shift accumulator" to account for all of the updates that accrue before calling `get_updated_html()`. This accumulated shift will be applied when swapping the input document with the output buffer, which should result in the pointer pointing to the same logical spot in the document it did before the udpate. In effect this patch adds a single workaround for treating the internal pointer like a bookmark, plus a temporary pointer which points to the beginning of the current tag when calling `get_updated_html()`. This will preserve the assumption that updating a document doesn't move that pointer, or shift which tag is currently matched.
Closed
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In WordPress/block-interactivity-experiments#141, we found an issue with the Tag Processor that seems to be vaguely related to
seeking after adding an attribute. We tried to distill this into a minimal test case for which @dmsnell found a fix in #4345.Unfortunately, applying that fix to WordPress/block-interactivity-experiments#141 proved insufficient -- the issue persisted 😕
Part of the problem is that it’s fairly hard to reproduce issues that we’ve found in the wild (e.g. when running the Movies Demo against WordPress/block-interactivity-experiments#141). I’ve been moderately successful at isolating a somewhat scaled-down test there, which -- while still not fully capturing all issues -- at least fails even with #4345 applied. Unfortunately, it involves an extra class with a newly introduced class method 😕
I’ve decided to carry this test over to this repo so we have at least a baseline to try our fixes out with, even though it’s far from minimal for now.
Trac ticket: https://core.trac.wordpress.org/ticket/58160
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.