Make WordPress Core

Opened 17 years ago

Last modified 40 hours ago

#2877 assigned defect (bug)

A Slash too much @ get_pagenum_link()

Reported by: davemad-davenet's profile dave@… Owned by: petitphp's profile petitphp
Milestone: 6.3 Priority: normal
Severity: normal Version: 2.0
Component: Posts, Post Types Keywords: has-patch needs-testing has-unit-tests has-testing-info
Focuses: template Cc:

Description

  • File: template-functions-links.php
  • Function: get_pagenum_link()

The following code is at the end of the function:

	// showing /page/1/ or ?paged=1 is redundant
	if ( 1 === $pagenum ) {
		$qstr = str_replace('page/1/', '', $qstr); // for mod_rewrite style
		$qstr = remove_query_arg('paged', $qstr); // for query style
	}

If you work with the mod_rewrite style, you will get this URL for the first page:

http://www.example.com/wordpress/index.php/
This wont work (/ at the end).

Correx:

	// showing /page/1/ or ?paged=1 is redundant
	if ( 1 === $pagenum ) {
		$qstr = str_replace('/page/1/', '', $qstr); // for mod_rewrite style
		$qstr = remove_query_arg('paged', $qstr); // for query style
	}

Notes:

  • sry for my bad english...
  • i've downloaded a german translation of wordpress 2.0.3, so maybe its just in this version... ?!

Attachments (3)

2877.diff (1.4 KB) - added by scribu 12 years ago.
2877.2.diff (1.4 KB) - added by jesin 8 years ago.
Refresh 2877.diff
2877-test.diff (891 bytes) - added by jesin 8 years ago.

Download all attachments as: .zip

Change History (21)

#1 @darkfate
17 years ago

No, it's that way in the english version. I don't think that would fix it and there would be a problem with that. Putting the slash in front like you did would make the server redirect to root. So if you have wordpress in a directory it would screw up like so:
You would want this:
http://site.com/wordpress/testing/1/
But you would get this:
http://site.com/testing/1/

#2 @Nazgul
16 years ago

  • Resolution set to wontfix
  • Status changed from new to closed

No traction in almost a year, so closing as wontfix.

Feel free to re-open it if you have additional information/suggestions/patches/...

#3 @scribu
12 years ago

  • Keywords needs-patch added
  • Milestone set to Future Release
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Only enhancements or feature requests can be closed when there's no traction.

@scribu
12 years ago

#4 @scribu
12 years ago

  • Keywords has-patch added; needs-patch removed

#5 @scribu
12 years ago

Note that the problem is with first-page links, not with /page/2, /page/3/ etc.

Original thread: http://wordpress.org/support/topic/plugin-wp-pagenavi-trailing-slash-at-first-page-url-regardless-of-defined-permalink-structure

Last edited 12 years ago by scribu (previous) (diff)

#6 @nacin
9 years ago

  • Component changed from General to Template

Interesting patch.

#7 @nacin
9 years ago

  • Keywords needs-unit-tests added

#8 @nacin
9 years ago

  • Component changed from Template to Posts, Post Types
  • Focuses template added

#9 @obenland
9 years ago

  • Keywords needs-refresh added

#10 @chriscct7
8 years ago

  • Version changed from 2.0.3 to 2.0

@jesin
8 years ago

Refresh 2877.diff

@jesin
8 years ago

#11 @mattheweppelsheimer
4 years ago

  • Keywords needs-testing added; needs-unit-tests needs-refresh removed

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


4 months ago

#13 @audrasjb
4 months ago

  • Keywords needs-refresh added
  • Owner set to petitphp
  • Status changed from reopened to assigned

We discussed this issue during this week Old Tickets Triage Session.

The patch needs to be refreshed. Assigning @petitphp who's willing to add a new patch/unit tests.

Thanks to @SergeyBiryukov, @maigret, @petitphp for discussing this issue :)

This ticket was mentioned in PR #3931 on WordPress/wordpress-develop by @petitphp.


4 months ago
#14

  • Keywords has-unit-tests added; needs-refresh removed

#15 @petitphp
4 months ago

PR with refreshed patch added.

I've made a change from the original patch to ensure query_args don't force a trailing slash, and I updated the tests accordingly.

I've tested multiple cases :

In every case, get_pagenum_link correctly matched the permalink structure and only added a trailing slash if present.

This should be ready for testing on more exotic WordPress install.

#16 @audrasjb
4 months ago

  • Milestone changed from Future Release to 6.3

Thanks for the PR @petitphp!
Moving for 6.3 consideration.

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


40 hours ago

#18 @mukesh27
40 hours ago

  • Keywords has-testing-info added

This ticket was discussed in the recent bug scrub.

The PR looks good and requires manual testing. Could someone from the testing team, such as @ironprogrammer or @webtechpooja, please review it?

Additional props to @oglekler

Note: See TracTickets for help on using tickets.