Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#29636 closed defect (bug) (fixed)

Query args in paginate_links()

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

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 12 years ago.
29636.2.diff​ (813 bytes ) - added by jorbin 12 years ago.
29636.3.diff​ (1015 bytes ) - added by jorbin 12 years ago.
29636.4.diff​ (1.6 KB ) - added by obenland 12 years ago.

Download all attachments as: .zip

Change History (21)

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


12 years ago

#2 @obenland
12 years ago

  • Description modified (diff)

@obenland
12 years ago

#3 @obenland
12 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
12 years ago

  • Milestone Awaiting Review → 4.1

#5 @wonderboymusic
12 years ago

  • Keywords needs-refresh added

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

#6 @wonderboymusic
12 years ago

  • Owner set to wonderboymusic
  • Resolution → fixed
  • Status new → 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
12 years ago

  • Resolution fixed
  • Status closed → reopened

#8 @jorbin
12 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
12 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
12 years ago

yeah, I think it was your flanger pedal

@jorbin
12 years ago

#11 @jorbin
12 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
12 years ago

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

@jorbin
12 years ago

#13 @jorbin
12 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
12 years ago

  • Resolution → fixed
  • Status reopened → closed

In 29913:

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

Props jorbin.
Fixes #29636.

#15 @boonebgorges
12 years ago

  • Resolution fixed
  • Status closed → 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
12 years ago

#16 @obenland
12 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
12 years ago

  • Resolution → fixed
  • Status reopened → 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.