Skip to content

Keep the sitemap <loc> a localized URL - #123

Open
AIC-BV wants to merge 1 commit into
wintercms:mainfrom
AIC-BV:fix/sitemap-loc-must-be-localized
Open

Keep the sitemap <loc> a localized URL#123
AIC-BV wants to merge 1 commit into
wintercms:mainfrom
AIC-BV:fix/sitemap-loc-must-be-localized

Conversation

@AIC-BV

@AIC-BV AIC-BV commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Problem

The winter.sitemap.addItem listener rewrites <loc> to the alternate for the default locale and clones a <url> for each of the others:

if ($locale === $defaultLocale->code) {
    $loc->nodeValue = $altUrl;
    continue;
}
$newElement = $urlElement->cloneNode(true);

When the default locale is not among alternateLinks, the rewrite never happens and the element keeps the untranslated URL it was built with. That URL belongs to no locale and, on an install that prefixes locales, redirects — so the sitemap advertises a redirect rather than a canonical URL, and the page also gets an extra entry no locale claims.

A listener on pages.menuitem.resolveItem may legitimately narrow alternateLinks. Ours does: three storefronts share one install, each on its own host serving its own subset of locales, and a host's sitemap must not announce hreflang for another host's languages. On the storefront whose locales exclude the global default, every page ended up with a redirecting <loc>.

Change

Fall back to the first alternate when the default locale is absent, so a <loc> is always one of the alternates:

$primaryLocale = isset($itemInfo['alternateLinks'][$defaultLocale->code])
    ? $defaultLocale->code
    : array_key_first($itemInfo['alternateLinks']);

Impact

Unfiltered alternateLinks always contain the default locale, so $primaryLocale is the default locale and behaviour is unchanged for a normal install. Verified on ours: the sitemap of the storefront that does serve the default locale comes out byte-identical before and after, hreflang ordering included, while the storefront that does not went from 115 redirecting URLs to none.

Summary by CodeRabbit

  • Bug Fixes
    • Improved sitemap generation so the primary URL is populated using the default locale when available, or an alternate locale when it is not.

The winter.sitemap.addItem listener rewrites <loc> to the alternate for the
default locale and clones a <url> for each of the others. When the default
locale is not among alternateLinks the rewrite never happens and the element
keeps the untranslated URL it was built with — a URL that belongs to no locale
and, on an install that prefixes locales, redirects. The sitemap then
advertises a redirect instead of a canonical URL.

A listener on pages.menuitem.resolveItem may legitimately narrow
alternateLinks: serving a subset of the locales per host, for instance. Falling
back to the first alternate keeps every <loc> a real localized URL. Unfiltered
alternateLinks always contain the default locale, so nothing changes for the
usual case.
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 13b823e3-fdd3-4959-a0bb-da78c9842509

📥 Commits

Reviewing files that changed from the base of the PR and between 1144951 and 08fd4ea.

📒 Files selected for processing (1)
  • Plugin.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

Sitemap generation now selects the default locale URL for <loc> when that locale exists in alternateLinks. If the default locale is unavailable after filtering, it selects the first available alternate locale instead.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 08fd4

Sitemap entries now use the default localized URL when available and otherwise an available alternate locale, avoiding untranslated or redirecting primary URLs for filtered locale sets. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: ensuring that the sitemap uses a localized URL.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Some tools did not complete. Review the errors below.

🔧 PHPStan (2.2.8)

Composer install failed: dependency resolution error. Check composer.json and composer.lock for version constraints.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant