Please provide some test instructions and describe how to reproduce the actual issue.

@SniperSister :

Can't say for sure what triggers the problem ($query variable without an id key). If you look into the commit, you'll see there's just added a missing check for variable.

Testing instructions:

• Set php error log reporting to max
• Create Home page with featured articles.
• Navigate to home page of the site, you should see one or more PHP notices: (Notice: Undefined index: id in ..\components\com_content\router.php on line 69)

@piotr-cz
About testing instructions: "Set php error log reporting to max" you are talking about joomla debug mode level or server confiuration?

@in-code

you are talking about joomla debug mode level or server confiuration?

see: Error Reporting in the Global Configuration --> Tab: Server

@zero-24

see: Error Reporting in the Global Configuration --> Tab: Server

That's what I thought. Can't reproduce this bug

I have tested this item unsuccessfully on fc28dea

Can't reproduce the error in my environment.

I have tested this item unsuccessfully on fc28dea

As I write before I can't reproduce this error

Closed as not reproducible

I have seen this notice get printed, (i will need to do some digging to find the exact scenario to reproduce it)

in configurations, that it does occur, usually it goes undetected or ignored, because error_reporting is usually off

in any case the check should have been there in the first place

i don't understand how this can not be defined - and until I understand I'm reluctant to merge code when it may just be masking the symptom of a bigger problem. If you can give me a reproducible case where this comes up I'm more than happy to reconsider :)

hhmm i will spend time to search (sooner or later)

but can anyone here provide proof that it is always set ?

• if we can not provide proof that it is always set then the check should have been there in the first place, or do i say something irrational ?

Hello

@wilsonge

i don't understand how this can not be defined

please re-open this, ... although the fix proposed here is incomplete and needs to be updated

Because in some cases the router **will try to use and possible even use the active menu item even of it belongs to a different component,

• thus you will may get a PHP notice
• or even worst you can get a broken SEF URL (404 error)**

I have replicated it and i also have an explanation of why it is happening:

• the router tries to compare with the currently active menu - item
• and tries even if it belongs to a different component

see this line:
https://github.com/joomla/joomla-cms/blob/staging/components/com_content/router.php#L45

// We need a menu item.  Either the one specified in the query, or the current active one if none specified
if (empty($query['Itemid']))
{
    $menuItem = $this->menu->getActive();
    $menuItemGiven = false;
}

then this line will not exclude active menu even if it belongs to a different component, because it checks "menuItemGiven" variable
https://github.com/joomla/joomla-cms/blob/staging/components/com_content/router.php#L55

Thus if the currently active menu item that can belong to any component

• has the *same view as the URL being built *

A. if does not have the 'id' variable

• then the notice will occur

B. if 'id' for the menu item is set and it is also the same value as the URL being built

• then you build the URL using the menu item of a different component aka a broken SEF URL

e.g. example for you get PHP notice,
*The active menu item *

