Make WordPress Core

Opened 10 years ago

Last modified 12 months ago

#28081 accepted defect (bug)

Do a canonical redirect for pages when query var 'paged' is set

Reported by: ocean90's profile ocean90 Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Canonical Keywords: has-patch SEO needs-unit-tests needs-refresh
Focuses: Cc:

Description

Example: http://make.wordpress.org/core/features-as-plugins/page/2323/

You can append /page/{any number} to a page and still get the same content as http://make.wordpress.org/core/features-as-plugins/

The same doesn't happen for posts.

trunk/src/wp-includes/canonical.php#L274: Seems like l274 and l276 should be !is_singular() as it was before [6115]. (Block was changed in [9697].)

Want to use this ticket to get some reasons for [6115]. Currently I only can think of custom page templates which are using a custom pagination, so maybe wontfix.

Attachments (3)

28081.patch (787 bytes) - added by ocean90 10 years ago.
28081.diff (1.7 KB) - added by peterwilsoncc 7 years ago.
1.diff (743 bytes) - added by sagarkbhatt 6 years ago.
Add fix for duplicate content issue and fix unwanted pagination

Download all attachments as: .zip

Change History (57)

This ticket was mentioned in IRC in #wordpress-dev by ocean90. View the logs.


10 years ago

@ocean90
10 years ago

#3 @wonderboymusic
10 years ago

  • Keywords has-patch added; dev-feedback removed
  • Milestone changed from Awaiting Review to 4.0

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


10 years ago

#5 @DrewAPicture
10 years ago

  • Milestone changed from 4.0 to Future Release

Seems like this needs a bit more discussion and a consensus. Punting.

#6 @SergeyBiryukov
10 years ago

#29599 was marked as a duplicate.

#8 @chriscct7
8 years ago

Patch still good but continues to need consensus

#9 @SergeyBiryukov
7 years ago

Leaving a note here that apparently the fix for #11694 was incomplete.

It introduced a sanity check for posts that use the <!--nextpage--> tag, but those that don't use it are still affected: https://make.wordpress.org/core/2016/09/15/media-weekly-update-sept-9/12345/

Last edited 7 years ago by SergeyBiryukov (previous) (diff)

#10 @dd32
7 years ago

#39183 was marked as a duplicate.

#11 @SergeyBiryukov
7 years ago

  • Keywords needs-unit-tests added
  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#12 @SergeyBiryukov
7 years ago

#39899 was marked as a duplicate.

#13 @henry.wright
7 years ago

This patch is still good; I tested today against 4.7.4.

#14 @johnbillion
7 years ago

#29655 was marked as a duplicate.

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


7 years ago

#16 @henry.wright
7 years ago

You can append /page/{any number} to a page and still get the same content as http://make.wordpress.org/core/features-as-plugins/

FYI

You can also append /{any number} to a page and still get the same content.

#17 @henry.wright
7 years ago

Note that the issue I mention in #16 is not fixed by 28081.patch.

28081.patch does fix:

https://make.wordpress.org/core/features-as-plugins/page/2323/

28081.patch doesn't fix:

https://make.wordpress.org/core/features-as-plugins/2323/

#18 @henry.wright
7 years ago

I tested 28081.patch against a custom page template which uses pagination.

Tried visiting the following with the patch applied:

  • example.com/pagename/page/2/
  • example.com/pagename/page/3/
  • example.com/pagename/page/4/ etc

...all have items to show but they all redirect to example.com/pagename/

As @ocean90 suggested above, custom page templates which are using pagination are a problem.

Last edited 7 years ago by henry.wright (previous) (diff)

#19 @SergeyBiryukov
7 years ago

#40848 was marked as a duplicate.

#20 @SergeyBiryukov
7 years ago

  • Milestone changed from Future Release to 4.9

This ticket was mentioned in Slack in #forums by sergey. View the logs.


7 years ago

#22 @SergeyBiryukov
7 years ago

#41758 was marked as a duplicate.

@peterwilsoncc
7 years ago

#23 @peterwilsoncc
7 years ago

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

In 28081.diff: original patch with unit test for canonical URL against a page with the paged query var.

#24 @dd32
6 years ago

  • Milestone changed from 4.9 to Future Release

Punting as there's been no activity in several months, and any changes to canonical generally need a lot of testing for edge cases none of us expected would ever be a thing.
@SergeyBiryukov feel free to re-milestone when you commit, if you do so this release.

#25 @SergeyBiryukov
6 years ago

  • Milestone changed from Future Release to 5.0

#26 @dd32
6 years ago

#42458 was marked as a duplicate.

#27 @chesio
6 years ago

Something I just noticed, perhaps related:

https://make.wordpress.org/core/features-as-plugins/page/123/ has <link rel="canonical" href="https://make.wordpress.org/core/features-as-plugins/" /> - duplicate content, but canonical URL is correct

vs.

https://make.wordpress.org/core/features-as-plugins/123/ has <link rel="canonical" href="https://make.wordpress.org/core/features-as-plugins/123/" /> - duplicate content with duplicate canonical URL

