If you go to admin/build/menu/item/edit for example, you'll see a double 404 message -- one with a blank page and one with blocks in it. This is probably due to the fact that this form is now being pulled by drupal_get_form as the menu callback.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

coreb’s picture

Version: x.y.z » 6.x-dev

moving from x.y.z to 6.x-dev version queue

RobRoy’s picture

Version: 6.x-dev » 5.x-dev

Actually, this one is a bug so should be filed against 5.x.

kkaefer’s picture

Status: Active » Needs review
FileSize
629 bytes

Fix for this issue. However, this needs some more general fixing, like the return value of forms being a constant like MENU_NOT_FOUND so that the global switch in index.php can decide.

Dries’s picture

Status: Needs review » Needs work

That's a bit ugly, IMO.

kkaefer’s picture

Status: Needs work » Postponed (maintainer needs more info)

I agree that this is not the best solution. However, I have no better option/solution to offer.

RobRoy’s picture

Status: Postponed (maintainer needs more info) » Active

Should just be set to 'active' since we have all the info we need to reproduce the bug, we just need to find a good solution.

chx’s picture

Status: Active » Needs work

then this is code needs work -- and I have no really good idea. But I will brainstorm with others...

RobRoy’s picture

Ah, oops.

So in my mind there are two ways to go about this and it depends on this question: Do we even want form callbacks to be able to call drupal_not_found/access_denied? Or, should we pull that code into another wrapper function? A good example is in menu.module where there are 4 drupal_not_found()s in form callbacks.

Option 1 (which is cleaner, but may require too much core refactoring for this late stage): Fix drupal_get_form(), drupal_retrieve_form(), and the menu handler system to allow us to return not found/access denied from forms and set a not found/denied status which actually persists back to index.php, respectively.

Option 2 (sloppier but less critical core changes maybe): Pull any code that calls drupal_not_found out into a wrapper function so form generation functions will ALWAYS return a form array.

Just wanted to get the ideas rolling, I'm sure someone else will come up with something better. :P

David Stosik’s picture

Version: 5.x-dev » 6.15
Status: Needs work » Active

Still getting this issue, on Drupal 6.15.
My configuration is quite simple : I have a menu item which page callback is 'drupal_get_form', and page arguments are 'my_form' and 4 (so that it takes the fifth element in the URL as an argument).
in my_form(), if the second argument (an id) is not found, I want to output a 404, so I call drupal_not_found().

To avoid the whole HTML to be output twice, I need to exit(); just after drupal_not_found().

inforeto’s picture

Found that exit(); worked to avoid a nested double error page.

A side issue is that if you are throwing a 404 programatically it just logs the page to watchdog.
You have to add your own watchdog message to add more detail on your custom 404 condition, invalid form options or attempts to hack or automate your form.

This leaves you with a bunch of useless 404 entries, so may be drupal_not_found() should take a string to pass to watchdog.
After all, your site may be collecting statistics on pages not found and having your form pages shown on it just clutter the list.

a.milkovsky’s picture

thanks for #9, You can use drupal_exit() instead of exit()

mdupont’s picture

Category: Bug report » Support request
Issue summary: View changes
Status: Active » Closed (works as designed)

Instead of calling drupal_not_found() in your menu callback, just "return MENU_NOT_FOUND;", which is the correct way to indicate a 404 error in a menu callback.