WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#29636 closed defect (bug) (fixed)

Query args in paginate_links()

Reported by: obenland Owned by: wonderboymusic
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.0
Component: General Keywords: has-patch needs-refresh
Focuses: template Cc:
PR Number:

Description (last modified by obenland)

This is a follow-up to #24606.

After r28785 we parse query args from the get_pagenum_link(), but we don't pass it on to the $defaults array. Not sure if this was an oversight when it was ported over from Twenty Fourteen, or intentional.

Attachments (4)

29636.diff (2.1 KB) - added by obenland 5 years ago.
29636.2.diff (813 bytes) - added by jorbin 5 years ago.
29636.3.diff (1015 bytes) - added by jorbin 5 years ago.
29636.4.diff (1.6 KB) - added by obenland 5 years ago.

Download all attachments as: .zip

Change History (21)

This ticket was mentioned in IRC in #wordpress-dev by obenland. View the logs.


5 years ago

#2 @obenland
5 years ago

  • Description modified (diff)

@obenland
5 years ago

#3 @obenland
5 years ago

  • Keywords has-patch added

Added a patch with additional UT. I'm not sure the query args need to be url-encoded. They are in Twenty Fourteen, but it looked like they are anyway, when I wrote the unit test.

Feedback on the unit test would also be greatly appreciated, not sure if it makes sense to test in that way.

#4 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 4.1

#5 @wonderboymusic
5 years ago

  • Keywords needs-refresh added

These unit tests need to have URLs that are generated, not hardcoded

#6 @wonderboymusic
5 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 29780:

In paginate_links(), ensure that query string args are propagated to the resulting paginated links.

Adds unit tests that use DOMDocument since assertTag is being deprecated - see #29545, [29746].

Props obenland, wonderboymusic.
Fixes #29636.

#7 @jorbin
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#8 @jorbin
5 years ago

This unit test is continuously failing. See https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/36779253

I'm investigating. It looks like we are setting permalink structure somewhere.

#9 @boonebgorges
5 years ago

Ha. I've been staring at this failure for the last week, and assumed it was my rig, so I ignored it.

#10 @wonderboymusic
5 years ago

yeah, I think it was your flanger pedal

@jorbin
5 years ago

#11 @jorbin
5 years ago

Above patch fixes the failure.

It stashes the permalink structure, reverts it, and then sets it back to how it was when we began.

#12 @wonderboymusic
5 years ago

[28964] and [28966] were fun - you should do that on setUp/tearDown for the whole class

@jorbin
5 years ago

#13 @jorbin
5 years ago

Makes sense. Above patch does it.

I looked into doing this for all classes, but it seems to break some other tests. I may try to revisit that in the future so our tests are better individual units.

#14 @wonderboymusic
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 29913:

Fix Tests_Paginate_Links tests by properly setting up / tearing down permalink structure.

Props jorbin.
Fixes #29636.

#15 @boonebgorges
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This test is still failing. (Every other time you run it. Whee!) [29913] sets the permastruct to the value of get_option( 'permalink_structure' ), which is correct when you first run it, but wrong if you run it after the non-multisite tests are run. jorbin's .2.diff did it correctly, by setting it to .

@obenland
5 years ago

#16 @obenland
5 years ago

r28785 introduced a bug, where encoded query arguments were decoded by parse_str() inside of wp_parse_str() but never re-encoded later on.
29636.4.diff encodes them again and adds UT for it.

#17 @wonderboymusic
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 30133:

[28785] introduced a bug, where encoded query arguments were decoded by parse_str() inside of wp_parse_str() but never re-encoded later on.

This encodes them, adds unit test.

Props obenland.
Fixes #29636.

Note: See TracTickets for help on using tickets.