-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Pagniation links with/without trailing slashes. #8591
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
ca321d4
a7f8f3d
72f32ca
c8fe267
c0c384f
ac9b052
8eb206b
03a8893
736a4b3
76fc55e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4672,9 +4672,26 @@ function paginate_links( $args = '' ) { | |||||
| // Append the format placeholder to the base URL. | ||||||
| $pagenum_link = trailingslashit( $url_parts[0] ) . '%_%'; | ||||||
|
|
||||||
| /* | ||||||
| * Ensures sites not using trailing slashes get links in the form | ||||||
| * `/page/2` rather than `/page/2/`. On these sites, linking to the | ||||||
| * URL with a trailing slash will results in a 301 redirect from the | ||||||
| * incorrect URL to the correctly formattted one. This presents an | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| * unnecessary performance hit. | ||||||
| */ | ||||||
| if ( $wp_rewrite->using_permalinks() && ! $wp_rewrite->use_trailing_slashes ) { | ||||||
| $pagenum_link = untrailingslashit( $url_parts[0] ); | ||||||
| } else { | ||||||
| $pagenum_link = trailingslashit( $url_parts[0] ); | ||||||
| } | ||||||
| $pagenum_link .= '%_%'; | ||||||
|
|
||||||
| // URL base depends on permalink settings. | ||||||
| $format = $wp_rewrite->using_index_permalinks() && ! strpos( $pagenum_link, 'index.php' ) ? 'index.php/' : ''; | ||||||
| $format .= $wp_rewrite->using_permalinks() ? user_trailingslashit( $wp_rewrite->pagination_base . '/%#%', 'paged' ) : '?paged=%#%'; | ||||||
| if ( $wp_rewrite->using_permalinks() && ! $wp_rewrite->use_trailing_slashes ) { | ||||||
| $format = '/' . ltrim( $format, '/' ); | ||||||
| } | ||||||
|
|
||||||
| $defaults = array( | ||||||
| 'base' => $pagenum_link, // http://example.com/all_posts.php%_% : %_% is replaced by format (below). | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,39 @@ class Tests_General_PaginateLinks extends WP_UnitTestCase { | |||||||||
|
|
||||||||||
| private $i18n_count = 0; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Post IDs created for shared fixtures. | ||||||||||
| * | ||||||||||
| * @var int[] | ||||||||||
| */ | ||||||||||
| protected static $post_ids = array(); | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Category ID created for shared fixtures. | ||||||||||
| * | ||||||||||
| * @var int | ||||||||||
| */ | ||||||||||
| protected static $category_id = 0; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Set up shared fixtures. | ||||||||||
| * | ||||||||||
| * @param WP_UnitTest_Factory $factory Factory instance. | ||||||||||
| */ | ||||||||||
| public static function wpSetUpBeforeClass( $factory ) { | ||||||||||
| self::$category_id = $factory->term->create( | ||||||||||
| array( | ||||||||||
| 'taxonomy' => 'category', | ||||||||||
| 'name' => 'Categorized', | ||||||||||
| ) | ||||||||||
| ); | ||||||||||
|
|
||||||||||
| self::$post_ids = $factory->post->create_many( 10 ); | ||||||||||
| foreach ( self::$post_ids as $post_id ) { | ||||||||||
| wp_set_post_categories( $post_id, array( self::$category_id ) ); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public function set_up() { | ||||||||||
| parent::set_up(); | ||||||||||
|
|
||||||||||
|
|
@@ -383,4 +416,127 @@ public function test_custom_base_query_arg_should_be_stripped_from_current_url_b | |||||||||
| $page_2_url = home_url() . '?foo=2'; | ||||||||||
| $this->assertContains( "<a class=\"page-numbers\" href=\"$page_2_url\">2</a>", $links ); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Ensures pagination links include trailing slashes when the permalink structure includes them. | ||||||||||
| * | ||||||||||
| * @ticket 61393 | ||||||||||
| */ | ||||||||||
| public function test_permalinks_with_trailing_slash_produce_links_with_trailing_slashes() { | ||||||||||
| update_option( 'posts_per_page', 2 ); | ||||||||||
| $this->set_permalink_structure( '/%postname%/' ); | ||||||||||
|
|
||||||||||
| $this->go_to( '/category/categorized/page/2/' ); | ||||||||||
|
|
||||||||||
| // `current` needs to be passed as it's not picked up from the query vars set by `go_to()` above. | ||||||||||
| $links = paginate_links( array( 'current' => 2 ) ); | ||||||||||
|
|
||||||||||
| $processor = new WP_HTML_Tag_Processor( $links ); | ||||||||||
| $found_links = 0; | ||||||||||
| while ( $processor->next_tag( 'A' ) ) { | ||||||||||
| ++$found_links; | ||||||||||
| $href = $processor->get_attribute( 'href' ); | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This just suppresses a PHPStan complaint about a
Suggested change
|
||||||||||
| $this->assertStringEndsWith( '/', $href, "Pagination links should end with a trailing slash, found: $href" ); | ||||||||||
| } | ||||||||||
| $this->assertGreaterThan( 0, $found_links, 'There should be pagination links found.' ); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Ensures pagination links do not include trailing slashes when the permalink structure doesn't include them. | ||||||||||
| * | ||||||||||
| * @ticket 61393 | ||||||||||
| */ | ||||||||||
| public function test_permalinks_without_trailing_slash_produce_links_without_trailing_slashes() { | ||||||||||
| update_option( 'posts_per_page', 2 ); | ||||||||||
| $this->set_permalink_structure( '/%postname%' ); | ||||||||||
|
|
||||||||||
| $this->go_to( '/category/categorized/page/2' ); | ||||||||||
|
|
||||||||||
| // `current` needs to be passed as it's not picked up from the query vars set by `go_to()` above. | ||||||||||
| $links = paginate_links( array( 'current' => 2 ) ); | ||||||||||
|
|
||||||||||
| $processor = new WP_HTML_Tag_Processor( $links ); | ||||||||||
| $found_links = 0; | ||||||||||
| while ( $processor->next_tag( 'A' ) ) { | ||||||||||
| ++$found_links; | ||||||||||
| $href = $processor->get_attribute( 'href' ); | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| $this->assertStringEndsNotWith( '/', $href, "Pagination links should end with a trailing slash, found: $href" ); | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| } | ||||||||||
| $this->assertGreaterThan( 0, $found_links, 'There should be pagination links found.' ); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Ensures the pagination links do not modify query strings (permalinks with trailing slash). | ||||||||||
| * | ||||||||||
| * @ticket 61393 | ||||||||||
| * @ticket 63123 | ||||||||||
| * | ||||||||||
| * @dataProvider data_query_strings | ||||||||||
| * | ||||||||||
| * @param string $query_string Query string. | ||||||||||
| * @param string $unexpected Unexpected query string. | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused param.
Suggested change
|
||||||||||
| */ | ||||||||||
| public function test_permalinks_with_trailing_slash_do_not_modify_query_strings( string $query_string ) { | ||||||||||
| update_option( 'posts_per_page', 2 ); | ||||||||||
| $this->set_permalink_structure( '/%postname%/' ); | ||||||||||
|
|
||||||||||
| $this->go_to( "/page/2/?{$query_string}" ); | ||||||||||
|
|
||||||||||
| // `current` needs to be passed as it's not picked up from the query vars set by `go_to()` above. | ||||||||||
| $links = paginate_links( array( 'current' => 2 ) ); | ||||||||||
|
|
||||||||||
| $processor = new WP_HTML_Tag_Processor( $links ); | ||||||||||
| $found_links = 0; | ||||||||||
| while ( $processor->next_tag( 'A' ) ) { | ||||||||||
| ++$found_links; | ||||||||||
| $href = $processor->get_attribute( 'href' ); | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| $this->assertStringEndsWith( "/?{$query_string}", $href, "Pagination links should not modify the query string, found: $href" ); | ||||||||||
| } | ||||||||||
| $this->assertGreaterThan( 0, $found_links, 'There should be pagination links found.' ); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Ensures the pagination links do not modify query strings (permalinks without trailing slash). | ||||||||||
| * | ||||||||||
| * @ticket 61393 | ||||||||||
| * @ticket 63123 | ||||||||||
| * | ||||||||||
| * @dataProvider data_query_strings | ||||||||||
| * | ||||||||||
| * @param string $query_string Query string. | ||||||||||
| * @param string $unexpected Unexpected query string. | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused.
Suggested change
|
||||||||||
| */ | ||||||||||
| public function test_permalinks_without_trailing_slash_do_not_modify_query_strings( string $query_string ) { | ||||||||||
| update_option( 'posts_per_page', 2 ); | ||||||||||
| $this->set_permalink_structure( '/%postname%' ); | ||||||||||
|
|
||||||||||
| $this->go_to( "/page/2?{$query_string}" ); | ||||||||||
|
|
||||||||||
| // `current` needs to be passed as it's not picked up from the query vars set by `go_to()` above. | ||||||||||
| $links = paginate_links( array( 'current' => 2 ) ); | ||||||||||
|
|
||||||||||
| $processor = new WP_HTML_Tag_Processor( $links ); | ||||||||||
| $found_links = 0; | ||||||||||
| while ( $processor->next_tag( 'A' ) ) { | ||||||||||
| ++$found_links; | ||||||||||
| $href = $processor->get_attribute( 'href' ); | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| $this->assertStringEndsWith( "?{$query_string}", $href, "Pagination links should not modify the query string, found: $href" ); | ||||||||||
| $this->assertStringEndsNotWith( "/?{$query_string}", $href, "Pagination links should not be slashed before the query string, found: $href" ); | ||||||||||
| } | ||||||||||
| $this->assertGreaterThan( 0, $found_links, 'There should be pagination links found.' ); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Data provider for | ||||||||||
| * - test_permalinks_without_trailing_slash_do_not_modify_query_strings | ||||||||||
| * - test_permalinks_with_trailing_slash_do_not_modify_query_strings | ||||||||||
| * | ||||||||||
| * @return array<string[]> Data provider. | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| */ | ||||||||||
| public function data_query_strings(): array { | ||||||||||
| return array( | ||||||||||
| array( 'foo=bar' ), | ||||||||||
| array( 'foo=bar&pen=pencil' ), | ||||||||||
|
Comment on lines
+538
to
+539
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| ); | ||||||||||
| } | ||||||||||
| } | ||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.