paginate_links() - incorrect links on return to the first page

[bobbingwide]

Sorry about this but this is a direct follow on to #30831.

With the exact same piece of code that I had originally,
when you return to the first page of the paginated content,
none of the links contain the correct paging values...
so they don't work.

function bw_navi_paginate_links( $id, $page, $pages ) {
  $base = add_query_arg( "bwscid$id", "%_%" );
  $format = "%#%";
  $args = array( "base" => $base
               , "format" => $format
               , "total" => $pages
               , "current" => $page
               , "before_page_number" => "["
               , "after_page_number" => "]"
               );

  $links = paginate_links( $args ); 
  e( $links );
}

regardless of whether or not I attempt to apply my workaround

              , "add_args" => false

Expected output:

Assume the original display is:

1 to 2 of 5
one
two
(1) [2] [3] Next

and page 2 display is:

3 to 4 of 5
three
four
Previous [1] (2) [3] Next

where (n) indicates it's not a link

with WordPress 4.0, and a value of 2, for the $id the pagination links were

• page 1: ​http://example.com/permalink/?bwscid2=
• page 2: ​http://example.com/permalink/?bwscid2=2
• page 3: ​http://example.com/permalink/?bwscid2=3

You could visit page 2, then return to page 1, then visit page 2 again

Actual output:

• On first visit the links are OK
• When you visit page 2 the links are OK
• When you return to page 1 all the links are broken

​http://example.com/permalink/?bwscid2

In order to be able to page you need to remove the query parm from the URL.

Demo:

For an example see ​http://wp-a2z.com/oik_api/paginate_links/
and try to page forwards then backwards through the source code.

Workaround:

Unknown. Please advise.

[bobbingwide]

OK, I have a workaround.

I need to change the value of REQUEST_URI in $_SERVER to remove my special query arg, otherwise
the new code in 4.1.1 will find it as the pagenum_link and add it back into the add_args array...
even though I've specifically passed add_args=>false.

function bw_navi_paginate_links( $id, $page, $pages ) {
  $string = remove_query_arg( "bwscid$id" );
  $_SERVER['REQUEST_URI'] = $string; 
  $base = add_query_arg( "bwscid$id", "%_%" );
  $format = "%#%";
  $args = array( "base" => $base
               , "format" => $format
               , "total" => $pages
               , "current" => $page
               , "before_page_number" => "["
               , "after_page_number" => "]"
               , "add_args" => false
               );
  $links = paginate_links( $args ); 
  e( $links );
}

[boonebgorges]

Thanks for the report, bobbingwide. The technique used to avoid overwriting the custom pagination query var in [31203] is problematic in two ways.

First, using parse_url( $url, PHP_URL_QUERY ) doesn't work properly when the $url contains a # - anything after the first # is interpreted as part of the 'fragment' rather than the 'query'. See https://core.trac.wordpress.org/browser/tags/4.1.1/src/wp-includes/general-template.php?marks=2632#L2629. When $args['base'] contains # - which it almost always will - this parsing will be broken. Instead of relying on parse_url(), we can just explode on ?.

Second, remove_query_arg() doesn't know what to do with a string like 'foo', you get when you parse the querystring out of example.com/?foo. That is, remove_query_arg( 'foo', 'foo' ) will do nothing. We need a technique for removing the query args that is agnostic between query vars that have values (?foo=bar) and those that don't (?foo). https://core.trac.wordpress.org/browser/tags/4.1.1/src/wp-includes/general-template.php?marks=2636#L2629

Have a look at the new parsing I'm proposing in 31939.diff​.

[boonebgorges]

In 32359:

In paginate_links(), improve handling of custom pagination query vars.

Custom pagination query vars, as provided in the 'base' parameter, must be
detected in the current page URL and removed before generating fresh pagination
links. The logic introduced in this changeset ensures that these custom
query vars are properly detected in cases where the 'format' param contains
a #.

This is a follow-up to [31203] #30831.

Fixes #31939.

[bobbingwide]

Sorry I didn't get a chance to provide feedback on the code before it was committed, but I had to adjust my workaround because of the latest security scare etc..

The comment I did want to make was that for performance the test on total pages should be a lot earlier.
Shall I raise a separate TRAC for this?

// Who knows what else people pass in $args
	$total = (int) $args['total'];
	if ( $total < 2 ) {
		return;
	}

And maybe remove the one line comment too?

[boonebgorges]

Replying to bobbingwide:

Sorry I didn't get a chance to provide feedback on the code before it was committed, but I had to adjust my workaround because of the latest security scare etc..

The comment I did want to make was that for performance the test on total pages should be a lot earlier.
Shall I raise a separate TRAC for this?

// Who knows what else people pass in $args
	$total = (int) $args['total'];
	if ( $total < 2 ) {
		return;
	}

And maybe remove the one line comment too?

Yeah, please open another ticket for this suggestion. Thanks!