HTML API: Preserve internal cursor across updates. #4371
Conversation
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.
dmsnell
force-pushed
the
html-api/track-accumulated-shift
branch
from
bb5f995
to
55ead4f
Compare
|
Instead of introducing a new concept of accumulated shift, could this be just another bookmark? |
|
@adamziel at the moment no because this has to track that shift without moving It's different than a bookmark because A good question, and one I hope we can possibly answer by getting rid of things, but I am skeptical we will be able to. |
|
@adamziel removed the "duality" of more tests pass now, including those that aren't part of this PR, and there are fewer code paths and fewer things in the processor. |
dmsnell
changed the title
|
Thank you very much for working on this @dmsnell ! I've tested this against WordPress/block-interactivity-experiments#141, and I'm happy to confirm that it fixes all failing unit tests that I added there while attempting to pin-point the issue! 🎉 Outside of unit tests, in markup produced by the Movies Demo, there is one extraneous <template data-wp-show="selectors.wpmovies.isVideosTab"><template ><div aria-hidden
role="tabpanel"
data-wp-bind.aria-hidden="selectors.wpmovies.isImagesTab"
aria-labelledby="wpmovies-videos-tab"
>
<div class="...
Interestingly, that is the same behavior that I get when using Core This includes another instance of three nested extraneous <template data-wp-show="selectors.wpmovies.isPlaying"><template ><template ><template ><div id="wp-movies-video-player" class="wpmovies-video-player wp-block-wpmovies-video-player">
<div class="wpmovies-video-wrapper">... that I originally thought were due to repeated processing of inner blocks and thus could be fixed by applying my other commit (WordPress/block-interactivity-experiments@a8cadac, later extracted into WordPress/block-interactivity-experiments#227). However, I discovered that that purported fix introduced another issue 🤦 Anyway, I don't think the latter is a blocker for this PR. I also don't think we need to spend much more time on investigating that latter issue; I believe I've found a blunt-yet-effective workaround for it: WordPress/block-interactivity-experiments@3dbff3f (pushed to WordPress/block-interactivity-experiments#141). Since testing fixes in Core against the Movies Demo with WordPress/block-interactivity-experiments#141 is also kinda finicky and time-consuming (I ended up destroying my local |
|
Committed to Core |
|
Committed to Core's |
In this "blessed" patch we're backporting some udpates coming from Core in the Tag Processor, namely a major fix to a rare bug that occurs when making changes and then seeking to an earlier position in a document. - Preserve internal cursor across updates [#4371](WordPress/wordpress-develop#4371)
In this "blessed" patch we're backporting some udpates coming from Core in the Tag Processor, namely a major fix to a rare bug that occurs when making changes and then seeking to an earlier position in a document. - Preserve internal cursor across updates [#4371](WordPress/wordpress-develop#4371) - Update code example formatting to fix Developer Resources docs [#4419](WordPress/wordpress-develop#4419) - Update whitespace formatting to pass Gutenberg linter [#4433](WordPress/wordpress-develop#4422)
Trac #58179-ticket
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 #4355.
In few words, the Tag Processor has not been treating its own internal pointer
bytes_already_parsedthe same way it treats its bookmarks. That is, when updates are applied to the input document and thenget_updated_html()is called, the internal pointer transfers to the newly-updated content as if no updates had been applied since the previous call toget_updated_html().In this patch we're refactoring how the Tag Processor applies enqueued changes. Previously there was a two-step process of appending updates into its
$this->output_bufferand maintaining a tracker$this->bytes_already_copiedfor how much of the input HTML had already been copied into the output buffer, and then of flushing everything into the buffer and then swapping it into the new "input HTML" so the processor can restart with all the updates applied. Now the two-step process has been collapsed into one step and the buffering process has been removed from the class. After everynext_tag()invocation the entire set of enqueued changes is flushed into the updated HTML and then the internal cursor is adjusted and reset to point into the updated HTML into the same spot it was pointing in the original input.