Make WordPress Core

Opened 4 years ago

Last modified 2 days 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: 6.7 Priority: normal
Severity: normal Version:
Component: Canonical Keywords: has-patch has-screenshots early has-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 4 years ago.
Patch refresh
Capture d’écran 2020-11-05 à 22.50.34.png (306.3 KB) - added by audrasjb 4 years ago.
before patch: example.com/page/3 exists
0dfda039df09e2d4fba44bd9788a2c7e.gif (352.7 KB) - added by audrasjb 4 years ago.
After patch: example.com/page/3 redirects to homepage

Download all attachments as: .zip

Change History (35)

#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
4 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.


4 years ago

@audrasjb
4 years ago

Patch refresh

@audrasjb
4 years ago

before patch: example.com/page/3 exists

@audrasjb
4 years ago

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

#6 @audrasjb
4 years ago

  • Keywords has-screenshots added; needs-refresh removed

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

#7 @mukesh27
4 years ago

Patch 50163.diff applied fine in trunk version

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

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

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


3 years ago

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


3 years ago

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


3 years ago

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


3 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!

#27 @hellofromTonya
2 months ago

  • Owner hellofromTonya deleted

As of today, I'm no longer a sponsored contributor and am starting a month or more AFK break. To not stand in the way, I'm clearing me as the ticket owner, which opens the ticket for someone to step in to shepherd this ticket forward to resolution.

#28 @Mte90
2 months ago

Any hope for this?
There is a ton of spam on GSC for urls with /page/[number] that doesn't exists, and this issue once resolved should help in some cases.
I see that the PR is waiting a review from @SergeyBiryukov since 4 years on github.

#29 @SergeyBiryukov
2 months ago

  • Milestone changed from Future Release to 6.6
  • Owner set to SergeyBiryukov
  • Status changed from assigned to accepted

This ticket was mentioned in PR #6421 on WordPress/wordpress-develop by @rajinsharwar.


8 weeks ago
#30

  • Keywords has-unit-tests added; needs-unit-tests removed

Using the 'paged' var if the singular page is the front page with unit tests

Trac ticket: https://core.trac.wordpress.org/ticket/50163

#31 @rajinsharwar
8 weeks ago

  • Keywords commit added

Sent a new PR, and also added the unit tests. As this was agreed that the patch would work, and only was missing unit tests, marking this as a candidate for commit.

#32 @hellofromTonya
2 days ago

  • Keywords commit removed
  • Milestone changed from 6.6 to 6.7
  • Owner changed from SergeyBiryukov to hellofromTonya
  • Status changed from accepted to assigned

This ticket was previously marked as an early candidate. Though it's not touching wpdb, it is touching an area that likely needs a longer soak time to ensure it works properly in Site Editor, front-end, etc. Thinking this one will be better for early commit in 6.7-alpha, which should open in less than 2 weeks.

Moving it to 6.7.

Also removing the commit keyword, as the PR is not yet approved. And now that I'm sponsored again, if @SergeyBiryukov doesn't mind, I'll reassign ownership to myself.

Hey Sergey, please feel free to reassign to yourself and move it back into the milestone if you think it's ready.

Note: See TracTickets for help on using tickets.