WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#21028 closed defect (bug) (wontfix)

Singular pagination canonical redirection causes some breakage

Reported by: duck_ Owned by: duck_
Milestone: Priority: normal
Severity: normal Version: 3.4
Component: Canonical Keywords: 2nd-opinion
Focuses: Cc:

Description

3.4 fixed canonical redirection for singular pagination, see #20385 and [20397].

However, some users were using the "page" query parameter for something other than WordPress' default pagination. The canonical redirect hijacks this even if the post/page is not multipaged.

An example is buddypress.org: http://buddypress.org/extend/plugins/?page=2

This can be fixed by those relying on the broken redirect. Instead of looking for $_GET['page'] retrieving the value from $wp_query can be done: get_query_var('page'). Using that it should work and give pretty URLs.

Originally reported by taropaa in #21020.

Change History (9)

#1 @duck_
8 years ago

I don't think I made it clear enough that redirecting ?page=X for singular requests is the intended behaviour and it was broken previously.

#2 @nacin
8 years ago

  • Milestone changed from Awaiting Review to 3.4.1

#3 follow-up: @nacin
8 years ago

  • Owner set to duck_
  • Status changed from new to assigned

Per IRC:

This doesn't seem like it broke a whole lot of themes/plugins. We should collect some more information. If it broke a lot, at worst, we revert. At best, we can wontfix this.

Also, it is entirely possible that it worked at one point, which would mean this is an instant wontfix regardless. duck_ found [15707] and is investigating.

#4 @nacin
8 years ago

Also:

This only refused to redirect when there was nothing else to redirect (i.e. the current URL was otherwise canonical). If it had to redirect ?p=X&page=X, then it would handle both of them. That seems to be another reason to wontfix.

#5 @markoheijnen
8 years ago

I had one site broke because of this ( 2 years old already ). I do have to admit the code was worse since the query part was in the theme.

It seems that the page var get removed when the page does not exists for the main query. Something I fully can understand and does help to do stuff on the right way. If this is true it should be wontfix I guess

#6 in reply to: ↑ 3 @duck_
8 years ago

Replying to nacin:

Also, it is entirely possible that it worked at one point, which would mean this is an instant wontfix regardless. duck_ found [15707] and is investigating.

The branch the pagination stuff was moved out of had !empty($_GET['p']) so 3.0.1 would also only redirect if another redirect was also taking place.

#7 @neoxx
8 years ago

for what it's worth, the page-redirect breaks some legacy plugins like falbum http://www.randombyte.net/blog/projects/falbum/

#8 @markjaquith
8 years ago

  • Milestone 3.4.1 deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

I'm calling this WONTFIX. If you're hijacking a WP core query var, you should be using the WP query var API to read that value. As noted, we already would do the page redirect if another redirect was queued up, so this is not a new situation, it's just more consistently revealed.

If new evidence comes in showing more widespread breakage than has been reported, re-open. But otherwise, I think this is acceptable.

#9 @nacin
8 years ago

Agreed with wontfix.

Note: See TracTickets for help on using tickets.