? ? Pending

User tests: Successful: Unsuccessful:

avatar SharkyKZ
SharkyKZ
10 Oct 2019

Pull Request for Issue # .

Summary of Changes

Removes deprecated code from Joomla\CMS\Menu\MenuItem.

Testing Instructions

Install sample data or create some menu items.
Navigate frontend and backend.
Inspect menu items.

Expected result

Works like before.

Documentation Changes Required

Some methods removed.
Added Joomla\CMS\Menu\AdministratorMenuItem class.

avatar SharkyKZ SharkyKZ - open - 10 Oct 2019
avatar SharkyKZ SharkyKZ - change - 10 Oct 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 10 Oct 2019
Category Administration com_menus Modules Front End com_contact com_content com_newsfeeds com_tags Libraries Plugins Templates (site)
avatar SharkyKZ SharkyKZ - change - 10 Oct 2019
Labels Added: ?
avatar amit4106udale amit4106udale - test_item - 19 Oct 2019 - Tested unsuccessfully
avatar amit4106udale
amit4106udale - comment - 19 Oct 2019

I have tested this item ? unsuccessfully on 9137ad0

Error: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params: Cannot access protected property Joomla\CMS\Menu\MenuI


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

avatar SharkyKZ
SharkyKZ - comment - 19 Oct 2019

On which page are you getting this?

avatar amit4106udale
amit4106udale - comment - 19 Oct 2019

On which page are you getting this?

I was testing this on cloudaccess. And applied the patch. Once applied site became black.

avatar SharkyKZ
SharkyKZ - comment - 19 Oct 2019

Do you have any 3rd party extensions installed? Maybe using Cloudbase template?

avatar amit4106udale
amit4106udale - comment - 19 Oct 2019

Do you have any 3rd party extensions installed? Maybe using Cloudbase template?

No pure joomla 4 nightly build

avatar joomla-cms-bot joomla-cms-bot - change - 22 Oct 2019
Category Administration com_menus Modules Front End com_contact com_content com_newsfeeds com_tags Libraries Plugins Templates (site) Administration com_menus Modules Front End com_contact com_content com_newsfeeds com_tags com_users Libraries Plugins Templates (site)
avatar SharkyKZ SharkyKZ - change - 22 Oct 2019
Title
[4.0] Remove deprecated code in Joomla\CMS\Menu\MenuItem
[4.0] [On Hold] Remove deprecated code in Joomla\CMS\Menu\MenuItem
avatar SharkyKZ SharkyKZ - edited - 22 Oct 2019
avatar SharkyKZ
SharkyKZ - comment - 22 Oct 2019

Should work now but don't test yet. Performance could be bad at the moment.

avatar SharkyKZ SharkyKZ - change - 26 Oct 2019
Title
[4.0] [On Hold] Remove deprecated code in Joomla\CMS\Menu\MenuItem
[4.0] Remove deprecated code in Joomla\CMS\Menu\MenuItem
avatar SharkyKZ SharkyKZ - edited - 26 Oct 2019
avatar SharkyKZ
SharkyKZ - comment - 26 Oct 2019

OK, this can be tested now.

4ededb2 11 Nov 2019 avatar SharkyKZ Typo
avatar Quy
Quy - comment - 11 Nov 2019

Go to Components Dashboard.

Notice: Undefined property: Joomla\CMS\Menu\MenuItem::$permission in \administrator\modules\mod_submenu\Menu\Menu.php on line 56

Notice: Undefined property: Joomla\CMS\Menu\MenuItem::$icon in \administrator\modules\mod_submenu\tmpl\default.php on line 22

avatar Quy
Quy - comment - 12 Nov 2019

Go to Components Dashboard:

Notice: Undefined property: Joomla\CMS\Menu\MenuItem::$icon in \administrator\modules\mod_submenu\tmpl\default.php on line 22

avatar Hackwar
Hackwar - comment - 14 Nov 2019

I like these changes. However, the change from browserNav to target is a B/C break and needs to be properly documented.

avatar SharkyKZ
SharkyKZ - comment - 14 Nov 2019

No, it's not.

avatar SharkyKZ
SharkyKZ - comment - 14 Nov 2019

Though maybe there's a better way to do this. browserNav is a numeric value from database. In some places in backend it's overwritten with the string used in markup. target is only used in backend and it also holds same string. Do we need both of these properties? Maybe remove target and use only browserNav?

In frontend it's much simpler. Only browserNav is used and it does not get overwritten.

avatar isidrobaq isidrobaq - test_item - 15 Nov 2019 - Tested successfully
avatar isidrobaq
isidrobaq - comment - 15 Nov 2019

