Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#53868 new enhancement

Add option to `paginate_links` to apply format on all pages

Reported by: ntsekouras's profile ntsekouras Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-unit-tests has-testing-info reporter-feedback
Focuses: Cc:

Description

paginate_links doesn't use the provided format when the page is 1. This is great for the main query as it removes the extra query params making the URL shorter, but in the case of custom queries is problematic.

If no additional data is appended via. add args / add fragment, the string remains empty at the time of output, which browsers then default it to the current page URL.

The code for this behavior is this: $link = str_replace( '%_%', 1 == $n ? '' : $args['format'], $args['base'] );

My proposed solution is to add a new option in paginate_links that would enforce applying the format for all pages, including the first one.

Attachments (1)

53868-reproduce.gif (1.6 MB) - added by hellofromTonya 3 years ago.
Testing to reproduce reported issue

Download all attachments as: .zip

Change History (7)

This ticket was mentioned in PR #1540 on WordPress/wordpress-develop by ntsekouras.


3 years ago
#1

  • Keywords has-patch has-unit-tests added

Trac ticket: https://core.trac.wordpress.org/ticket/53868

paginate_links doesn't use the provided format when the page is 1. This is great for the main query as it removes the extra query params making the URL shorter, but in the case of custom queries is problematic.

If no additional data is appended via. add args / add fragment, the string remains empty at the time of output, which browsers then default it to the current page URL.

The code for this behavior is this: $link = str_replace( '%_%', 1 == $n ? '' : $args['format'], $args['base'] );

My proposed solution is to add a new format_all_pages option in paginate_links that would enforce applying the format for all pages, including the first one. I could use some input for a better name for this option 😄 .

## Notes
In order to test this properly you have to enable pretty permalinks. If not the url will have the page_id param and therefore the final url will not be empty.

## Testing instructions

To reproduce:

  1. Create a couple of posts - no content is needed
  2. Create a page and insert Query Loop block, set the Items per Page to 1 and make sure the inherit is false. After that insert a Query Pagination block inside Query Loop.
  3. Save and in front-end navigate to the second page of the query.
  4. Observe that the link for the first page is pointing to the current url.

To test the solution:

  1. You can test by editing this line in Query Pagination Numbers:https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/blocks/query-pagination-numbers.php#L46 and add the new option: format_all_pages = true. (Noting that this will need a follow up in GB to update the block functionality.
  2. Observe that after navigating to the second page, the first page's link is correct and is not the current url.

This ticket was mentioned in Slack in #core by ntsekouras. View the logs.


3 years ago

#3 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 5.9

@hellofromTonya
3 years ago

Testing to reproduce reported issue

#4 @hellofromTonya
3 years ago

  • Keywords has-testing-info added

Test Report to Reproduce

Env:

  • WordPress: trunk 5.9-alpha
  • OS: macOS Big Sur
  • Browser: Firefox
  • Plugins: none
  • Theme: Twenty Twenty-Two (TT2)

Test 1: Query Block

Steps to setup the query block page:

  1. Create a couple of posts - no content is needed
  2. Create a page and insert Query Loop block, set the Items per Page to 1 and make sure the inherit is false. After that insert a Query Pagination block inside Query Loop.
  3. Save and in front-end navigate to the second page of the query.
  4. Observe that the link for the first page is pointing to the current url.

Results:

  • Clicked on Next Page => http://localhost:8889/test-3/?cst&query-147-page=2
  • Clicked on Previous Pate => http://localhost:8889/test-3/?cst&query-147-page=1
  • Clicked on Next Page
  • Clicked on 1 => http://localhost:8889/test-3/?cst
  • Clicked on page in menu to reset the URL
  • Clicked on 2 => http://localhost:8889/test-3/?query-147-page=2&cst

Test 2: Classic Posts Page

Steps:

  1. Go to Settings > Reading
  2. Set Blog pages show at most to 1
  3. Go to home page
  4. Click through the pagination and observe the 1 URL

Results:

  • Clicked on Next Page => http://localhost:8889/page/2/
  • Clicked on Previous Page => http://localhost:8889/
  • Clicked on 2 => http://localhost:8889/page/2/
  • Clicked on 1 => http://localhost:8889/
  • Clicked on 3 => http://localhost:8889/page/3/
  • Clicked on 1 => http://localhost:8889/

#5 @hellofromTonya
3 years ago

  • Keywords reporter-feedback added
  • Milestone changed from 5.9 to Awaiting Review
  • Type changed from defect (bug) to enhancement

This seems like an enhancement rather an a bug. It's behavior is by design from [28785] #24606 where it was changed to not set page 1 to ?page=1 or /page/1. To avoid forcing a canonical redirect.

@ntsekouras how does this new argument help in the Query Pagination? Is there an open issue in Gutenberg requesting this change? If yes, please link here for further context.

Are there other use cases where this new argument helps? (I'm curious.)

5.9 is in feature freeze. Moving this to back to Awaiting Review pending feedback and further discussion.

#6 @ntsekouras
3 years ago

@ntsekouras how does this new argument help in the Query Pagination? Is there an open issue in Gutenberg requesting this change? If yes, please link here for further context.

A [workaround PR in Gutenberg repo]https://github.com/WordPress/gutenberg/pull/33629 has been merged and there are more details of the problem in the description as well.

Are there other use cases where this new argument helps? (I'm curious.)

Currently this helps to have multiple custom queries pagination work properly. Before someone who needed to implement this, had to do it manually and it seems it wasn't a common use case but it existed. This is needed for core, as Query Loop block is part of core since 5.8, and multiple custom queries are easily implemented.

Note: See TracTickets for help on using tickets.