stdClass Object
(
    [id] => 750
    [menutype] => mainmenu
    [title] => Home Page
    [alias] => home-page
    [note] => 
    [route] => home-page
    [link] => index.php?option=com_flexicontent&view=category&cid=19
    [type] => component
    [level] => 1
    [language] => *
    [browserNav] => 0
    [access] => 1
...

Is compared the currently URL being built:

Array
(
    [option] => com_content
    [view] => category
    [id] => 240
    [lang] => en-GB
)

Re-opened as requested

Thanks @ggppdk detailed for debugging

Thanks

The current implementation of the router

• is only supposed to use menu items of the same component, right ?

Now about the fix , i think it is in this line:
https://github.com/joomla/joomla-cms/blob/staging/components/com_content/router.php#L55

Replacing:

if ($menuItemGiven && isset($menuItem) && $menuItem->component != 'com_content')

with

if ( !empty($menuItem) && $menuItem->component != 'com_content' )

i am guessing because the isset($menuItem) was not enough, someone had added:
$menuItemGiven && isset($menuItem)

• but we can use !empty($menuItem) to be sure that

$menuItem = $this->menu->getActive();
...
$menuItem = $this->menu->getItem($query['Itemid']);

have returned a menu item

No, my suggestion is incomplete,

i will post back a proper fix little later, when i will have time to look at it

Proper fix is to replace lines:
https://github.com/joomla/joomla-cms/blob/staging/components/com_content/router.php#L55-L59

with:

// Check again
if ( !empty($menuItem) && $menuItem->component != 'com_content')
{
    // Clear active menu item, because later code uses it without checking: $menuItemGiven 
    $menuItem = null;
    $menuItemGiven = false;
    unset($query['Itemid']);
}

@piotr-cz Could you update this PR with the provided code changes by @ggppdk? People can re-test the PR and we can move this forward. Thanks

This PR has received new commits.

CC: @dam-man, @in-code

This PR has received new commits.

CC: @dam-man, @in-code

@roland-d, I've merged latest staging branch, reverted my fix and applied one provided by @ggppdk.
I can't check if it solves the issue as I'm not really sure how to reproduce it.

Thanks for help here!

Still not clear how to reproduce this issue

@pete-rosseti
thanks for your interest, i described the circumstances above

but a quicker / simpler way to reproduce should be

1. Enable error reporting to maximum
2. Unpublish ALL com_content menu items
3. Make your home page be some other component but select a menu item that DOES NOT have ID variable, BUT IT has a view name either 'article' or 'category' so that you get the undefined variable notice,
4. Display a Joomla latest articles module in your home page (make sure you have some items in it)
5. [EDIT] Enable SEF urls and view home page

NOTE:

• if your 3rd party menu item has view 'category', then notice will be on category links
• if your 3rd party menu item has view 'article', then notice will be on article links
• the 3rd party menu item, must not have 'id' variable, or can even trigger notice with a 3rd party menu item that does not have 'view' URL variable at all

I have tested this code that i proposed, if you look at it, you see that

• the SEF URL building of any non-SEF URL that uses com_content menu item, is of course not effected
• and it makes sure that a current menu item, that is non-com_content, will not be used by later code

in particular this (later) code DOES NOT CHECK 'option'=='com_content':

        if (($menuItem instanceof stdClass)
            && $menuItem->query['view'] == $query['view']
            && isset($query['id'])
            && $menuItem->query['id'] == (int) $query['id'])

Because it assumes that menuitem was cleared if it was non-content !!, but current code is bogus and does not do it

Hello,

I can not reproduce this issue. I have followed instructions provided in above comment. I have tested this in Joomla! 3.5.1 clean installation and unpublished home menu (com_content default menu item) and assigned featured contacts as the homepage, then added latest articles module on home page. When I viewed the home page, I did not get any error message (error reporting was set to development and SEF was on).

Please see the screen-shot given below.

@AnishaVora

it is ok, others have seen this issue,

i have seen it, in several occasions, maybe i have missed a step above or maybe you have

here is another way to reproduce it:
#10721

out of curiosity do the created article SEF links have a menu item ? , can you copy paste one of them here ? thanks

@ggppdk

Articles SEF do have menu item link, it takes the menu item of the homepage (in my case it is featured contacts page). I have tested it in my local so I will not be able to paste the URL here.

@AnishaVora

I have tested it in my local so I will not be able to paste the URL here.

you can paste the URLs here,

because i am not interested to see the 'localhost' domain neither i want to click any link,

• i just want to see the structure of the created SEF URL

you can copy paste the part of the created article SEF URLs that is immediately after "localhost"

@ggppdk,

Here is the SEF URL : index.php/component/content/article/2-uncategorised/1-test-article1?Itemid=103

What is the type of menu item with id 103 ?

open it and paste its "link" here

As mentioned earlier, Its the home page menu item of type 'featured contacts'.

Is this php version specific ?

Am unable to reproduce on
PHP Version 5.5.28

I was referred over to the issue list from the CMS forum (google group) for this issue. I have been getting this error since upgrading to 3.6.0 on our live site.

The error in the error log is:
PHP Notice: Undefined index: view in /.../public_html/components/com_content/router.php on line 73

The error is being triggered by the following request
journeywithjesus.net/component/content/article?id=434comprehensive-index-of-book-reviews&Itemid=118

The menu 118 is an menu item alias pointing to the home page (screen shot below)

I would be willing to try temporarily installing the patched router files but I am not sure which ones to use (I don't usually use github) - please advise

Can you check if my PR #11343 fix the problem.

@rhellyer

Changes you can see at:
https://github.com/joomla/joomla-cms/pull/6488/files

Download file from https://raw.githubusercontent.com/piotr-cz/joomla-cms/64f5ac9ed1a1d98f0ab20246bf8a2b55b90bf526/components/com_content/router.php

and copy it to /libraries/cms/router/site.php

[UPDATED]
/components/com_content/router.php

Then test you request as you mentioned at #6488 (comment)
Check also whether urls generated on that page are correct.

I tried the pull request and it didn't work. Left comments here but can't find them so not sure if I clicked 'comment' correctly (new to this). I was able to replicate on local version of site so can reproduce if there are other changes needing testing. Please advise

@rhellyer Do you replace the correct file? I have created a mistake above? The file which should be replaced is /components/com_content/router.php

OK. That appears to work OK with my test case.

I reverted the libraries/cms/router/site.php to its original version
I changed /components/com_content/router.php to the new version

When I went to the URL that had previously created the error condition and it displays without errors now

I won't have time to install this change on the live site until tomorrow.
I noticed that the libraries/cms.../site.php was also changed in your pull request that you originally referred to .. do I need to install that new version also (it appeared to have two areas that were changed). From my simple test it seems that the change to the com_content/router.php file fixed the immediate issue.

I noticed that the libraries/cms.../site.php was also changed in your pull request that you originally referred to .. do I need to install that new version also

No, my PR is for some other case and may be change later.

@csthomas I've replaced the router.php on our live site and it works fine without throwing the 'undefined index' error -- many thanks!

I am closing this in favor of #6488 as that is now the active pull request. Thank you for your contribution.

Umm @roland-d this is #6488 :P

@wilsonge Correct, it is supposed to be #11343

@roland-d

• #11343 is not related to this PR, this PR fixes a different issue,
• furthermore the fix in this PR is proper

comment about being being related, was later corrected to say it is not related

This PR prevents com_content router,
from trying to compare query variables to the query variables of menu item that is of a different component

Re-opened as this is not related to the other PR. Thanks for the heads-up.

I have tested this item 🔴 unsuccessfully on 64f5ac9

Tried multiple approaches before adding the patch to trigger the warning. Also added the Home Alias Menu Item. Dont get this error.

@icampus

@wmchris

ok now i am confused
if you don't get the error then how have you tested?

in the end, don't take my word on it

you can just read the source code

Also added the Home Alias Menu Item. Dont get this error.

• There should be Notice. You have to set error_reporting to show notice logs.
• Menu item should not be related to com_content, use menu item to com_contact for example.

@wmchris

thanks for spending time to test,

just also marking an unsuccessful test
because you did not succeed to trigger the bug, it was akward to me

sorry we're currently working on joomla from the web development weeks. I didnt know there was a button for "could not reproduce"

i already set error_reporting to all and display errors to on. Original error didnt appear in unmodified joomla staging.

to be honest,
I would rather see an unsuccessful test than see no tests happening

I have not tested this item.

didn't find the arror message @icampus

Thanks for helping (taking over this issue) @ggppdk

Please everybody see the picture i post (forgive my spelling mistakes , i am in a hurry)
Also i have updated my instructions above to include missing instruction, that the active menu item must have a view either 'category' or 'article' but does not have 'id' variable !

@schmidtpaddy @wmchris Please test again with the new instructions. Thanks.

I have tested this item ✅ successfully on 64f5ac9

@roland-d wrote a component to test this issue. We could reproduce the issue and applying the patch fixed the issue.

I have tested this item ✅ successfully on 64f5ac9

Tested with the component from @roland-d . Successful tested!

@piotr-cz
This needs to be rebased ?

@ggppdk Looks so. But against which branch?

@piotr-cz staging

I've tried to rebase but the router file has been rewritten from ground up and it's not possible to resolve conflicts.

We need a confirmation that the issue is still present on latest staging branch and if so, probably address it other way

@piotr-cz We did see the issue on the current staging at the PBF last week.

Legacy routing uses exactly the same code. The file just got moved to https://github.com/joomla/joomla-cms/blob/staging/components/com_content/helpers/route.php so if this is an issue then it just needs the same PR but at the new file

Actually the file has been moved here: https://github.com/joomla/joomla-cms/blob/staging/components/com_content/helpers/legacyrouter.php

I've done rebase, which was effectively a reset to latest commit of staging branch and readded original commit changes to legacyrouter.php.

But the router.php hasn't just moved to legacyrouter.php, there are quite few changes and I think we need at least one more test.

This PR is very unclear when I look at files changes or commits.
Maybe removing last merging commit and clean merge can help.

upstream is a repo from joomla/joomla-cms

git reset --hard HEAD~1
git merge upstream/staging
git push -f

This PR is completely broken and considering that the routers are basically rewritten in 3.7, I'm asking for this PR to be closed.

It wasn't broken back in 2015 when I created it.

I understand and it is not good that this PR rotted around like this.

The bug is still here, see:
#15823

com_content legacy router still is examining menu items that belong to other components, without caring if they have the id URL variable

Yes, the bug is still present as J 3.7 is still using exactly same router.