Support » Plugin: Max Mega Menu » Max Mega Menu + MultiLingual-Press = PHP Fatal Error

  • Resolved Tim Reeves

    (@tim-reeves)


    I was getting errors like this:

    Got error 'PHP message: PHP Fatal error:  Uncaught TypeError:
    Argument 1 passed to Mlp_Nav_Menu_Frontend::maybe_delete_obsolete_item() must be an instance of WP_Post, instance of stdClass given,
    called in /var/www/vhosts/ayurveda.report/httpdocs/wp-content/plugins/multilingual-press/inc/nav-menu/Mlp_Nav_Menu_Frontend.php on line 53 and
    defined in /var/www/vhosts/ayurveda.report/httpdocs/wp-content/plugins/multilingual-press/inc/nav-menu/Mlp_Nav_Menu_Frontend.php:74
    Stack trace:
    #0 /var/www/vhosts/ayurveda.report/httpdocs/wp-content/plugins/multilingual-press/inc/nav-menu/Mlp_Nav_Menu_Frontend.php(53): Mlp_Nav_Menu_Frontend->maybe_delete_obsolete_item(Object(stdClass))
    #1 /var/www/vhosts/ayurveda.report/httpdocs/wp-includes/class-wp-hook.php(300): Mlp_Nav_Menu_Frontend->filter_items(Array)
    #2 /var/www/vhosts/ayurveda.report/httpdocs/wp-includes/plugin.php(203): WP_Hook->apply_filters(Array, Array)
    #3 /var/www/vhosts/ayurveda.report/httpdocs/wp-includes/nav-menu-template.php(189): apply_filters('wp_nav_menu_obj...', A...

    If I deactivate either of the plugins, the site works again.

    I’ve looked at the code: MultiLingual-Press does Type Hinting on its function arguments, in particular where a Post is expected the hint is “WP_Post”.
    That normally works, and is no doubt good practice… BUT will fail if any other plugin has created or recast post objects without explicitly making them of type WP_Post. Sadly, it seems that Max Mega Menu (and its clone AP Mega Menu) are creating navigation objects without specifying their type, so they default to StdClass. Then, when they get “walked” by some function in MultiLingual-Press, we end up with the type mismatch fatal error.

    I also checked out how WordPress itself handles this topic: The expected object function argument type WP_Post is often documented in comment, but never type hinted. So WordPress itself always carefully makes menu items which are of type WP_Post, but never, so to speak, forces the issue. You’ll find code like this: $_post = new WP_Post( $post );

    References to consult:

    https://codex.wordpress.org/Plugin_API/Filter_Reference/nav_menu_link_attributes => $item – Object containing item details. E.G: If the link is to a page $item will be a WP_Post object

    https://stackoverflow.com/questions/7839059/type-hinting-for-any-object

    http://php.net/manual/en/functions.arguments.php

    Now the really interesting question: Should MultiLingual-Press be more pragmatic and less purist, and remove the type hints to match WordPress itself?
    Or should Max Mega Menu be improved to ensure that all the menu item objects it modifies or creates are of type WP_Post?

    My personal preference would be for Max Mega Menu to ensure the correct type is set, but I don’t really mind as long as a solution pops up soon… because I’ve had to shift to “Mega Main Menu”, but in general definitely prefer the way Max Mega Menu works.

    This forum post is going in identical wording on the support forums of both plugins. Hope this analysis helps!

    Tim

