Bug #77296

TMENU - get the parentMenuArr in itemArrayProcFunc - broken

Added by Stefan Bürk about 3 years ago. Updated 12 months ago.

Status:
Closed
Priority:
Should have
Assignee:
Category:
-
Target version:
Start date:
2016-07-28
Due date:
% Done:

100%

TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:
On Location Sprint

Description

Hi there,

after upgrading from 6.2.25 to 7.6.9/7.6.10 our TMENU itemArrayProcFunc stopped working probably.
Not the function itself, but in it we used the parentMenuArr from the parentObj to determine the ITEM_STATE of the parent menu item, which this TMENU and items belongs to.

Given following abstract Menu TS CONF


HMENU {

  1 = TMENU
  1 {}

  2 = TMENU
  2 {}

  3 = TMENU
  3 = {
     itemArrayProcFunc = myUserFunc
  }
}

For the 3rd level, we must do things based on the ITEM_STATE of the parentMenuItem from Level 2.
Not even that, also clearing the menuArr under some cirtumstances (flags in extension based fields) if parent state have specified flags etc.

To visualize this a little bit:

1.1                      
  1.1.2.1
  1.1.2.2
    1.1.2.2.3.1       NO
    1.1.2.2.3.2       NO
  1.1.2.3             
    1.1.2.3.3.1       IF(1.1.2.3 ITEMSTATE === ACTIVE/CURRENT) THEN USERDEF1, OTHERWISE USERDEF2    
    1.1.2.3.3.1       IF(1.1.2.3 ITEMSTATE === ACTIVE/CURRENT) THEN USERDEF1, OTHERWISE USERDEF2

Okay, if you have to just render all Level2/3 Menu's in the same way for each parentLeven, than no user func is needed really ..
but if you want to render items for level based on the direct parent menu item item state, what than ?

I found no other solutions for that, so reimplementing the parentMenuArr behaviour was the only situation for our case ... are we so special ?

In the past (Typo 6.2.25) we had the following pseudo code in the userFunc to get the ITEM_STATE of the parent menuItem.

function($menuArr, $conf) {

  $parentItem = NULL;
  if (is_array($conf['parentObj']->parentMenuArr) && count($conf['parentObj']->parentMenuArr) > 0) {
    foreach($conf['parentObj']->parentMenuArr AS $menuItem) {
      if ($menuItem['uid'] === $conf['parentObj']->id) {
        $parentItem = $menuItem;
        break; // end the loop, we have what we wanted.
      }
    }
  }

  if (is_array($parentItem) && isset($parentItem['ITEM_STATE'])) {

    // loop through menuArr and do some special settings to the ITEM_STATE's and override virtually the titles

  }
}

After debugging a lot, i'm ended up looking into the typo core and the functions for the menu rendering stuff.

See https://review.typo3.org/#/c/36584/5/typo3/sysext/frontend/Classes/ContentObject/Menu/AbstractMenuContentObject.php LINE 458-460 in the old code.

In Typo 7.6.9 source and in current master, there was a lot changed in that class.
I think that 3 liner was dropped, because that property was not defined and even more, not returning an array every time. Maybe it was also removed, because the core itselfs do not need this -- but for userland functions this could be a usefull information.

I'm calling this a bug .. because removing this "public" property was not mentioned anywhere and broke's userland code.

Instead of simply readding the code, it should be done in proper way:

- not providing an public accessible property
- using setter/getter and returning always an array, an empty one if no parent level exists ( menu level 1 ).
- adding a function to just get the direct menuItem to prevent the need to loop through the parentMenuArray for each sublevel itemArrayProcFunc call, to optimize performance

Any suggestions welcome ;) I'm going to provide an patch for this bug, with the stated "improvements" instead of simply readding this.

tmenu_test_0.0.1.zip - Testextension for this issue (7.19 KB) Stefan Bürk, 2016-08-03 21:31

Associated revisions

Revision 5b1dcea4 (diff)
Added by Stefan Bürk almost 3 years ago

[BUGFIX] Adding back access to parentMenuArr and menuitem in subMenu

Adding the possibility to access the menuArr and direct menuItem from
the parent menu in subMenu instances to provide the ability to check
for parent item information or parent sibling item informations.

