Skip to content

Commit

Permalink
Remove column from articles URL (see #6594)
Browse files Browse the repository at this point in the history
Description
-----------

While preparing for the _Content URL Generator_, I noticed this: did you know that (sometimes) Contao generates the URL to an article as `.../articles/section:alias` and not just `.../articles/alias`?

The reason behind this is, that `Controller::getFrontendModule` will ignore the articles parameter if it does not belong the the given column (section). **However**, this was

a) not applied in all place
b) totally unnecessary because we can achieve the same correct thing without having it in the URL (see PR code)
c) allowed for URL manipulation, I could theoretically generate an article in the wrong section (not saying that would be a real problem, but … )

This PR removes the need for passing the section in the article alias, because it was irrelevant to most places anyway. This makes generating the URL of an article easier and probably also fixes bugs because no third-party developer I know would currently have generated a correct URL 😅

Commits
-------

6713bfe Remove column from articles URL
d4f7559 Also adjust the `Controller::redirectToFrontendPage()` method

Co-authored-by: leofeyer <1192057+leofeyer@users.noreply.github.com>
  • Loading branch information
aschempp and leofeyer committed Dec 5, 2023
1 parent d079895 commit 4f16e58
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 55 deletions.
10 changes: 1 addition & 9 deletions core-bundle/contao/elements/ContentTeaser.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,9 @@ public function generate()
*/
protected function compile()
{
$link = '/articles/';
$objArticle = $this->objArticle;

if ($objArticle->inColumn != 'main')
{
$link .= $objArticle->inColumn . ':';
}

$link .= $objArticle->alias ?: $objArticle->id;
$this->Template->href = $this->objParent->getFrontendUrl($link);

$this->Template->href = $this->objParent->getFrontendUrl('/articles/' . ($objArticle->alias ?: $objArticle->id));
$this->Template->text = $objArticle->teaser;
$this->Template->headline = $objArticle->title;
$this->Template->readMore = StringUtil::specialchars(sprintf($GLOBALS['TL_LANG']['MSC']['readMore'], $objArticle->title));
Expand Down
29 changes: 15 additions & 14 deletions core-bundle/contao/library/Contao/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -304,24 +304,25 @@ public static function getFrontendModule($intId, $strColumn='main')
// Show a particular article only
if ($objPage->type == 'regular' && Input::get('articles'))
{
list($strSection, $strArticle) = explode(':', Input::get('articles')) + array(null, null);
$strArticle = Input::get('articles');

if ($strArticle === null)
if (str_contains($strArticle, ':'))
{
$strArticle = $strSection;
$strSection = 'main';
trigger_deprecation('contao/core-bundle', '5.3', 'Passing the column of an article in the URL is deprecated. Only provide the article alias instead.');

list(, $strArticle) = explode(':', Input::get('articles'));
}

if ($strSection == $strColumn)
{
$objArticle = ArticleModel::findPublishedByIdOrAliasAndPid($strArticle, $objPage->id);
$objArticle = ArticleModel::findPublishedByIdOrAliasAndPid($strArticle, $objPage->id);

// Send a 404 header if there is no published article
if (null === $objArticle)
{
throw new PageNotFoundException('Page not found: ' . Environment::get('uri'));
}
// Send a 404 header if there is no published article
if (null === $objArticle)
{
throw new PageNotFoundException('Page not found: ' . Environment::get('uri'));
}

if ($objArticle->inColumn == $strColumn)
{
// Send a 403 header if the article cannot be accessed
if (!static::isVisibleElement($objArticle))
{
Expand Down Expand Up @@ -1204,9 +1205,9 @@ protected function redirectToFrontendPage($intPage, $strArticle=null, $blnReturn
$strParams = null;

// Add the /article/ fragment (see #673)
if ($strArticle !== null && ($objArticle = ArticleModel::findByAlias($strArticle)) !== null)
if ($strArticle)
{
$strParams = '/articles/' . (($objArticle->inColumn != 'main') ? $objArticle->inColumn . ':' : '') . $strArticle;
$strParams = '/articles/' . $strArticle;
}

$strUrl = $objPage->getPreviewUrl($strParams);
Expand Down
9 changes: 2 additions & 7 deletions core-bundle/contao/modules/ModuleArticle.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,9 @@ protected function compile()
$this->cssID = $arrCss;
}

$article = $this->alias ?: $this->id;
$href = '/articles/' . (($this->inColumn != 'main') ? $this->inColumn . ':' : '') . $article;

$this->Template->teaserOnly = true;
$this->Template->headline = $this->headline;
$this->Template->href = $objPage->getFrontendUrl($href);
$this->Template->href = $objPage->getFrontendUrl('/articles/' . ($this->alias ?: $this->id));
$this->Template->teaser = $this->teaser ?? '';
$this->Template->readMore = StringUtil::specialchars(sprintf($GLOBALS['TL_LANG']['MSC']['readMore'], $this->headline), true);
$this->Template->more = $GLOBALS['TL_LANG']['MSC']['more'];
Expand All @@ -152,9 +149,7 @@ protected function compile()
}

// Get section and article alias
$chunks = explode(':', Input::get('articles') ?? '');
$strSection = $chunks[0] ?? null;
$strArticle = $chunks[1] ?? $strSection;
$strArticle = Input::get('articles');

// Overwrite the page metadata (see #2853, #4955 and #87)
if (!$this->blnNoMarkup && $strArticle && ($strArticle == $this->id || $strArticle == $this->alias) && $this->title)
Expand Down
17 changes: 2 additions & 15 deletions core-bundle/contao/modules/ModuleBreadcrumb.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,28 +163,15 @@ protected function compile()
'data' => $pages[0]->row(),
);

list($strSection, $strArticle) = explode(':', Input::get('articles')) + array(null, null);

if ($strArticle === null)
{
$strArticle = $strSection;
}

$objArticle = ArticleModel::findByIdOrAlias($strArticle);
$strAlias = $objArticle->alias ?: $objArticle->id;

if ($objArticle->inColumn != 'main')
{
$strAlias = $objArticle->inColumn . ':' . $strAlias;
}
$objArticle = ArticleModel::findByIdOrAlias(Input::get('articles'));

if ($objArticle !== null)
{
$items[] = array
(
'isRoot' => false,
'isActive' => true,
'href' => $this->getPageFrontendUrl($pages[0], '/articles/' . $strAlias),
'href' => $this->getPageFrontendUrl($pages[0], '/articles/' . ($objArticle->alias ?: $objArticle->id)),
'title' => StringUtil::specialchars($objArticle->title, true),
'link' => $objArticle->title,
'data' => $objArticle->row(),
Expand Down
10 changes: 1 addition & 9 deletions core-bundle/src/Controller/ContentElement/TeaserController.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,9 @@ protected function getResponse(FragmentTemplate $template, ContentModel $model,

[$article, $page] = $articleAndPage;

$href = $page->getFrontendUrl(
sprintf(
'/articles/%s%s',
'main' !== $article->inColumn ? "$article->inColumn:" : '',
$article->alias ?: $article->id,
),
);

$template->set('article', $article);
$template->set('page', $page);
$template->set('href', $href);
$template->set('href', $page->getFrontendUrl('/articles/'.($article->alias ?: $article->id)));

return $template->getResponse();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private function getParams(Request $request, int $pageId): string|null
}

// Add the /article/ fragment (see contao/core-bundle#673)
return sprintf('/articles/%s%s', 'main' !== $article->inColumn ? $article->inColumn.':' : '', $article->alias);
return '/articles/'.($article->alias ?: $article->id);
}

private function getFragmentUrl(Request $request, PageModel $pageModel): string
Expand Down

0 comments on commit 4f16e58

Please sign in to comment.