WordPress.org

Make WordPress Core

Opened 6 years ago

Last modified 8 days ago

#28081 accepted defect (bug)

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

Reported by: ocean90 Owned by: SergeyBiryukov
Milestone: 5.6 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 6 years ago.
28081.diff (1.7 KB) - added by peterwilsoncc 3 years ago.
1.diff (743 bytes) - added by sagarkbhatt 2 years ago.
Add fix for duplicate content issue and fix unwanted pagination

Download all attachments as: .zip

Change History (50)

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


6 years ago

@ocean90
6 years ago

#3 @wonderboymusic
6 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.


6 years ago

#5 @DrewAPicture
6 years ago

  • Milestone changed from 4.0 to Future Release

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

#6 @SergeyBiryukov
6 years ago

#29599 was marked as a duplicate.

#8 @chriscct7
5 years ago

Patch still good but continues to need consensus

#9 @SergeyBiryukov
4 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 4 years ago by SergeyBiryukov (previous) (diff)

#10 @dd32
4 years ago

#39183 was marked as a duplicate.

#11 @SergeyBiryukov
4 years ago

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

#12 @SergeyBiryukov
3 years ago

#39899 was marked as a duplicate.

#13 @henry.wright
3 years ago

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

#14 @johnbillion
3 years ago

#29655 was marked as a duplicate.

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


3 years ago

#16 @henry.wright
3 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
3 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
3 years ago

I tested 28081.patch against a custom page template which uses a custom query var for 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 a custom pagination are a problem.

Version 1, edited 3 years ago by henry.wright (previous) (next) (diff)

#19 @SergeyBiryukov
3 years ago

#40848 was marked as a duplicate.

#20 @SergeyBiryukov
3 years ago

  • Milestone changed from Future Release to 4.9

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


3 years ago

#22 @SergeyBiryukov
3 years ago

#41758 was marked as a duplicate.

@peterwilsoncc
3 years ago

#23 @peterwilsoncc
3 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
3 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
3 years ago

  • Milestone changed from Future Release to 5.0

#26 @dd32
3 years ago

#42458 was marked as a duplicate.

#27 @chesio
3 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
3 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
2 years ago

Add fix for duplicate content issue and fix unwanted pagination

#29 @SergeyBiryukov
2 years ago

#43928 was marked as a duplicate.

#30 @swissspidy
2 years ago

#44105 was marked as a duplicate.

#31 @SergeyBiryukov
22 months ago

  • Milestone changed from 5.0 to 5.0.1

#32 @pento
20 months ago

  • Milestone changed from 5.0.1 to 5.0.2

#33 @SergeyBiryukov
20 months ago

  • Milestone changed from 5.0.2 to 5.1

#34 @SergeyBiryukov
19 months ago

#40773 was marked as a duplicate.

#35 @pento
19 months 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
19 months ago

  • Milestone changed from 5.1 to 5.2

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


16 months ago

#38 @audrasjb
16 months 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
16 months ago

#46844 was marked as a duplicate.

#40 @jeremyfelt
14 months 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 14 months ago by jeremyfelt (previous) (diff)

#41 @SergeyBiryukov
3 months 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
3 months 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
3 months 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
3 months ago

  • Milestone changed from Future Release to 5.5

#45 @bradleyt
3 months ago

#40307 was marked as a duplicate.

#46 @SergeyBiryukov
3 months ago

In 47781:

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

Follow-up to [47727].

See #28081, #40773, #45337.

#47 @SergeyBiryukov
8 days 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.

Note: See TracTickets for help on using tickets.