? ?

Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
4 Mar 2017

Pull Request for Issue #9698.

Summary of Changes

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.

Testing Instructions

  1. Enable modern routing
  2. Create an article with links to another article, a contact and a newsfeed and, disabling the editor, edit all three URLs to only contain &id= instead of &id=:.
  3. Check in the frontend that the URL looks right except for the last segment, which is only the ID.
  4. Enable "Remove IDs from URL" for all three components and check again and see that now all three links have an alias, but also still contain the ID.
  5. Apply these changes
  6. Check again in the frontend and see that all three links are now correctly formed, containing just an alias and no ID.
  7. Disable the "Remove IDs from URL" for all three components and now see that all three URLs contain an ID and an alias.

Documentation Changes Required

No doc changes required.

avatar Hackwar Hackwar - open - 4 Mar 2017
avatar Hackwar Hackwar - change - 4 Mar 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 4 Mar 2017
Category Front End com_contact com_content com_newsfeeds
avatar Hackwar
Hackwar - comment - 4 Mar 2017

This should also fix #14308

avatar philip-sorokin philip-sorokin - test_item - 4 Mar 2017 - Tested unsuccessfully
avatar philip-sorokin
philip-sorokin - comment - 4 Mar 2017

I have tested this item 🔴 unsuccessfully on a9781ac

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
avatar Hackwar
Hackwar - comment - 4 Mar 2017

@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.

avatar Hackwar
Hackwar - comment - 4 Mar 2017

@infograf768 Please add that to the appropriate issue, for example #13978. It has nothing to do with this PR.

avatar philip-sorokin
philip-sorokin - comment - 4 Mar 2017

@Hackwar

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.

avatar Hackwar
Hackwar - comment - 5 Mar 2017

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.

avatar philip-sorokin
philip-sorokin - comment - 5 Mar 2017

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...

avatar Hackwar Hackwar - change - 7 Mar 2017
Labels Added: ?
avatar Hackwar
Hackwar - comment - 7 Mar 2017

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.

avatar AlexRed AlexRed - test_item - 7 Mar 2017 - Tested successfully
avatar AlexRed
AlexRed - comment - 7 Mar 2017

I have tested this item successfully on 197e7d4


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14322.

avatar infograf768 infograf768 - test_item - 8 Mar 2017 - Tested successfully
avatar infograf768
infograf768 - comment - 8 Mar 2017

I have tested this item successfully on 197e7d4

Great! Works fine for me.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14322.

avatar infograf768 infograf768 - change - 8 Mar 2017
Status Pending Ready to Commit
avatar infograf768
infograf768 - comment - 8 Mar 2017

RTC.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14322.

avatar wilsonge wilsonge - change - 9 Mar 2017
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: ?
avatar wilsonge wilsonge - close - 9 Mar 2017
avatar wilsonge wilsonge - merge - 9 Mar 2017

Add a Comment

Login with GitHub to post a comment