fix: Post Content dynamic value reads the loop globals instead of its context post - #2939
Open
lucadobrescu wants to merge 2 commits into
Open
fix: Post Content dynamic value reads the loop globals instead of its context post#2939lucadobrescu wants to merge 2 commits into
lucadobrescu wants to merge 2 commits into
Conversation
Contributor
Bundle Size Diff
|
pirate-bot
added
the
pr-checklist-complete
Contributor
|
Plugin build for fd9560c is ready 🛎️!
|
Contributor
E2E TestsPlaywright Test Status: See serial and parallel matrix jobs Performance ResultsserverResponse: {"q25":366,"q50":389.3,"q75":412.5,"cnt":10}, firstPaint: {"q25":1373.1,"q50":1519.1,"q75":1738.5,"cnt":10}, domContentLoaded: {"q25":3647.5,"q50":3697.45,"q75":3785.2,"cnt":10}, loaded: {"q25":3649.5,"q50":3699.5,"q75":3787.4,"cnt":10}, firstContentfulPaint: {"q25":4141.3,"q50":4187.6,"q75":4278.9,"cnt":10}, firstBlock: {"q25":14723.3,"q50":14775.2,"q75":14855.5,"cnt":10}, type: {"q25":27.58,"q50":28.62,"q75":30.29,"cnt":10}, typeWithoutInspector: {"q25":23.5,"q50":25.05,"q75":26.29,"cnt":10}, typeWithTopToolbar: {"q25":34.89,"q50":36.55,"q75":38,"cnt":10}, typeContainer: {"q25":17.13,"q50":17.96,"q75":18.83,"cnt":10}, focus: {"q25":142.81,"q50":147.21,"q75":150.55,"cnt":10}, inserterOpen: {"q25":44.4,"q50":48.08,"q75":52.45,"cnt":10}, inserterSearch: {"q25":16.5,"q50":17.72,"q75":19,"cnt":10}, inserterHover: {"q25":6.17,"q50":6.45,"q75":6.99,"cnt":20}, loadPatterns: {"q25":1828.5,"q50":1848.91,"q75":1867.74,"cnt":10}, listViewOpen: {"q25":241.99,"q50":248.99,"q75":258.74,"cnt":10} |
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
Nested-tag guard coverage and E2E resource isolation remain incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Fixes Post Content dynamic values so they resolve against their context post rather than loop globals.
Changes:
- Passes the context post as
get_the_content()’s third argument. - Adds PHP and E2E regression coverage.
- Adds an E2E-only corrupted-loop-global test rig.
File summaries
| File | Description |
|---|---|
inc/plugins/class-dynamic-content.php |
Corrects context post resolution. |
tests/test-dynamic-content.php |
Adds PHP regression tests. |
src/blocks/test/e2e/blocks/dynamic-content-frontend.spec.js |
Adds frontend regression coverage. |
packages/e2e-tests/mu-plugins/otter-e2e-bootstrap.php |
Simulates corrupted loop globals. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| return $data; | ||
| } | ||
| $content = get_the_content( $data['context'] ); | ||
| $content = get_the_content( null, false, $data['context'] ); |
Comment on lines
+19
to
+23
| // The Query Loop block has no include/post__in arg, so the loop is | ||
| // scoped to the target post via a unique search token in its title. | ||
| await requestUtils.createPost({ | ||
| title: 'Dynamic content target frontier2929', | ||
| content: `<!-- wp:paragraph --><p>${ TARGET_CONTENT }</p><!-- /wp:paragraph -->`, |
lucadobrescu
pushed a commit
that referenced
this pull request
Aug 3, 2026
Addresses the Copilot review on #2939. The guard in mark_exceptions() is what the get_the_content() argument fix actually changes: it reads the context post's content to decide whether a nested postContent tag would recurse. Both mismatch cases are now covered - a nested tag in the context post must fire the guard, and a nested tag in the loop-global post must not blank a clean context post. The guard is asserted directly before rendering because an unfired guard makes apply_dynamic_content() recurse until the process dies, so the test has to fail fast instead of hanging. The e2e spec namespaced its Query Loop search token per run and now deletes its own post and page in afterAll. wp-env is persistent and the fixed token had already accumulated twelve records over six local runs, any of which the perPage:1 loop could resolve instead of the run's own post. Teardown is best-effort per record so one failed request cannot orphan the others. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Ready to approve
The API correction is appropriate and comprehensively covered by focused unit and frontend tests.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
+744
to
+755
| /** | ||
| * Issue #2929 rig: with ?otter_e2e_corrupt_pages=1, mimic a theme/plugin that | ||
| * clobbers the $pages loop global (main templates are included at global scope, | ||
| * so any template-level $pages variable overwrites it) while Otter evaluates a | ||
| * dynamic tag, and surface PHP notices in the output so the spec can assert | ||
| * none are emitted. | ||
| * | ||
| * The corruption is scoped to blocks carrying an <o-dynamic> tag and restored | ||
| * straight after Otter's filter (priority 10): leaving it in place for every | ||
| * block would make core's own the_content() warn too, which is core behavior | ||
| * rather than the bug under test. | ||
| */ |
Contributor
There was a problem hiding this comment.
we can make this less verbose
Soare-Robert-Daniel
requested changes
Aug 3, 2026
lucadobrescu
changed the base branch from
development
to
fix/2937-wp-filesystem-guard
August 3, 2026 08:39
…ntext post get_the_content() takes the post as its third argument; the context post ID was passed as the first one ($more_link_text), so $post stayed null and core fell back to the loop globals. When a theme or plugin leaves those globals clobbered, post-template.php warns "Undefined array key -1" (plus a preg_match() deprecation on PHP 8) and the tag renders empty. Fixes #2929 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Addresses the Copilot review on #2939. The guard in mark_exceptions() is what the get_the_content() argument fix actually changes: it reads the context post's content to decide whether a nested postContent tag would recurse. Both mismatch cases are now covered - a nested tag in the context post must fire the guard, and a nested tag in the loop-global post must not blank a clean context post. The guard is asserted directly before rendering because an unfired guard makes apply_dynamic_content() recurse until the process dies, so the test has to fail fast instead of hanging. The e2e spec namespaced its Query Loop search token per run and now deletes its own post and page in afterAll. wp-env is persistent and the fixed token had already accumulated twelve records over six local runs, any of which the perPage:1 loop could resolve instead of the run's own post. Teardown is best-effort per record so one failed request cannot orphan the others. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lucadobrescu
force-pushed
the
fix/2929-dynamic-content-globals
branch
from
August 3, 2026 09:17
fe7dc90 to
fd9560c
Compare
This file contains hidden or 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
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.
Closes #2929.
Summary
The Post Content dynamic value read the loop globals instead of the post it was bound to. When a theme or plugin left those globals clobbered, WordPress logged
Undefined array key -1 in wp-includes/post-template.php on line 330plus apreg_match()deprecation, and the tag rendered nothing.get_the_content()takes the post as its third argument. Otter passed the context post ID as the first one, which is$more_link_text, so the post argument stayednulland core fell back to$pages,$pageand the other loop globals.Post Content dynamic value — resolves from its own context post. The output no longer depends on the state of the loop globals.
mark_exceptions()— the infinite-loop guard reads the same context post. Before, it inspected whichever post the globals pointed at, so it could miss a nestedpostContenttag or flag one that was not there.Note
Both call sites had the same argument mistake, so a page could emit the warning twice per dynamic tag. The other
get_the_content()callers in the plugin already pass the post correctly and are untouched.Render flow
flowchart LR A[Block with<br/>Post Content tag] --> B[Resolve the<br/>context post ID] B --> C[Changed:<br/>pass the post as<br/>the 3rd argument]:::changed C --> D{Loop globals<br/>clobbered?} D -- Yes --> E[Post content renders] D -- No --> E classDef changed fill:#9a6700,color:#fff,stroke:#5c3d00,stroke-width:3px,stroke-dasharray:6 3Test instructions
Open
wp-config.php. SetWP_DEBUGtotrue, setWP_DEBUG_LOGtotrue, and setWP_DEBUG_DISPLAYtofalse. Deletewp-content/debug.logif the file exists.Go to
WP Admin → Posts → Add New. Add a paragraph with the textTarget body copy. Give the post the titleDynamic target. Publish the post.Go to
WP Admin → Pages → Add New. Open the editor options menu (three dots, top right) and select Code editor. Paste this markup, then switch back to the visual editor and publish:Expect: the editor shows a Query Loop that contains one Dynamic Value placeholder.
Select View Page to open the page on the front end.
Expect: the page shows
Target body copy. No PHP notice appears on the page.Open
wp-content/debug.log.Expect: the file has no
Undefined array key -1entry and nopreg_match(): Passing nullentry forpost-template.php.Build the same page through the interface instead of markup, to check the editor path: add a Query Loop block, insert a paragraph inside it, select Dynamic Value in the block toolbar, choose Post Content in the Dynamic Value by Otter modal, and publish.
Expect: the front end shows the looped post's content, and
debug.logstays clean.Checklist before the final review