Make WordPress Core

Opened 4 years ago

Last modified 2 years ago

#50163 assigned defect (bug)

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

Reported by: dd32's profile dd32 Owned by: hellofromtonya's profile hellofromTonya
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Canonical Keywords: has-patch has-screenshots needs-unit-tests early
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 3 years ago.
Patch refresh
Capture d’écran 2020-11-05 à 22.50.34.png (306.3 KB) - added by audrasjb 3 years ago.
before patch: example.com/page/3 exists
0dfda039df09e2d4fba44bd9788a2c7e.gif (352.7 KB) - added by audrasjb 3 years ago.
After patch: example.com/page/3 redirects to homepage

Download all attachments as: .zip

Change History (29)

#2 @SergeyBiryukov
4 years 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
4 years 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
3 years 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.


3 years ago

@audrasjb
3 years ago

Patch refresh

@audrasjb
3 years ago

before patch: example.com/page/3 exists

@audrasjb
3 years ago

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

#6 @audrasjb
3 years ago

  • Keywords has-screenshots added; needs-refresh removed

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

#7 @mukesh27
3 years ago

Patch 50163.diff applied fine in trunk version

#8 @SergeyBiryukov
3 years 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
3 years ago

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

#10 @SergeyBiryukov
3 years 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
3 years 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
3 years 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.


3 years ago

#14 @chaion07
3 years 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.


3 years ago

#16 @hellofromTonya
3 years 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.

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


2 years ago

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


2 years ago

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


2 years ago

#20 @hellofromTonya
2 years ago

  • Keywords early added
  • Milestone changed from 5.9 to 6.0

With 5.9 Beta 1 tomorrow and this patch touching wpdb, moving to 6.0. I'll work on and finish the unit tests early in the 6.0 cycle to get it committed.

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


2 years ago

#22 @Mte90
2 years ago

Any updates for this? maybe for 6.0?

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


2 years ago

#24 @chaion07
2 years ago

Thanks @dd32 for reporting this. We reviewed this ticket during a recent bug-scrub session. The team is hopeful that @hellofromTonya can take look at her convenience for this ticket to be moving forward during the 6.0 cycle.

Props to @peterwilsoncc

Cheers!

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


2 years ago

#26 @chaion07
2 years ago

  • Milestone changed from 6.0 to Future Release

Updating milestone to Future Release as per discussion during today's bug-scrub session. We can always pull this ticket back if the tests happen to land before the beta 1 which gets into effect on April 12.

Props to @peterwilsoncc & @costdev

Cheers!

Note: See TracTickets for help on using tickets.