FIX: Ensure that canonical URLs don’t redirect elsewhere.

Redirecting canonical URLs violate HTTP rules and break some search 
spiders including Swiftype.

As part of this change, DocumentationPage loses responsibility for
calculating canonical URLs. I’ve opted to leave the methods in there
but they will throw warnings.

Fixes https://github.com/silverstripe/doc.silverstripe.org/issues/181
This commit is contained in:
Sam Minnee 2019-04-04 13:47:03 +13:00 committed by Robbie Averill
parent de147fb1a2
commit 0e922895f2
3 changed files with 25 additions and 20 deletions

View File

@ -588,10 +588,26 @@ class DocumentationViewer extends Controller implements PermissionProvider
*/
public function getCanonicalUrl()
{
if (!$this->getPage()) {
$page = $this->getPage();
if (!$page) {
return '';
}
return $this->getPage()->getCanonicalUrl();
// Build the basic canoncial URL, relative to link_base as is mapped in the manifest
$url = Controller::join_links(
$page->getEntity()->getLanguage(),
$page->getRelativeLink()
);
// If the canoncial URL is, in fact, a redirect, post that directly - redirecting canoncials are a bad thing.
// See https://github.com/silverstripe/doc.silverstripe.org/issues/181 for discussion
if ($potentialRedirect = $this->manifest->getRedirect($url)) {
$url = $potentialRedirect;
}
// Turn into an absolute URL
return Director::absoluteUrl(Controller::join_links(self::getDocumentationBaseHref(), $url));
}
/**

View File

@ -376,6 +376,8 @@ class DocumentationPage extends ViewableData
*/
public function setCanonicalUrl($canonicalUrl)
{
user_error("DocumentationPage::setCanonicalUrl() is deprecated; it won't affect DocumentationViewer", E_USER_WARNING);
$this->canonicalUrl = $canonicalUrl;
return $this;
}
@ -388,6 +390,11 @@ class DocumentationPage extends ViewableData
*/
public function getCanonicalUrl()
{
user_error(
"DocumentationPage::getCanonicalUrl() is deprecated; use DocumentationViewer::getCanonicalUrl() instead.",
E_USER_WARNING
);
if (!$this->canonicalUrl) {
$this->populateCanonicalUrl();
}

View File

@ -90,22 +90,4 @@ class DocumentationPageTest extends SapphireTest
$this->assertEquals('Sort - Doctest', $page->getBreadcrumbTitle());
}
public function testGetCanonicalUrl()
{
$page = new DocumentationPage(
$this->entity,
'file.md',
DOCSVIEWER_PATH . '/tests/docs/en/test/file.md'
);
$this->assertContains(
'dev/docs/en/test/file/',
$page->getCanonicalUrl(),
'Canonical URL is determined, set and returned'
);
$page->setCanonicalUrl('some-other-url');
$this->assertSame('some-other-url', $page->getCanonicalUrl(), 'Canonical URL can be adjusted via public API');
}
}