KnpMenu icon indicating copy to clipboard operation
KnpMenu copied to clipboard

Twig Template fails when a child name matches a item's method name

Open oparadis opened this issue 9 years ago • 2 comments

When using the TwigRenderer, the current implementation of "knp_menu.html.twig" accesses to the methods of an MenuItem by using a property path syntax.

{{ item.label }} accesses the $item->getLabel() method.

Unfortunately, since ItemInterface extends \ArrayAccess, it is possible that a child name conflicts with a method name. Since \ArrayAccess has priority, it'll not call the intended methods.

For example, by adding a menu such as $menu->addChild('label', []);, in the twig template, {{ item.label }} will access $item['label'] instead. It will return an ItemInterface object and fail with a "object cannot be converted to a string" error.

By adding a 'children' item, you could skip part of a menu because it uses the child's children instead of the item's children.

I guess that it should be safer to use method names in the twig template,such as {{ item.getLabel() }}, to ensure that the methods are called and not children items.

oparadis avatar Jan 20 '16 15:01 oparadis

@oparadis I would suggest creating a Pull Request so the core team is presented with the possibility to merge and a discussion can form around the idea. I do think these are more edge case scenarios, however I do agree that removing any possible obscure hurdles for beginners is nice.

jonny827 avatar May 19 '16 08:05 jonny827

@oparadis i think you are right. i am not entirely sure whether there could be some BC risk when people pass an array to a twig template instead of an Item object - but imho a new minor version is enough for this as its really an edge case and not the expected use of this library.

do you want to do a pull request to change the places where we use the PropertyAccess instead of calling methods?

dbu avatar Jun 21 '16 14:06 dbu