Providing two getters and a combined setter, called in the subMenu
method to readd this data access instead of simple using the old
unsafe and undeclared property set.

Leaving the parentMenuArr member public for now, even it is not
nice.

Resolves: #77296
Releases: master, 7.6
Change-Id: I3cfce3c67e840e6e97fa9c55ffaf997a509ee0e8
Reviewed-on: https://review.typo3.org/49260
Reviewed-by: Anja Leichsenring <>
Tested-by: Anja Leichsenring <>
Tested-by: TYPO3com <>
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>

Revision 9167a9ea (diff)
Added by Stefan Bürk almost 3 years ago

[BUGFIX] Adding back access to parentMenuArr and menuitem in subMenu

Adding the possibility to access the menuArr and direct menuItem from
the parent menu in subMenu instances to provide the ability to check
for parent item information or parent sibling item informations.

Providing two getters and a combined setter, called in the subMenu
method to readd this data access instead of simple using the old
unsafe and undeclared property set.

Leaving the parentMenuArr member public for now, even it is not
nice.

Resolves: #77296
Releases: master, 7.6
Change-Id: I3cfce3c67e840e6e97fa9c55ffaf997a509ee0e8
Reviewed-on: https://review.typo3.org/50376
Tested-by: TYPO3com <>
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>

History

#1 Updated by Gerrit Code Review about 3 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#2 Updated by Gerrit Code Review about 3 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#3 Updated by Gerrit Code Review about 3 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#4 Updated by Gerrit Code Review about 3 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#5 Updated by Stefan Bürk about 3 years ago

  • Sprint Focus set to On Location Sprint

#6 Updated by Stefan Bürk about 3 years ago

Added an testextension for reproducing the issue and for patch review.

The testextions works only for Typo 7.6 up to current master.

Provides three static templates with menus and itemArrayProcFunc functions:

- Testmenu - TopLevelCheck
- Testmenu - subMenu working in 6.2, broken in 7.6
- Testmenu - subMenu new getter

Template 1 and 2 works with and without the patch ( noting whenn the broken parentMenuArr is detected ),
with human readable messages.

The last one is to test the new getters provided by the patch.

The second one, subMenu working in 6.2, broken in 7.6 simulates
the real workcase we had, and what was broken in 7.6.

Theres a README.md in the extension.

Requirement for testing this is at least a two level pagetree with some pages,
Introduction/Bootstrap distribution works fine for it.

Just select the static template in the pagetemplate. Make sure cache are cleared
and page is not cached for testing purpose.

Hope the test extension helps.

Netherless, besides the issue, createing the testextension some caveats came in mind again.

In our workcase we are setting the ITEM_STATE of the parent/topLevel menu through the itemArrayProcFunc,
to rely on it in the second one. That means, all stats, not just the USERDEF ones.

This is done, because the ITEM_STATS are prozessed, after the subMenu's are prozessed.

Means:


Level1 - fetch pages
Level1 - itemArrayProcFunc
Level1 - iterate items
    Level2 - if Level1 has subItems, and a SubMenu is defined, 
             create the subMenu and fetch the submenu pages
    Level2 - itemArrayProcFunc
Level1 - generte/makeMenu ( prozess item states of the level 1 items ).

So, could be thought of if there could be a "prepared itemState" in the menuArr, before
handing it over as parentMenuArr to the submenu .. would be a bigger patch.

Maybe there are reasons why the processItemStats() is called so late after the the subMenus.

But this should be another discussion.

Note: The three static templates overrides page.5 to define the testmenu.

#7 Updated by Gerrit Code Review about 3 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#8 Updated by Gerrit Code Review about 3 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#9 Updated by Gerrit Code Review about 3 years ago

Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#10 Updated by Gerrit Code Review almost 3 years ago

Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#11 Updated by Gerrit Code Review almost 3 years ago

Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#12 Updated by Gerrit Code Review almost 3 years ago

Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/49260

#13 Updated by Gerrit Code Review almost 3 years ago

Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50376

#14 Updated by Gerrit Code Review almost 3 years ago

Patch set 2 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/50376

#15 Updated by Stefan Bürk almost 3 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#16 Updated by Benni Mack 12 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF