Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.#484060: node_add needs a sanity check successfully removed the extra access check. But, as agentrickard pointed out in http://drupal.org/node/484060#comment-1675134, the function is still overly complex and, hence, confusing.
- The docblock is wrong. The sole purpose of the function is to return a node submission form.
- There's no need to check if the content type exists. node_menu() already does that.
- At the very least, $output should be defined before being returned. Although
isset($types[$type]will always evaluate to TRUE and thus $output will always be set. (But that of course just proves that you don't need the if() at all.)
I'm happy to roll a patch if this is a valid (albeit minor) bug.
Comments
Comment #1
CrookedNumber CreditAttribution: CrookedNumber commentedComment #2
aspilicious CreditAttribution: aspilicious commentedReturn => Returns
Comment #3
CrookedNumber CreditAttribution: CrookedNumber commentedThanks. Re-roll attached.
Comment #4
AaronBaumanWe can simplify this further:
If the sole purpose of node_add is to return the node form, it shouldn't set the page title either.
This also alleviates the side effect of developers having to reset the page title whenever they call node_add to get the node form.
Is there a historical reason that node_menu's node/add/* doesn't just set the final title?
Here is a re-roll with the suggestion.
Comment #6
AaronBaumanThis patch had the side effect of renaming links on "node/add" from "@type" to "Create @type", since node_add_page uses
system_admin_menu_blockto display its content.revised patch:
drupal_set_titlefromnode_add.node_add_page_title, which returns the translated "Create @name" titletheme_node_add_listto use the menu item's $item['link_title'] (the human readable type name) instead of $item['title'] (the translated menu title "Create @name") in order to address the side effect mentioned at the beginning of this comment.Comment #8
AaronBaumanLooks like the reason for using drupal_set_title in node_add, rather than declaring it in the menu callback, is to make the menus cleaner all around. Please ignore comments #4 - #7 and see patch #3, which works for me.
sorry for the WOB.
Comment #9
CrookedNumber CreditAttribution: CrookedNumber commentedNo problem. I was wondering about that, too. Now I learned something.
Re-rolling patch from #3 (vs latest HEAD), so folks scanning this list see green at the bottom.
Comment #10
betz CreditAttribution: betz commentedPatch tested and works.
node/add/nonexistingtypegives me thenode/addpage, which is a good thing ^^Comment #11
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.