Fix pagination reset issue for inherited queries by using get_query_var#51
Fix pagination reset issue for inherited queries by using get_query_var#51agarciar wants to merge 1 commit into
Conversation
Replaced $_GET-based page detection with get_query_var('paged') when the query is inherited. This ensures the correct current page is preserved when navigating back using the browser’s back button, preventing duplicated posts and broken pagination flow.
WalkthroughThe logic for determining the current page number in the ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Plugin.php (1)
254-256: Tighten sanitisation of$_GETinputCasting to
(int)still permits negative values. Usingabsint()(orfilter_input()withFILTER_VALIDATE_INT) guarantees a non-negative integer and avoids pointless DB queries:- : ( empty( $_GET[ $page_key ] ) ? 1 : (int) $_GET[ $page_key ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended + : ( empty( $_GET[ $page_key ] ) ? 1 : absint( $_GET[ $page_key ] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Plugin.php(1 hunks)
🔇 Additional comments (1)
src/Plugin.php (1)
257-259: Solid choice of$wp_query& dynamic parameter keyUsing the global
$wp_querywhen inheriting and falling back to a freshWP_Queryfor custom contexts keeps the behaviour consistent with core. Nice work.
| $page = $inherit | ||
| ? ( get_query_var( 'paged' ) ? get_query_var( 'paged' ) : 1 ) | ||
| : ( empty( $_GET[ $page_key ] ) ? 1 : (int) $_GET[ $page_key ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Avoid duplicate get_query_var() calls & cover front-page edge-case
Two identical get_query_var( 'paged' ) calls are executed and the logic silently drops to 1 when WordPress sets paged = 0 (static front-page). Both issues are easy to eliminate:
- $page = $inherit
- ? ( get_query_var( 'paged' ) ? get_query_var( 'paged' ) : 1 )
+ $paged = absint( get_query_var( 'paged' ) );
+ if ( 0 === $paged ) { // front-page uses `page`
+ $paged = absint( get_query_var( 'page' ) );
+ }
+ $page = $inherit
+ ? max( 1, $paged )
: ( empty( $_GET[ $page_key ] ) ? 1 : (int) $_GET[ $page_key ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended• One function call instead of two.
• Handles the paged = 0 scenario.
• Uses absint()/max() for defensive safety.
Refine pagination logic: eliminate duplicate get_query_var() calls and handle static front-page
Introduce a single, sanitized $paged variable, fall back to page when on a static front-page, and ensure a minimum of 1:
• Declare and sanitize $paged once.
• If $paged is zero (static front-page), pull from get_query_var('page').
• Use max(1, $paged) to enforce a floor of 1.
• Remove the duplicate get_query_var('paged') invocation.
- $page = $inherit
- ? ( get_query_var( 'paged' ) ? get_query_var( 'paged' ) : 1 )
+ $paged = absint( get_query_var( 'paged' ) );
+ if ( 0 === $paged ) { // static front-page uses `page`
+ $paged = absint( get_query_var( 'page' ) );
+ }
+ $page = $inherit
+ ? max( 1, $paged )
: ( empty( $_GET[ $page_key ] ) ? 1 : (int) $_GET[ $page_key ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $page = $inherit | |
| ? ( get_query_var( 'paged' ) ? get_query_var( 'paged' ) : 1 ) | |
| : ( empty( $_GET[ $page_key ] ) ? 1 : (int) $_GET[ $page_key ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended | |
| $paged = absint( get_query_var( 'paged' ) ); | |
| if ( 0 === $paged ) { // static front-page uses `page` | |
| $paged = absint( get_query_var( 'page' ) ); | |
| } | |
| $page = $inherit | |
| ? max( 1, $paged ) | |
| : ( empty( $_GET[ $page_key ] ) ? 1 : (int) $_GET[ $page_key ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended |
🤖 Prompt for AI Agents
In src/Plugin.php around lines 254 to 256, the pagination logic calls
get_query_var('paged') twice and does not handle the case when 'paged' is zero
on a static front-page. Refactor by assigning get_query_var('paged') once to a
variable, then if that value is zero, assign get_query_var('page') to it. Use
max(1, absint($paged)) to ensure the page number is at least 1. Replace the
original conditional with this sanitized variable and remove the duplicate
get_query_var('paged') calls.
|
@agarciar Thank you for the PR. The latest version 1.0.17 fixes the issue. #57 |
This PR addresses Issue #50, where pagination state is lost when using “Load More” or “Infinite Scroll” with Update URL enabled on archive pages that rely on inherited queries.
The fix uses WordPress’s native get_query_var('paged') to determine the current page when dealing with inherited queries, falling back to $_GET for non-inherited ones.
Summary by CodeRabbit