Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent query block from looping in classic themes #43221

Merged
merged 2 commits into from
Aug 19, 2022

Conversation

draganescu
Copy link
Contributor

What?

Closes #43198

Why?

In classic themes templates depend on have_posts() to return false to exit the loop. The change added by #40656
uses the global WP_Query to figure out global query when inherit is on. This in turn loops over the global posts
object and resets it once it reaches the end. Thus when rendering posts and meeting one post using the query loop block the
upper most have_posts is never false (because the global posts always get reset).

How?

An easy and quick fix is to clone the global WP_Query. This allows the block to render the loop without resetting the
global posts. This way the block renders the correct loop but does not interfere with the global loop.

Testing Instructions

  1. Using this Gutenberg and this PR
  2. Activate a classic theme
  3. Create a post and add a query block in it
  4. Set up the block by clicking:
    a) choose
    b) then choose again in the modal
  5. In the block inspector toggle on "Inherit query from template"
  6. Publish the post
  7. Visit the post
  8. There should be no endless loop, the page should render all right.

Screenshots or screencast

N/A

@draganescu draganescu marked this pull request as ready for review August 15, 2022 07:58
@draganescu draganescu requested a review from ajitbohra as a code owner August 15, 2022 07:58
@draganescu draganescu added [Type] Bug An existing feature does not function as intended [Block] Query Loop Affects the Query Loop Block Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release Backport to Gutenberg Minor Release Pull request that needs to be backported to a Gutenberg minor release Needs Technical Feedback Needs testing from a developer perspective. labels Aug 15, 2022
@anton-vlasenko anton-vlasenko self-requested a review August 16, 2022 11:27
Copy link

@martinkrcho martinkrcho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cloning the WP_Query class should be fine as long as none of the properties that are objects are modified (read more on this topic in Don't clone your php objects, DeepCopy them).

Otherwise the global query would be working with altered objects later on. That doesn't seem to be the case here since the query is used to loop through a list of posts and render them. Even though I would recommend running some tests where the query parameters include meta and taxonomy queries.

However, I think the post data should be reset using wp_reset_postdata further down in code even when the global query is used.

Copy link

@martinkrcho martinkrcho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested changes look good to me. As long as the block only loops through the posts (rather than changing the query properties that are objects), cloning is sufficient here.

@draganescu draganescu force-pushed the fix/query-loop-loop branch from 9e2ec4f to 78a0eb4 Compare August 16, 2022 15:02
@oandregal oandregal removed the Backport to Gutenberg Minor Release Pull request that needs to be backported to a Gutenberg minor release label Aug 17, 2022
@oandregal
Copy link
Member

oandregal commented Aug 17, 2022

Removing the "Backport to Gutenberg RC" as this bug seems to have been introduced in April, not during the 13.9 release cycle. It can go in the next Gutenberg release.

@draganescu draganescu merged commit 959c00f into trunk Aug 19, 2022
@draganescu draganescu deleted the fix/query-loop-loop branch August 19, 2022 08:21
@draganescu
Copy link
Contributor Author

Merged w/ failing react native tests as to me they seemed unrelated 🙏🏻

@github-actions github-actions bot added this to the Gutenberg 14.0 milestone Aug 19, 2022
gziolo pushed a commit that referenced this pull request Aug 23, 2022
* clone the global WP_Query inside the query loop block

* always reset post data after query loop
@gziolo
Copy link
Member

gziolo commented Aug 23, 2022

Cherry-picked with 3c19b1b for WordPress 6.0.2 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Query Loop Affects the Query Loop Block Needs Technical Feedback Needs testing from a developer perspective. [Type] Bug An existing feature does not function as intended
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Query loop infinite loop when inheriting query from template
4 participants