I have tested this item successfully on 0460dd6


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

avatar cheiff cheiff - test_item - 15 Nov 2019 - Tested successfully
avatar cheiff
cheiff - comment - 15 Nov 2019

I have tested this item successfully on 0460dd6


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

avatar alikon alikon - change - 15 Nov 2019
Status Pending Ready to Commit
avatar alikon
alikon - comment - 15 Nov 2019

RTC


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

avatar SharkyKZ SharkyKZ - change - 21 Nov 2019
Labels Added: ?
avatar wilsonge
wilsonge - comment - 24 Nov 2019

Though maybe there's a better way to do this. browserNav is a numeric value from database. In some places in backend it's overwritten with the string used in markup. target is only used in backend and it also holds same string. Do we need both of these properties? Maybe remove target and use only browserNav?

If that works - and they are exactly the same thing (struggling to tell from a quick code review - I'd probably go down that route. Maybe revert the rename here https://github.com/joomla/joomla-cms/pull/26537/files#diff-1b5557519e9638a5bde91cf3432456dfR403 and then make that change in a separate PR?

avatar SharkyKZ
SharkyKZ - comment - 26 Nov 2019

I think it's best to subclass MenuItem for administrator items and add missing properties there.

avatar joomla-cms-bot joomla-cms-bot - change - 26 Nov 2019
Category Administration com_menus Modules Front End com_contact com_content com_newsfeeds com_tags Libraries Plugins Templates (site) com_users Administration com_menus Modules Front End com_contact com_content com_newsfeeds com_tags com_users Libraries Plugins
avatar SharkyKZ SharkyKZ - change - 26 Nov 2019
The description was changed
avatar SharkyKZ SharkyKZ - edited - 26 Nov 2019
avatar wilsonge wilsonge - change - 30 Nov 2019
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-11-30 21:38:07
Closed_By wilsonge
avatar wilsonge
wilsonge - comment - 30 Nov 2019

Thanks!

avatar xillibit
xillibit - comment - 2 Dec 2019

I have this kind of call in my component :
$params = Factory::getApplication()->getMenu()->getActive()->params; with this change it display : Cannot access protected property Joomla\CMS\Menu\MenuItem::$params

Can-i call instead $params = Factory::getApplication()->getMenu()->getActive()->getParams(); ? getParams() isn't deprecated anymore ?

avatar SharkyKZ
SharkyKZ - comment - 2 Dec 2019

Yes, use getParams() to access params.

avatar bugsmafia
bugsmafia - comment - 16 Dec 2019

Error: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params

Are these changes due to a bug on the frontend?

avatar SharkyKZ
SharkyKZ - comment - 16 Dec 2019

@bugsmafia Are you using any 3rd party extensions?

avatar bugsmafia
bugsmafia - comment - 16 Dec 2019

@bugsmafia Are you using any 3rd party extensions?

no, clean joomla 4 dev.
Files were updated (developer build of December 16) (with full replacement) after 1 month of development (build of November 7, 2019).

avatar SharkyKZ
SharkyKZ - comment - 16 Dec 2019

On which pages does this error occur?

avatar bugsmafia
bugsmafia - comment - 16 Dec 2019

On which pages does this error occur?
Any articles, including the default Featured Articles page
(whether a problem occurs with other components is unknown)

avatar bugsmafia
bugsmafia - comment - 16 Dec 2019

/index.php?option=com_content&view=article&id=2&Itemid=103
Build on November 7th. on the links from the menu to any article gave an error 404.
after updating to the build on December 16th.
"Error: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params: Cannot access protected property Joomla\CMS\Menu\MenuItem::$params"

avatar SharkyKZ
SharkyKZ - comment - 16 Dec 2019

I can't replicate this issue on current nightly (https://developer.joomla.org/nightly-builds.html) and on my git clone.

If you upload a zip of your installation (only files, no database), I could take a look.

avatar bugsmafia
bugsmafia - comment - 20 Dec 2019

I can't replicate this issue on current nightly (https://developer.joomla.org/nightly-builds.html) and on my git clone.

If you upload a zip of your installation (only files, no database), I could take a look.

write me bugsmafia@gmai.com or Telegram @bugsm

yep, update only file (full pack),
update structure DB does not fix the error

avatar bugsmafia
bugsmafia - comment - 20 Dec 2019

I Found the source of the problem in my template.
$pageclass = $menu->params->get('pageclass_sfx');
or
$pageclass = $menu->getParams()->get('pageclass_sfx');

Add a Comment

Login with GitHub to post a comment