Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#16644 closed defect (bug) (fixed)

[15825] Changed the format of our category links from ?cat=123 to ?category_name=slug

Reported by: markjaquith's profile markjaquith Owned by:
Milestone: 3.1.1 Priority: high
Severity: normal Version: 3.1
Component: Permalinks Keywords: has-patch
Focuses: Cc:

Description

Before [15825], default category links used the ID (?cat=123). After [15825], they started using ?category_name=slug. There are no canonical redirects.

Attachments (3)

16644.diff (1.3 KB) - added by greuben 13 years ago.
16644.2.diff (505 bytes) - added by dd32 13 years ago.
get_term_link modification for ?cat=
16644.3.diff (1.6 KB) - added by greuben 13 years ago.

Download all attachments as: .zip

Change History (22)

#1 follow-up: @bigonroad
13 years ago

When you say "no canonical redirects", do you mean one of the links doesn't work at all, or just that neither link is 301-ing to the other?

#2 @bigonroad
13 years ago

  • Cc bigonroad added

#3 in reply to: ↑ 1 @kawauso
13 years ago

Replying to bigonroad:

When you say "no canonical redirects", do you mean one of the links doesn't work at all, or just that neither link is 301-ing to the other?

Both links work, no 301-ing takes place.

#4 @dd32
13 years ago

This comes back to the query_Var being set to category_name rather than 'cat' as you'd expect in this case.

The solution would be to change that var, however, I'm pretty sure we'll then also run into the fact that the rewrite rules are also based on that query variable.

#5 @markjaquith
13 years ago

dd32 — what do rewrite rules matter? This is for default permalinks — no rewrite rules are in play.

@greuben
13 years ago

#6 @greuben
13 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

#7 follow-up: @nacin
13 years ago

We need to move back to cat=. category_name= should still work and should probably canonically redirect.

#8 in reply to: ↑ 7 @greuben
13 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

Replying to nacin:

We need to move back to cat=. category_name= should still work and should probably canonically redirect.

Oops I thought ?cat= should redirect to ?category_name=

#9 @dd32
13 years ago

Replying to markjaquith:

dd32 — what do rewrite rules matter? This is for default permalinks — no rewrite rules are in play.

I was attempting to make the point of why the change has happened, That it's using the query_var specified in the register_taxonomy call, of whcih, is also utilised by the rewrite rule generation code, when you also look at the get_term_link code, it either uses the permastruct in use, or the query_var, Ie. The generated rewrite rules, as well as the non-rewrite rules rely on the same query var param

#10 @dd32
13 years ago

get_term_link either needs a specific branch for categories in the non-rewrite context to use cat + id rather than category_name + slug (as all other taxonomy non-rewrite rules are)

@dd32
13 years ago

get_term_link modification for ?cat=

#11 @dd32
13 years ago

  • Keywords has-patch added; needs-patch removed

attachment 16644.2.diff added

  • modifies get_term_link to return ?cat= links.

@greuben
13 years ago

#12 @greuben
13 years ago

attachment 16644.3.diff added

extended dd32's patch for canonical redirects

#13 @ryan
13 years ago

Per bug scrub, 16644.2.diff is the direction we want to go. We don't want to change canonical in a dot release. Let's test .2 and get it in.

#14 @aaroncampbell
13 years ago

I tested 16644.2.diff on a couple trunk installs and it works for me

#15 follow-up: @markjaquith
13 years ago

greuben — can you open a ticket with category_name canonical redirect against future release? Just don't want that in 3.1.1

16644.2.diff is good. Going in.

#16 @markjaquith
13 years ago

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

(In [17494]) Revert to the cat=X permalinks from 3.0 and earlier. props dd32. fixes #16644 for trunk

#17 @markjaquith
13 years ago

(In [17495]) Revert to the cat=X permalinks from 3.0 and earlier. props dd32. fixes #16644 for 3.1

#18 in reply to: ↑ 15 @greuben
13 years ago

Replying to markjaquith:

greuben — can you open a ticket with category_name canonical redirect against future release? Just don't want that in 3.1.1

#16728

#19 @scribu
13 years ago

?cat= doesn't include children after all, so get_term_link() will return URLs with different results, based on wether permalinks are turned on or not.

However:

(12:34:42 AM) rboren: That should return it whatever 3.0 did, yes? I don't care what it does as long it is what 3.0 does. :-)
(12:35:16 AM) rboren: I need a drink to talk about taxonomy.

Alas, I don't have enough whiskey to make that change. :)

Note: See TracTickets for help on using tickets.