Joomla-cms: JRoute::_() broken in 3.8.4

[OctavianC]

Steps to reproduce the issue

Itemid was supposed to be inherited from the current menu item - this is how it's always been as far as I remember (1.5.0) and what the SEF docs say.

• SEF turned off (doesn't matter, issue occurs regardless of SEF but it's easier to debug)
• Create an Articles (com_content) menu item - doesn't matter which type
• Open /components/com_content/content.php
• Replace it with the code:

var_dump(JRoute::_('index.php?option=com_content&view=test'));
var_dump(JRoute::_('index.php?view=test'));
die;

• Access the menu item.

Expected result

string(63) "/j384/index.php?option=com_content&view=test&Itemid=121" string(63) "/j384/index.php?view=test&option=com_content&Itemid=121"

Actual result

string(48) "/j384/index.php?option=com_content&view=test" string(63) "/j384/index.php?view=test&option=com_content&Itemid=121"

System information (as much as possible)

3.8.4

Additional comments

This is a regression from #19099
If you replace libraries/src/Router/SiteRouter.php with the one from 3.8.3 everything works as it should.
If there's no option= in the URL, then the Itemid gets inherited correctly, but I don't think developers should have to rewrite all URLs because of this (what I consider to be...) B/C change.

[roland-d]

@csthomas Can you have a look at this?

[OctavianC]

Fix is rather easy for this case, but since that MR dealt with several other cases, I'm not sure if this would break them? Just update the createUri() function to include the else clause that was removed by the PR:

protected function createUri($url)
{
    // Create the URI
    $uri = parent::createUri($url);

    // Get the itemid form the URI
    $itemid = $uri->getVar('Itemid');

    if ($itemid === null)
    {
        if (!$uri->getVar('option'))
        {
            $option = $this->getVar('option');

            if ($option)
            {
                $uri->setVar('option', $option);
            }

            $itemid = $this->getVar('Itemid');

            if ($itemid)
            {
                $uri->setVar('Itemid', $itemid);
            }
        }
        else
        {
            if ($itemid = $this->getVar('Itemid'))
            {
                $uri->setVar('Itemid', $itemid);
            }
        }
    }
    else
    {
        if (!$uri->getVar('option'))
        {
            if ($item = $this->menu->getItem($itemid))
            {
                $uri->setVar('option', $item->component);
            }
        }
    }

    return $uri;
}

[csthomas]

There was a change in routing and now

Itemid was supposed to be inherited from the current menu item

only when you do not add an option=... to link.

For full link like JRoute::_('index.php?option=com_content&view=test') there is no inheritance.

but there is inheritance for JRoute::_('index.php?view=test') but I'm not sure if this is correct.

In general, you can not inherit Itemid from other component.

[OctavianC]

It's a B/C change and breaks several of our extensions. I don't see why this change was made and I don't really see the reasoning behind omitting option so that everything works - especially when the current URI option matches, perhaps there should be a change to account for that?
Something in the likes of:

else
{
    if ($this->getVar('option') === $uri->getVar('option') && ($itemid = $this->getVar('Itemid')))
    {
        $uri->setVar('Itemid', $itemid);
    }
}

[csthomas]

A full link should not inherit Itemid.

If there is no menu item for full link then it should be without Itemid (/component/content/article/1) or with default (current lang) for multilingual website (/en/component/content/article/1).

I did it for one purpose. The full link can not depend on the page on which it is generated.

[brianteeman]

Itemid is the glue that holds the site together

If there is no itemid then what are you expecting to be used to display modules or tempplate?

[OctavianC]

Sorry but I still think the reason for this is flawed:

• It breaks compatibility - I've been writing URLs as index.php?option=com_test&view=articles as far as I remember and I don't think I'm the only one. Why should extensions break? The PR had to change test cases (test cases were removed, why?!) so this clearly wasn't thought through.
• I don't think it takes into account "subpages" (think of index.php?option=com_test&view=article&id=1 - this would inherit the original Itemid and display a nice SEF URL, such as /articles/1-my-article
• It magically works when omitting the option.

I agree it shouldn't inherit the Itemid if the option is different, so that's why I'm going to create a PR that fixes this.

But I honestly think #19099 should be reverted entirely.

[Bakual]

For full link like JRoute::_('index.php?option=com_content&view=test') there is no inheritance.
but there is inheritance for JRoute::_('index.php?view=test') but I'm not sure if this is correct.

That certainly makes no sense and looks wrong. It shouldn't matter at all if the option is passed or not. Actually I would say it should be passed always anyway.

A full link should not inherit Itemid.

Of course it should, especially if the option matches.

If there is no menu item for full link then it should be without Itemid (/component/content/article/1) or with default (current lang) for multilingual website (/en/component/content/article/1).

There should be no case where NO menu item is assigned since this will break a lot (templatestyle, modules). So at least the default menuitem should be assigned.

[joomla-cms-bot]

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/19497

[ghost]

closed as having Pull Request #19498

[brianteeman]

I am keeping this open for now. It will be more visible for new posters and will enable further discussion as to should it be reverted in its entirety or just partially as suggested in #19408

[csthomas]

I understand the routing as, joomla uses menu item to URL only if it match

• ex Itemid for article should be use only for certain article.
• for top category, which does not have own menu item and there is no menu item for categories view, the link should look like /component/content/category/[id-category-alias]

If there is no itemid then what are you expecting to be used to display modules or tempplate?

Modules assignment equal to "On all pages" or "On all pages except those selected" will be visible.
Default template will be loaded.

If there's no option= in the URL, then the Itemid gets inherited correctly

IMO a link like JRoute::_('index.php?view=test') also should not inherit Itemid. Only links like &start=... or index.php?start=... should inherit Itemid.
I forgot about it when I added PR.

[brianteeman]

That is a break in 13 years of behaviour.

Feeling annoyed at myself that I was working on j4 stuff and never tested or fully understood your PR

[Bakual]

Modules assignment equal to "On all pages" or "On all pages except those selected" will be visible.
Default template will be loaded.

THERE SHOULD ALWAYS BE AN ITEMID.

[csthomas]

Itemid was supposed to be inherited from the current menu item - this is how it's always been as far as I remember (1.5.0) and what the SEF docs say.

I did not see any text that say: Always inherit Itemid from current page.
There is only sentence:

echo \Joomla\CMS\Router\Route::_('index.php?view=article&id=1&catid=20');

Notice that it is possible to leave out the parameters option and Itemid. option defaults to the name of the component currently being executed, and Itemid defaults to the current menu item's ID.

I understood it as there is no "option" parameter, then use inheritance Itemid otherwise no.

In PR #19501 I added additional inheritance when there is option parameter but view is missing.

[OctavianC]

That doesn't make any sense.

[csthomas]

Because I can not convince anybody, I will not convince any more. Do what you have to do.

If you want to revert all this "improvement" then there is a list to potentially PRs to revert:
https://github.com/joomla/joomla-cms/pull/19415
https://github.com/joomla/joomla-cms/pull/19295
https://github.com/joomla/joomla-cms/pull/19268
https://github.com/joomla/joomla-cms/pull/19099

[brianteeman]

Closed as the routing changes have been reverted for 3.8.5 with #19512