Viewing 4 replies - 1 through 4 (of 4 total)
  • Plugin Author megamenu

    (@megamenu)

    Hi Tim,

    Thanks for the analysis, very helpful 🙂

    MMM adds the widgets as standard objects to the menu items. Those items aren’t strictly WP_Post objects, they lack post_excerpt, post_modified etc.

    It’s never likely that you’d use MultiLingualPress and WPML together, but WPML is another plugin I can think of that also dynamically adds non-WP_Post objects to the menu items (it does this to dynamically add language switcher items to the menu). I think PolyLang is another, and there will definitely be other plugins and themes (and lots of custom code snippets out there) that do the same thing (to add dynamic cart items, login, logout links etc).

    To me it doesn’t feel ‘right’ to me to cast non WP_Post objects as WP_Post objects, it seems like that could cause problems down the line.

    (A fee minutes later..) In fact, doing that could well break the MultiLingualPress code. For example, in the “maybe_delete_obsolete_item” function it looks like it could end up calling wp_delete_post on a non-WP_Post item.

    For compatibility with MMM and a handful of other plugins and themes, I think it would be best for MultiLingualPress to remove the type hinting, but check the object “is_a” WP_Post inside the function.

    Alternatively, it may be possible to simply adjust the priority of the MultiLingualPress wp_nav_menu_objects filter so that it runs before MMM has added the widgets to the menu. I could also maybe do that from my end*, and if you’d like to test it you could try changing the priority of this filter in megamenu.php to “11”:

    add_filter( 'wp_nav_menu_objects', array( $this, 'add_widgets_to_menu' ), 10, 2 );

    (*that won’t necessarily fix potential similar conflicts with MultiLingualPress and other plugins/themes, though).

    Happy to hear your thoughts 🙂

    Regards,
    Tom

    • This reply was modified 2 years, 2 months ago by megamenu.
    • This reply was modified 2 years, 2 months ago by megamenu.
    • This reply was modified 2 years, 2 months ago by megamenu.
    • This reply was modified 2 years, 2 months ago by megamenu.

    Dear Tom,

    thanks so much for that in-depth analysis. Now I think we all understand pretty clearly what is going on.

    Bumping up the priority of your filter add_widgets_to_menu to 11 does indeed fix my problem! Will you consider incorporating that into the official version? Otherwise I would have to re-fix every time the plugin gets an update…

    I did fear that the whole situation may be more complex than I had groked, and it seems that it is. The question for me now is, will Thorsten (@tfrommen) and the guys at Inpsyde GmbH say “OK” to your suggestion, or will they say “er nope, non WP_Post objects should not be in a list of menu items”. We’ll see.

    Thanks so much for your prompt and friendly help, and a great plugin!

    Tim

    Hi Tim and Tom! 😀

    As I already replied in the according topic in the MultilingualPress forum, we added the type hint for good reasons, and I don’t see us removing it.

    It is true that WordPress does not make use of type hints (in this context), but it is also true that all documentation of nav menu items specifies the type WP_Post. This is true for the filter that we are talking about, and, of course, also for things like nav menu walker implementations.
    Due to a lack of type hinting/checking, you can pass a stdClass – but this might (and eventually will) fail with any code trusting what the documentation says. If someone tries to access a property that a WP_Post instance has, but that is missing on your stdClass, you end up with a PHP Notice, and maybe unexpected behavior. If you try to access WP_Post methods, for example to_array(), you will end up running into a Fatal Error.

    Unless WordPress tells differently, a nav menu item should be, and is thus expected to be, a WP_Post instance.

    Cheers,
    Thorsten

    PS: regarding the possibility of MultilingualPress deleting a post by accident, this shouldn’t happen. If the item, which has to be of our Language type or a custom link, does not have a remote site ID defined as post meta, the code will not run.

    Plugin Author megamenu

    (@megamenu)

    Hi Thorsten,

    I understand what you’re saying 100%. Until widgets are stored as posts by WordPress, it doesn’t make sense to me to cast those objects as posts – you’ve pointed out the pitfalls in doing this.

    If you’re not happy with removing the type hinting and do your checking inside the function, could you consider updating the priority of your filter to 9? Your code will still be “correct” and also fix the Fatal Error. I’m mainly suggesting you do this from your end because it will not only fix the compatibility issue with MMM, but also the many themes and plugins (and custom code snippets out there) that add non-WP_Post objects to the menu.

    Regards,
    Tom

    • This reply was modified 2 years, 2 months ago by megamenu.
    • This reply was modified 2 years, 2 months ago by megamenu.
Viewing 4 replies - 1 through 4 (of 4 total)
  • The topic ‘Max Mega Menu + MultiLingual-Press = PHP Fatal Error’ is closed to new replies.