WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 4 months ago

#50163 assigned defect (bug)

Perform a canonical redirect when paginated states of the front page are not found

Reported by: dd32 Owned by: hellofromTonya
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Canonical Keywords: has-patch has-screenshots needs-unit-tests
Focuses: Cc:

Description

As a followup to #45337, [34492], and [47727].

When example.com/ has a static front page assigned and uses <!--nextpage--> accessing non-existing pages uses example.com/page/3/ and Canonical won't kick in.

This is because paged states of the front page use the paged query var, not the page query var.

Should also fix https://meta.trac.wordpress.org/ticket/5184 :)

Attachments (3)

50163.diff (925 bytes) - added by audrasjb 12 months ago.
Patch refresh
Capture d’écran 2020-11-05 à 22.50.34.png (306.3 KB) - added by audrasjb 12 months ago.
before patch: example.com/page/3 exists
0dfda039df09e2d4fba44bd9788a2c7e.gif (352.7 KB) - added by audrasjb 12 months ago.
After patch: example.com/page/3 redirects to homepage

Download all attachments as: .zip

Change History (19)

#2 @SergeyBiryukov
17 months ago

  • Milestone changed from Awaiting Review to 5.5
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

Thanks for the ticket, I'm working on this as part of #28081 :)

#3 @SergeyBiryukov
15 months ago

  • Milestone changed from 5.5 to 5.6

Didn't get a chance to write the tests for this in time for 5.5 RC, moving to 5.6.

#4 @mukesh27
12 months ago

  • Keywords needs-refresh added

Adding the refresh keyword for the PR merge conflict.

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


12 months ago

@audrasjb
12 months ago

Patch refresh

@audrasjb
12 months ago

before patch: example.com/page/3 exists

@audrasjb
12 months ago

After patch: example.com/page/3 redirects to homepage

#6 @audrasjb
12 months ago

  • Keywords has-screenshots added; needs-refresh removed

50163.diff refreshes the previous patch. It appears to fix the issue.

#7 @mukesh27
12 months ago

Patch 50163.diff applied fine in trunk version

#8 @SergeyBiryukov
11 months ago

  • Milestone changed from 5.6 to 5.7

Thanks for the refresh! This still needs tests though, moving to the next release for now.

#9 @audrasjb
11 months ago

Just to be sure: do we need unit tests @SergeyBiryukov ?
Because in my testing the suggested patch fixes the issue.

#10 @SergeyBiryukov
11 months ago

  • Keywords needs-unit-tests added

Yes, sorry I was not clear enough :) Any functional changes to the Canonical component should really be accompanied by unit tests, to avoid unexpected regressions like comment:40:ticket:12456.

#11 @audrasjb
11 months ago

Ok thanks for clarifying. I'll make sure to get those unit tests for the next release as I already saw at least one example of SEO impact of this issue.

#12 @lukecarbis
8 months ago

  • Milestone changed from 5.7 to 5.8

Since we're in 5.7 beta 3, let's move this to the next release.

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


5 months ago

#14 @chaion07
5 months ago

We reviewed this ticket during a [recent bug-scrub]https://wordpress.slack.com/archives/C02RQBWTW/p1622579264170500. Pinging @audrasjb for the [unit tests]https://core.trac.wordpress.org/ticket/50163#comment:9. Are these likely to be done before Beta 1 or this should be punted to the Future Releases? Thanks!
Props to @jeffpaul https://wordpress.slack.com/archives/C02RQBWTW/p1622579402171400

This ticket was mentioned in Slack in #core-test by engahmeds3ed. View the logs.


5 months ago

#16 @hellofromTonya
4 months ago

  • Milestone changed from 5.8 to 5.9
  • Owner changed from SergeyBiryukov to hellofromTonya
  • Status changed from accepted to assigned

Today is 5.8 Beta 1. This one needs tests. Punting to 5.9. But also adding to my tests TODO list. And to ensure I don't forget, @SergeyBiryukov changing ownership to me for the tests.

Note: See TracTickets for help on using tickets.