Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#10435 closed defect (bug) (duplicate)

Canonical redirect does not work on IIS7

Reported by: sirzooro Owned by: markjaquith
Milestone: Priority: normal
Severity: normal Version: 2.8.1
Component: Canonical Keywords: has-patch tested early
Focuses: Cc:


redirect_canonical() does not take into account that IIS7 may support url rewriting. Attached patch corrects this.

Attachments (2)

canonical.php.diff (639 bytes) - added by sirzooro 6 years ago.
10435.patch (3.6 KB) - added by peaceablewhale 6 years ago.

Download all attachments as: .zip

Change History (12)

@sirzooro6 years ago

@peaceablewhale6 years ago

comment:1 @peaceablewhale6 years ago

  • Keywords tested needs-review added; needs-testing removed

In order for the patch to work, iis7_supports_permalinks() has to be moved from wp-admin/includes/misc.php to wp-includes/vars.php. I have uploaded a patch to do this. Also tested on IIS 7.5 on Windows 7 RC with URL Rewrite Module 2.0 beta.

comment:2 @peaceablewhale6 years ago

By the way, the name of iis7_supports_permalinks() will be changed later. See also #10384.

comment:3 @westi5 years ago

  • Cc ruslany added
  • Keywords early added
  • Milestone changed from 2.9 to 3.0

This patch looks ok but I would like to get some more testing done on it - don't have IIS here.

Also would prefer to change this early in a release cycle just in case it causes issues.

Moving to 3.0 early.

CC'ing ruslany as hopefully he can test it for us and give feedback.

comment:4 @ruslany5 years ago

Verified the 10435.patch and confirmed that it works.

However, in order to fix this completely the patch for #10187 needs to be applied. Otherwise wp_redirect function uses HTTP refresh header instead of a proper 301 redirect. This is not preferrable from the SEO perspective. I have also verified the patch for #10187 - see my comments for that ticket.

comment:5 @hakre5 years ago

Asking to those who care about IIS: What about merging this ticket with #10187? Maybe then this get's better into line.

comment:6 follow-up: @ruslany5 years ago

The suggestion makes sense. Should 10187 have a patch with both fixes and then resolve 10435 as a duplicate?

comment:7 in reply to: ↑ 6 @hakre5 years ago

Replying to ruslany:

The suggestion makes sense. Should 10187 have a patch with both fixes and then resolve 10435 as a duplicate?

Strictly spoken it's not a duplicate, but well, I'm for merging so we get the redirects into order together. Would be easier for IIS tester as well as for the committer.

comment:8 @hakre5 years ago

  • Keywords needs-review removed

comment:9 @peaceablewhale5 years ago

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

Merged with #10187

comment:10 @nacin5 years ago

  • Milestone 3.0 deleted
Note: See TracTickets for help on using tickets.