User tests: Successful: Unsuccessful:
Pull Request for Issue #9698.
The modern router currently has a slightly broken behavior regarding malformed router input. If a URL given to the router does not contain a complete slug, it will either create a URL without an alias (when it is supposed to create them with IDs), or it will create a URL with a proper alias, but also with an ID, even though removing IDs is enabled. This fixes that.
No doc changes required.
Status | New | ⇒ | Pending |
Category | ⇒ | Front End com_contact com_content com_newsfeeds |
I have tested this item
JRoute::_()
works fine, but the component router makes two database queries insted of single query. Please, pay attention that URLs are not going into cache in JRoute::_()
after building.
components/com_content/router.php
if (!strpos($id, ':'))
{
$db = JFactory::getDbo();
$dbquery = $db->getQuery(true);
$dbquery->select($dbquery->qn('alias'))
->from($dbquery->qn('#__content'))
->where('id = ' . $dbquery->q($id));
$db->setQuery($dbquery);
$id .= ':' . $db->loadResult();
echo $id . '<br>';
}
components/com_contact/router.php
echo $id . '<br>';
Execute the code:
require_once JPATH_ROOT . '/components/com_content/helpers/route.php';
require_once JPATH_ROOT . '/components/com_contact/helpers/route.php';
require_once JPATH_ROOT . '/components/com_newsfeeds/helpers/route.php';
echo JRoute::_(ContentHelperRoute::getArticleRoute(8, 19)) . '<br>';
echo JRoute::_(ContactHelperRoute::getContactRoute(2, 34)) . '<br>';
echo JRoute::_(NewsfeedsHelperRoute::getNewsfeedRoute(2, 17)) . '<br>';
And you will see the result:
8:beginners
8:beginners
/using-joomla/extensions/components/content-component/article-category-list/beginners.html
2:webmaster
2:webmaster
/using-joomla/extensions/components/contact-component/contact-categories/park-site/webmaster.html
/using-joomla/extensions/components/news-feeds-component/news-feed-category/new-joomla-extensions.html
@philip-sorokin I don't understand you. The URL is cached in JRouter. I don't know why the method apparently is called twice. But since this should be the exception and not the norm for a call to a component router, an additional, lightweight query doesn't seem like a problem to me.
@infograf768 Please add that to the appropriate issue, for example #13978. It has nothing to do with this PR.
If I add a link in an article <a href="index.php?option=com_content&view=article&catid=19&id=8">Link</a>
, it will pass through JRoute
in the same way, so, it cannot be an exception. To build this link the component router will make 2 queries.
About caching. If I add a slightly modified link <a href="index.php?option=com_content&view=article&catid=19&id=8&start=3" />Link</a>
, the component router will make another unnecessary query (twice). What if I want to add 100 page links in an article?
And I am not talking about different links, but behaviour in this case the same.
The URL normally contains the slug of the article. In that case no query is run. So when you deliberately leave that of, you have to run additional queries, since we otherwise don't know how to build the URL. Yes, slightly different URLs will not hit the cache. But that is because normally those point to somewhere else. Right now this is an acceptable trade-off for me.
Normally, when you add an article link in your text editor, the link does not contain slug, it contains only category and article IDs, and this is good, because I can change the article alias without editing this link placed in another article.
BTW, I noticed that some article links are built twice, while other only once, as expected. The link in my example is built twice.
Nothing criminal, but...
Labels |
Added:
?
|
I have extended the PR to fix another issue that came up with the list all categories view. See #13978 (comment) for the problem and testing instructions.
I have tested this item
I have tested this item
Great! Works fine for me.
Status | Pending | ⇒ | Ready to Commit |
RTC.
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-03-09 13:44:41 |
Closed_By | ⇒ | wilsonge | |
Labels |
Added:
?
|
This should also fix #14308