Fix logic for seeking during optimization loop to prevent emitting seek() notices #1376
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
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
westonruter
added
[Type] Bug
thelovekesh
approved these changes
Jul 18, 2024
westonruter
changed the title
westonruter
changed the title
thelovekesh
approved these changes
Jul 18, 2024
| @@ -309,6 +308,79 @@ public function data_provider_test_od_optimize_template_output_buffer(): array { | |||
| ', | |||
| ), | |||
|
|
|||
| 'many_images' => array( | |||
| * @return int Count of seek() calls. | ||
| */ | ||
| public function get_seek_count(): int { | ||
| return $this->seek_count; |
| static function () { | ||
| $tags = array(); | ||
| for ( $i = 1; $i < WP_HTML_Tag_Processor::MAX_SEEK_OPS + 1; $i++ ) { | ||
| $tags[] = sprintf( '<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[%d][self::IMG]" src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800" loading="lazy">', $i ); |
github-actions
bot
temporarily deployed
to
wp.org plugin: optimization-detective
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.
Summary
As observed by @thelovekesh, Optimization Detective is causing many notices to show up in the error log:
This is due to my incorrect implementation of the optimization loop, where it is seeking back to the current tag after calling a visitor, even if that visitor never seeked (sought?) or called
next_tag(). I guess HTML Tag Processor should actually short-circuit if the bookmark being seeked to is the same as the current cursor (bytes_already_parsed). In any case, we can work around this by exposing theseek_countwith a getter and only seeking back to the current tag if the seek count has increased and doing the same by counting the number of times thatnext_token()was called and seeking back if it was incremented.This also fixes a bug with a translation string I noticed.
optimization-detectiveImportant
Stable tag change: 0.4.0 → 0.4.1
svn status:svn diff