#28 @joostdevalk
6 years ago

  • Keywords SEO added

@chesio yeah the first example you give is not a correct URL for a paginated version of the post, whereas the second is (even though there is of course no pagination within the post), and the canonical code doesn't handle that well.

@sagarkbhatt
6 years ago

Add fix for duplicate content issue and fix unwanted pagination

#29 @SergeyBiryukov
6 years ago

#43928 was marked as a duplicate.

#30 @swissspidy
6 years ago

#44105 was marked as a duplicate.

#31 @SergeyBiryukov
5 years ago

  • Milestone changed from 5.0 to 5.0.1

#32 @pento
5 years ago

  • Milestone changed from 5.0.1 to 5.0.2

#33 @SergeyBiryukov
5 years ago

  • Milestone changed from 5.0.2 to 5.1

#34 @SergeyBiryukov
5 years ago

#40773 was marked as a duplicate.

#35 @pento
5 years ago

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

This is some deep magic, I fell down a rabbit hole reviewing it.

Moving to 5.2. Adding many more unit tests showing that it works, and doesn't have side effects, would improve the chances of it actually landing.

#36 @pento
5 years ago

  • Milestone changed from 5.1 to 5.2

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


5 years ago

#38 @audrasjb
5 years ago

  • Keywords needs-refresh added
  • Milestone changed from 5.2 to Future Release

The last patch still needs a refresh and some unit-tests. As per today's bug scrub, let's move the ticket to future release.

#39 @SergeyBiryukov
5 years ago

#46844 was marked as a duplicate.

#40 @jeremyfelt
5 years ago

Related, #45337, which deals with the page query var and a similar issue rather than the paged query var covered by this ticket. (I think)

Last edited 5 years ago by jeremyfelt (previous) (diff)

#41 @SergeyBiryukov
4 years ago

In 47727:

Canonical: Redirect paged requests for non-paginated posts to the post permalink.

This avoids displaying duplicate content of the same post under different URLs and ensures the canonical URL is correct.

Previously, requests for invalid page numbers were only redirected to the post permalink if the post was actually paginated using the <!--nextpage--> marker.

Follow-up to [34492].

Props jeremyfelt, prografika, sachit.tandukar, subrataemfluence, hronak, ekatherine, henry.wright, chesio, dd32, SergeyBiryukov.
Fixes #40773. See #45337, #28081, #11694.

#42 @SergeyBiryukov
4 years ago

In 47760:

Canonical: Redirect paged requests for a static page assigned as the "Posts page".

This avoids displaying duplicate content of the home page under different URLs with appended page numbers.

This change only affects the <!--nextpage--> pagination (page query variable) and not the regular multiple posts pagination (paged query variable).

The posts page does not support the <!--nextpage--> pagination, so requests for invalid page numbers should be redirected to the page permalink, applying the logic previously implemented for single posts or pages.

Follow-up to [34492], [47727].

Props jeremyfelt, sachit.tandukar, SergeyBiryukov.
Fixes #45337. See #40773, #28081, #11694.

#43 @SergeyBiryukov
4 years ago

In 47761:

Canonical: Only redirect non-existing page requests to the post permalink if the post is found.

Follow-up to [47760].

See #45337, #40773, #28081, #11694.

#44 @peterwilsoncc
4 years ago

  • Milestone changed from Future Release to 5.5

#45 @bradleyt
4 years ago

#40307 was marked as a duplicate.

#46 @SergeyBiryukov
4 years ago

In 47781:

Tests: Give canonical test fixtures for paginated content more descriptive names.

Follow-up to [47727].

See #28081, #40773, #45337.

#47 @SergeyBiryukov
4 years ago

  • Milestone changed from 5.5 to 5.6

Didn't get a chance to bring this one over the finish line. Moving to 5.6, along with #50163.

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


3 years ago

#49 @SergeyBiryukov
3 years ago

  • Milestone changed from 5.6 to 5.7

#50 @SergeyBiryukov
3 years ago

#52478 was marked as a duplicate.

#51 @lukecarbis
3 years ago

  • Milestone changed from 5.7 to Future Release

We're too late in the release cycle for 5.7 to get this one through. Moving to Future Release.

#52 @SergeyBiryukov
3 years ago

#53470 was marked as a duplicate.

#53 @oglekler
2 years ago

The issue is still there. I've checked in 5.8 and 5.9-RC1 versions. Such pages are returning the 200 code but at least have the correct canonical.

While testing the sites that I updating I was thinking that I have an issue with some plugin and turned out this is almost a norm ( And most inconvenient is that as a developer you are trying to clarify a problem that other developers have spent time clarifying as well.

To know the system's existing issues is a big thing as well as learning its features, abilities and API. I am suggesting establishing a 'known issues digest' to publish alongside releases with a call for contributors to solve them.

#54 @SergeyBiryukov
12 months ago

#57970 was marked as a duplicate.

Note: See TracTickets for help on using tickets.