Joomla! Issue Tracker | Joomla! CMS #31100 - [4.0] Active menu does not exist, fatal error

? ? ?

Closed
16 Oct 2020
Medium
Build: 4.0-dev
# 31100
Diff
sakiss:active_menu_null

Pending

User tests: Successful: Unsuccessful:

sakiss
15 Oct 2020

Pull Request for Issue # .
#30812

Summary of Changes

$app->getMenu()->getActive();
can return a MenuItem or null

by calling a function when null is returned, it generates a fatal error.

Testing Instructions

Add a dummy/non existent ItemId in your non sef urls.
E.g. /index.php?option=com_content&view=article&id=2:another-article&catid=9&itemId=5000
In that case there is no menu item with id 5000

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

No fatal error

Documentation Changes Required

sakiss - open - 15 Oct 2020

sakiss - change - 15 Oct 2020

Status

New

⇒

Pending

joomla-cms-bot - change - 15 Oct 2020

Category

⇒

Front End Templates (site)

SharkyKZ - comment - 15 Oct 2020

Can you do it the same as in error.php?

joomla-cms/templates/cassiopeia/error.php

Lines 28 to 29 in f4b2644

    
           $menu     = $app->getMenu()->getActive(); 
        
           $pageclass = $menu !== null ? $menu->getParams()->get('pageclass_sfx', '') : '';

Otherwise $pageclass is undefined when no menu item.

sakiss - change - 15 Oct 2020

Labels

Added: ?

richard67 - comment - 15 Oct 2020

@SharkyKZ This PR here changes error.php, too, and I don't understand why. It was ok like it was before, or not? Am I missing something?

richard67 - comment - 15 Oct 2020

@sakiss Please revert your change in error.php. It was good like it was before, using the ternary expression, and you now introduced an error with undefined $pageclass when there is no menu item. And as @SharkyKZ suggested please change index.php so it uses the same ternary expression as error.php does without your change.

richard67 - comment - 15 Oct 2020

@sakiss And in addition to the above comment, please update the branch of this PR to the latest 4.0-dev branch of the CMS repository, not of your fork. It seems your 4.0-dev branch was very outdated when you have created this PR and the other one, #31099 based on it, because both PRs fail in our automated test due to outdated testing configuration from before February.

Let me know if you need help with.

Thanks in advance.

richard67 - comment - 16 Oct 2020

Added the "Updates Requested" label so it's clear for testers that this PR needs changes as specified in review comments above.

7cc2d74 16 Oct 2020