[wp-trac] [WordPress Trac] #50910: 5.5 Sitemap URLs are incorrectly paginated

[WordPress Trac]

#50910: 5.5 Sitemap URLs are incorrectly paginated
--------------------------------------+-----------------------------
 Reporter:  jonathanstegall           |       Owner:  SergeyBiryukov
     Type:  defect (bug)              |      Status:  accepted
 Priority:  normal                    |   Milestone:  5.5.1
Component:  Sitemaps                  |     Version:  5.5
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:
--------------------------------------+-----------------------------

Comment (by pbiron):

 Regarding [https://github.com/WordPress/wordpress-
 develop/pull/482/files#r471067309 the comment on the PR]:

 > Fun this passes with phpunit --filter test_sitemaps_canonical_redirects
 but fails with the full run or phpunit --group canonical. My suspicion is
 something is hanging around $wp from a previous test.

 The reason the unit tests in the PR fail when run as `phpunit --group
 canonical` has to do with an interaction between the way sitemaps adds
 rewrite rules/tags and `WP_UnitTestCase_Base::go_to()`, which is called by
 `WP_Canonical_UnitTestCase::assertCanonical()`.

 Sitemaps adds rewrite tags (e.g., `sitemap`, `sitemap-subtype` and
 `sitemap-stylesheet`) on `init` and those tags are added to
 `WP::public_query_vars`.  `WP_UnitTestCase_Base::go_to()` creates a new
 instance of the `WP` class, but since sitemaps aren't "reinitialized",
 `$wp->public_query_vars` doesn't contain the sitemaps rewrite tags in
 subsequent tests.  I hope that makes sense!

 Moving the unit test to `Tests_Canonical_Sitemaps` gets around that
 problem since `Tests_Canonical_Sitemaps::setUp()` "manually" initializes
 sitemaps before each test and thus the rewrite tags are correctly added.

 That ''might'' be good enough, but I want to float an idea.  The sitemaps
 feature plugin had to call `add_rewrite_tag()` when adding it's rewrite
 rules.  But ''maybe'', now that sitemaps are in core, it would be better
 to add those directly to `WP::$public_query_vars`, as in:

 {{{#!php
 <?php
 class WP {
         /**
          * Public query variables.
          *
          * Long list of public query variables.
          *
          * @since 2.0.0
          * @since 5.5.1 Added 'sitemap', 'sitemap-subtype' and 'sitemap-
 stylesheet'
          * @var string[]
          */
         public $public_query_vars = array( 'm', 'p', 'posts', 'w', 'cat',
 'withcomments', 'withoutcomments', 's', 'search', 'exact', 'sentence',
 'calendar', 'page', 'paged', 'more', 'tb', 'pb', 'author', 'order',
 'orderby', 'year', 'monthnum', 'day', 'hour', 'minute', 'second', 'name',
 'category_name', 'tag', 'feed', 'author_name', 'pagename', 'page_id',
 'error', 'attachment', 'attachment_id', 'subpost', 'subpost_id',
 'preview', 'robots', 'favicon', 'taxonomy', 'term', 'cpage', 'post_type',
 'embed', 'sitemap', 'sitemap-subtype', 'sitemap-stylesheet' );
 }
 ...
 }
 }}}

 and **not** call `add_rewrite_tag()` in
 `WP_Sitemaps::add_rewrite_rules()`.

 @swissspidy what do you think about that idea?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/50910#comment:15>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform