Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Fix news feeds in multidomain setups (#8327) #8329

Closed
wants to merge 2 commits into from

Conversation

fritzmg
Copy link
Contributor

@fritzmg fritzmg commented Apr 29, 2016

When using Contao in a multidomain setup, the news feed might contain the wrong links to the items (see #8327). This simple change should fix it completely.

@fritzmg
Copy link
Contributor Author

fritzmg commented Apr 30, 2016

I have updated the pull request. Now news entries with redirect targets to pages or articles will also have the correct absolute link in the news feed.

@fritzmg fritzmg changed the title fixed news feeds in multidomain setups (#8327) Fix news feeds in multidomain setups (#8327) May 2, 2016
@leofeyer
Copy link
Member

The changes are not correct. The getLink() method must only return absolute URLs if $strBase is set. Also, I'm pretty sure that the calendar bundle uses similar logic and should therefore be fixed as well, shouldn't it?

@fritzmg
Copy link
Contributor Author

fritzmg commented Jun 13, 2016

The changes are not correct. The getLink() method must only return absolute URLs if $strBase is set.

The getLink method currently uses:

$strBase . $objTarget->getFrontendUrl();

so, this might result in either

  • an absolute url
  • a relative URL
  • or a nonsense URL

because \PageModel::getFrontendUrl either returns an absolute URL or a relative URL, depending on the page's root DNS setting and the current base.

So the resulting URL might result in something like

http://www.example1.org/http://www.example2.org/news-entry/foo.html

in multidomain setups as shown in #8327.

Also, I'm pretty sure that the calendar bundle uses similar logic and should therefore be fixed as well, shouldn't it?

Yes, the structure is different, but as far as I can see the news feed generation for events will also be wrong in multidomain setups, because it also uses

$strBase . $objTarget->getFrontendUrl();

@leofeyer
Copy link
Member

$strBase needs to be removed of course and replaced by $blnAbsoluteUrl.

@fritzmg
Copy link
Contributor Author

fritzmg commented Jun 13, 2016

But that would break the backwards compatibility, wouldn't it? Someone might use the getLink function with a custom $strBase.

@leofeyer
Copy link
Member

Correct. Then we need a fourth parameter I guess.

@fritzmg
Copy link
Contributor Author

fritzmg commented Jun 13, 2016

Btw. this is also a case where a getRelativeUrl function (contao/core-bundle#458) would come in handy. Because if a $strBase is set, the URL generated for the page must be relative, otherwise it would be nonsense again.

i.e.

return $strBase ? $strBase . $objTarget->getRelativeUrl() : $objTarget->getAbsoluteUrl()

@leofeyer
Copy link
Member

Fixed in d16c629.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants