Hi everyone,

I am using Drupal 5.1. The left column vanishes when I get a 404 error. This happens with the standard Drupal-generated 404 page, as well as my own custom error page.

I experienced this problem with the Chameleon theme and a tweaked version of Bluemarine (see it @ http://mediameriquat.com/missing_left_column). No idea about other modules. I never had this kind of problem with previous versions of Drupal.

Maybe this is an attempt to keep the 404 pages as "light" as possible?

I would like the users to see all links in the left blocks, so they can access the contents or the site faster.

Comments

minimism’s picture

This is happening for me too.... presumably there's a template to be tweaked somewhere??

Drupal 5.1 here too

lozzagmk’s picture

I have this problem using Chameleon with 5.1, too.

I have a lot of problems with the theme, including working with images being buggy and inconsistant. I rekon a change to another theme that is better written might be in order...

arvinsingla’s picture

Ditto for me as well. No matter what theme I try (including Bluemarine or Garland) the sidebars go away on a 404. 403's however don't exhibit this and seem to work fine.

minimism’s picture

Is anyone likely to be looking at this?? I'm now seeing it on a different site (404 only, 'access denied' page is Ok). New site is using the foundation theme.

martig’s picture

This is not a bug. Its purpose is to keep the 404 pages light. There was a post in the forums on how to enable the blocks.

behindthepage’s picture

If you want to hack the core, all you need to do is change line 356 in common.inc (found in the includes folder)

From
  print theme('page', $return, FALSE);
To
  print theme('page', $return);

Easy!

cosmicdreams’s picture

Status: Active » Postponed (maintainer needs more info)

Interesting issue. How has this changed with the onset of Drupal 6?

cosmicdreams’s picture

Status: Postponed (maintainer needs more info) » Fixed
pasqualle’s picture

Status: Fixed » Active

This is not fixed.

There should be an option to preserve blocks for custom error pages.

pasqualle’s picture

Title: Left column disappears when 404 » Keep blocks on error pages
Category: bug » feature

Create an option to keep blocks on error pages.
Show the option on page admin/settings/error-reporting.

cosmicdreams’s picture

The Hack from #6 didn't work for you?

pasqualle’s picture

rule no 1: don't hack core

elv’s picture

The post in the forum with the solution martig wrote about is here: http://drupal.org/node/129762#comment-232868

pasqualle’s picture

Status: Active » Needs review
StatusFileSize
new2.12 KB

visibility of blocks can be set on page: admin/settings/error-reporting
the blocks are disabled at high server load

pasqualle’s picture

Project: » Drupal core
Version: » 6.x-dev
Component: usability » system.module
pasqualle’s picture

StatusFileSize
new2.16 KB

reroll

robloach’s picture

StatusFileSize
new2.11 KB

Since it's a boolean value, wouldn't it make more sense as a checkbox? The attached patch ends up looking like the following...

[X] Display Blocks
Show the blocks on error reporting pages.

pasqualle’s picture

Yes, I also like the checkbox better, but when I saw the admin/settings/performance page, I though the radiobuttons is an UI standard, or something..

tested, still working. RTBC

just let someone else to test it also

catch’s picture

The admin/navigation menu doesn't show for me, not sure if that's a bug or by design.

Doesn't this mean an extra database call with variable_get every time there's a 404?

pasqualle’s picture

admin/navigation menu only show up on admin pages, it does not show up on any other page.. I think it is a block setting or something..

Yes it is 1 or 2 extra database call, but the variables are cached somehow. Does not really know what happens exactly at variable_get function call.

catch’s picture

Navigation shows up on non-admin pages as well, admin is a sub-item.

If someone can explain how the variable cacheing works that'd be great - I have over 100,000 404s on my site a month so the less drupal has to do for each one the better (hence why removing the blocks in the first place was a good idea).

gábor hojtsy’s picture

On variable_init(), all the variables are cached to an array in memory. So a variable_get() just looks up from the array in memory.

catch’s picture

Gabor, thanks for that, makes sense, I know something changed with variables in D6 but couldn't remember what.

The only remaining issue seems to be that all menu blocks disappear on 404s regardless of this setting. I just tried with the primary links block and a custom menu - same behaviour.

catch’s picture

Status: Needs review » Needs work

forgot to change status.

pasqualle’s picture

StatusFileSize
new13.62 KB
new2.23 KB

@catch: you are right, thanks for the review

reroll

The problem with menu is that the function menu_get_item($path = NULL, $router_item = NULL) in menu.inc returns an empty array for non existent $path, so no menu is displayed.
I don't feel like I want to touch anything in that or related functions. But maybe someone will..

other, much more easier problem: The Display Blocks setting checkbox does not have a (section) title. see: picture

pancho’s picture

StatusFileSize
new2.46 KB
new30.55 KB

@Pasqualle: A section title is not really needed. What would it say if not something redundant? But I would move the new checkbox further up to be right after 403/404 page fields (which the new option belongs to) and before the "Error reporting" field.

Moved the checkbox up, so the UI should be okay now. Didn't solve the menu_get_item issue though. So we are only on half way.

chx’s picture

Status: Needs work » Closed (won't fix)

As the forum post says (simplified):

function YOURTHEME_page($content, $show_blocks = TRUE) {
  return phptemplate_page($content);
}

works fairly well, no need to patch core for this.

pasqualle’s picture

Status: Closed (won't fix) » Active

ok, it works in drupal 5 but does not work in drupal 6. phptemplate_page() was removed in d6.

I tried to make the something similar in d6

function garland_preprocess_page(&$variables) {
  $variables['show_blocks'] = TRUE;
}

but this does not work either, because garland_preprocess_page() in template.php is called after template_preprocess_page() in theme.inc.

I am not familiar with theme functions. Can someone help me with this problem?

robloach’s picture

Title: Keep blocks on error pages » Display blocks on error pages
Version: 6.x-dev » 7.x-dev
Status: Active » Needs work

The "Display Blocks" option is something that should be present in the error reporting administration interface, as requiring someone to write code for doing this can be a bit overkill. This also makes it impossible to show the blocks in Garland, Minnelli, or any other core theme without making an entirely new sub-theme, and then adding the code block.

Since this is a bit too late for Drupal 6, it's being moved to 7.x (feature freeze, string freeze).

robloach’s picture

Component: system.module » theme system

I couldn't find where to set the $show_blocks within the system before garland_preprocess_page(&$variables).

dvessel’s picture

This should really be handled through CSS by using the $body_classes and styling with that. I'd guess that 403/404 hit's to most sites is not insignificant and rendering the blocks could add up and get expensive.

It is possible force the block to show in Drupal 6, but it's more complicated. Another alternative for now is to use the side regions named other than 'left' or 'right' and they will always show.

As for providing a UI for this, I'd be okay with it as long as a description's provided on the potential performance hit.

drewish’s picture

subscribing... i'm totally in favor of making this an option that's off by default with a warning that it has performance implications.

catch’s picture

OK so there's two places it could go (I think). admin/build/block and admin/settings/performance. At the moment there's nothing in admin/settings/performance that's on by default - although I don't see any reason not to change that - seems more appropriate there alongside block caching, and can always link from the help text on block admin page.

mrfelton’s picture

subscribing - I'd also really like to know how to do this in template.php for drupal 6.x

tcconway’s picture

Subscribing as well...

I know this is not the best place to ask this, but I too would *LOVE* to know how to set this for Drupal 6.x.

dvessel: You mentioned you knew how. Can you point us in the right direction?
Thanks!

dvessel’s picture

I mentioned the workaround. Just name the side block regions to anything other than "left" or "right". Change it inside the .info file and the page.tpl.php file so they match the new name.

moritzz’s picture

Title: Display blocks on error pages » #27 Works great for 5.x

Thanks a lot for this hint.

damien tournoud’s picture

Title: #27 Works great for 5.x » Display blocks on error pages
nancydru’s picture

There is a patch in #141162: 403/404 errors should allow blocks that puts the settings in the error-reporting settings page, which IMHO is the appropriate place.

catch’s picture

Putting this in error reporting seems like the right place.

Nancy, I marked #141162: 403/404 errors should allow blocks as duplicate, could you post your patch here?

nancydru’s picture

StatusFileSize
new4.3 KB

This patch adds two check boxes to the error-reporting settings page (system.module) to indicate whether the blocks should be shown. Then it modifies common.inc to check those values when displaying the error pages.

lilou’s picture

Status: Needs work » Needs review
alan d.’s picture

It is possible to use the pre-process function in Drupal 6, you simply need to do the steps that the programmers decided to block out. (Try selling this logic to a designer!!!)

I've posted the solution here. http://drupal.org/node/129762#comment-1013470

Anonymous’s picture

Status: Needs review » Needs work

The last submitted patch failed testing.

Mark Theunissen’s picture

Subscribing, I would like to see this feature in Drupal 7. Thanks.

johnalbin’s picture

Title: Display blocks on error pages » Display blocks & menu links on 404 error page

FYI, the left/right block regions are displayed just fine on 403 Access Denied pages. See drupal_access_denied().

BTW, I fixed this issue as a contrib for D6: 404 Blocks. As always, reviews/patches are welcome!

Since I just wrangled my brain to try to figure out why/how this is happening, let me re-iterate the problem. By default, on 404 Not Found pages:

  1. The "Left" and "Right" regions of your theme are turned off by drupal_not_found() passing FALSE to the $show_blocks parameter of theme('page').
  2. The "Primary links" block, the "Navigation" block, and any other menu-based blocks are turned off. This is because, by default, site_404 is blank, so there's no active menu item and, thus, menu_get_item() returns FALSE and menu_tree_page_data() will return no data. And since menu_tree() relies on menu_tree_page_data(), there's no menu trees.
  3. The Primary links and Secondary links of your theme are turned off. Again, menu_primary_links() relies on menu_tree_page_data().

The easy fix for #2 and #3 is setting the site_404 variable to a valid path.

So my proposed solution is this:

  1. Create a new internal path, “drupal-404”, that is the default value in variable_get('site_404', 'drupal-404') and ensuring that drupal_not_found() always calls menu_set_active_item($path) with a valid path. Instant fix for #2 and #3. Also, on admin/settings/error-reporting, show a blank text field and, on submit, swap a blank site_404 with 'drupal-404' (i.e. hide the implementation detail so the user can’t accidentally break it.)
  2. Change drupal_not_found() so that, by default, it displays left/right regions. Fix for #1.
  3. Add 2 checkboxes to admin/settings/performance:
    1. “On 404 error pages, prevent blocks in left/right regions from being rendered.” This would toggle the $show_blocks parameter of theme('page') in drupal_not_found().
    2. “On 404 error pages, prevent menus from being rendered.” This would cause menu_get_item() to always return FALSE on 404 pages.

    Those 2 new options would allow high-traffic sites (the minority of Drupal sites) to have the features that core provides by default now.

What do you think? The tricky bit (from experience) will be determining if $_GET['q'] == variable_get('site_404', '') means we are on a 404 page or means the user has specified a valid menu item as its 404 page (i.e. did they set site_404 to "search", etc.) You'll see what I mean when I post the patch.

nancydru’s picture

The patch I provided worked fine for me and I had none of those problems, but I do have a 404 page.

wildmtsky’s picture

Version: 7.x-dev » 5.15
Category: feature » support

I realize I may be out of my league here but I am trying to install this patch to my drupal install. First time doing a patch, but I do have the understanding of how and why I want to do this. I am attempting to do this manually. But I have an issue with the second half of the patch. First, I have no "modules/system/system.admin.inc" file. I have a system.module file. I will assume these are one in the same because the code matches. Next the following is really confusing.

 @@ -1210,6 +1210,13 @@
     '#description' =>  t('Specify where Drupal, PHP and SQL errors are logged. While it is recommended that a site running in a production environment write errors to the log only, in a development or testing environment it may be helpful to write errors both to the log and to the screen.')
   );
 
+  $form['error_page_show_blocks'] = array(
+    '#type' => 'checkbox',
+    '#title' => t('Display Blocks'),
+    '#default_value' => variable_get('error_page_show_blocks', FALSE),
+    '#description' => t('Show blocks on error reporting pages.')
+  );
+
   $form['#validate'][] = 'system_error_reporting_settings_validate';
 
   return system_settings_form($form); 

I don't know where to put it. I looked for line 1210 but it wasn't anything. I tried to just add all of this and it gave me an error of "double arrow in wrong place"..

Please any direction would be appreciated. I hope I didn't post in the wrong area. If so, I apologize.

Thanks

wildmtsky’s picture

Status: Needs work » Fixed

Disregard my previous submission! Thanks so much for this patch! Works beautifully!

pasqualle’s picture

Version: 5.15 » 7.x-dev
Category: support » feature
Status: Fixed » Needs work
deshilachado’s picture

hope this patch goes to D7,
for D6 i'll just use http://drupal.org/project/blocks404

xano’s picture

Title: Display blocks & menu links on 404 error page » Show regions at 404 page
Component: theme system » base system
Assigned: Unassigned » xano
Status: Needs work » Needs review
StatusFileSize
new3.32 KB

The attached patch lets Drupal show regions at 404 pages. Because those contain search forms and menus, they are vital for navigating away from the page after being presented a 404. With he introduction of hook_page_alter() sites that don't want regions can easily prevent them from being rendered.

johnalbin’s picture

Status: Needs review » Reviewed & tested by the community

Okay, Xano’s solution is much simpler. And I’m fine with moving the eating-baby-kitten scope creep of “menu links” to a new issue. :-) #423696: Allow display of menu links on 404 error page

I also think that arbitrarily turning off just the left and right regions is the wrong solution on high-volume sites that need 404 page optimizations. Each site admin should determine which regions (per theme) they want to turn off and do it with hook_page_alter() as Xano suggests. Here’s the original issues which I just stumbled upon: #76824: Drupal should not handle 404 for certain files #80201: Don't show blocks on bootstrapped 404 pages

Patch review:

  1. For those wondering why $page = drupal_get_page($return); got nixed (I was), drupal_render_page actually calls drupal_get_page() if $return is a string, so it was redundant, I think.
  2. With the patch applied, the left and right sidebars are restored on 404 pages. yay! Marking this RTBC.

Another tangent: However, I think if we want to remove this bit of crufty-unoptimization from 404 pages, we should look at overhauling the entire $show_blocks variable system. That idea is baby-kitten-eating as well, so I've opened another new issue: #423992: Remove show_blocks page variable Xano, I haven't investigated hook_page_alter() enough to know if we should throw $show_blocks away or make it more configurable; can you give this new issue a quick glance?

robloach’s picture

When posting the patch at #14, I was under the impression we were giving the user a setting to either enable or disable the blocks on 404/403 pages, due to the hit on the server? I understand that hook_page_alter() could let contrib modules disable them, but shouldn't that option be in core?

nancydru’s picture

I agree that this should be in core and should be a clickable option. Hook_page_alter requires programming knowledge and how often do we see people post that they don't know how to code? The patch in #41 does this.

xano’s picture

I disagree. The current 404 pages are hell. There is no way to navigate away, unless the custom page offers a search box or some kind of menu. This behaviour is bad for UX. If we have a checkbox to disable those regions, we suggest that disabling regions at 404 pages is not a bad thing to do, while in fact it is.

nancydru’s picture

That was my argument in the very beginning, but it will fall on deaf ears. That decision was made when 5.0 was being developed.

xano’s picture

The problem is that providing a checkbox allws users to disable regions, making 404 pages as bad as they are now. Without knowledge of coding, those pages will never get any better. If there are modules that provide an improved 404 page, those modules can programmatically disable regions.

alan d.’s picture

Just my 50c on this issue.

If the site requires the tiny amount of performance increases that hiding the blocks on a 404 would give, they should have the resources to create a simple hook.

If the site doesn't require the performance increases, they probably do not have the ability or resources to switch on these regions.

All regions should be displayed by default for maximum usability for the entire community.

Also, consider this link:

http://drupal.org/sdgsdfg

Just two links and a search box that doesn't work. Even the best sites need help sometimes!

nancydru’s picture

The Drupal-gods have already decided that those regions should be disabled although you and I agree that was not a good idea. At least the checkboxes will give me the option to turn those regions back on. I cannot think of an instance where I really want those regions turned off by any module, so if I found a module that did that, I would request the same option from that developer.

alan d.’s picture

Just a side note, wouldn't it be better to look at ways to intelligently handle what the 404 was?

An image file would just require the header mime, size = 0, and 404 status set. A grand total of 50B maybe. The above 404 link was 45K.

dries’s picture

If we add a block setting, where do we expect that setting to live? On the 404 settings page or on the block configuration pages? I'm thinking it would be another block visibility setting on the block configuration page.

nancydru’s picture

In my patch, I had it on the 404/403 settings page so that every block wouldn't have to be set and my site looks the same no matter what. But I can see some logic to doing it per-block.

robloach’s picture

In my patch, I had it on the 404/403 settings page

That page was merged into the Site Information settings. I was thinking about adding it in the Block Cache fieldset in the Performance settings, but not too sure if that's appropriate.

xano’s picture

@Dries: for what reason exactly do you want blocks to be hidden at 404 pages?

nancydru’s picture

How about this possibility: Add another special page designation to the page specific visibility settings, perhaps <error>. This minimizes the impact on the admin and is a part of the UI that all admins should be familiar with. They could then include or exclude easily and in a way that becomes self-documenting.

webchick’s picture

Status: Reviewed & tested by the community » Needs review

@Xano: This functionality was originally put in in #232037: block_list() renders all blocks, even on 404 and you can read the reasoning there; namely that block_list() will render ALL blocks the first time it is called, which is a big performance drain.

While users sometimes trigger 404s and 403s, more often than not these are triggered by things such as a stupid version of IE checks for http://www.example.com/favicon.info, or a badly coded search engine crawler, or script kiddie hitting your site with a script checking for four thousand 1994 exploits like "/cgi-bin/wwwboard.pl" not to mention if you have legacy content and don't have 401 redirects in there. This kind of stuff can start to eat processing power of even a modest site with very little traffic. As a result, 404s and 403s need to be as cheap as possible.

@#62 it sounds like Dries is okay with putting a setting in for this on the off-chance that your site is free of this kind of crap and actually gets a larger portion of real human visitors triggering 404s, so I guess that's one way to do it (I personally would opt for a single setting "Show blocks on 403/404 pages" in the same place as you tell it what 403/404 pages are, defaulting to off.)

The main issue here though is I don't see any of the people involved in making the original change speaking up in this issue, and that needs to happen before we commit anything. So touch base with people like pwolanin, moshe, killes, etc. and see if you can come up with a compromise here.

webchick’s picture

Issue tags: +Performance
Bojhan’s picture

Doesn't really look like a usability issue - unless you actually give them a search box. Either way, please provide a screenshot what the proposed solution is.

Status: Needs review » Needs work

The last submitted patch failed testing.

robloach’s picture

Status: Needs work » Needs review
Issue tags: -Needs screenshots
StatusFileSize
new55.73 KB
new2.6 KB

This patch adds the ability to show blocks on the 403/404 pages and sticks the configuration in the Site Information page like webchick suggested.

Status: Needs review » Needs work

The last submitted patch failed testing.

tstoeckler’s picture

Confirmed the test fails.
I also tested manually and that doesn't work properly either.
When set to show blocks on 404/403:
On 404 page: Shows blocks
On 403 page: Shows blocks

When set to hide blocks in 404/403:
On 404 page: Shows only the right sidebar (On Garland, with some core blocks in the right sidebar)
On 403 page: Shows blocks

johnalbin’s picture

site_error_show_blocks should default to TRUE. Users shouldn't have to hunt for an option to allow site navigation when there's a broken link. Users with performance issues will do their research and find this checkbox easily.

pasqualle’s picture

and for users who upgraded from Drupal 6, site_error_show_blocks should default to FALSE.

nancydru’s picture

@Pasqualle: I respectfully disagree. It was never an option in D5 or D6, so I should not be penalized because I went to those releases.

tstoeckler’s picture

It's not penalizing it's just not changing anything in your default experience, which, IMO is good.
But I also think that discussion shouldn't hold this issue up.

xano’s picture

@NancyDru: when upgrading to newer versions of Drupal we try to change as little of the existing configuration as possible, to prevent unwanted side effects of the upgrade.

nancydru’s picture

I understand that, but think back to how this even got started. This change was made in 5.x without regard for what I wanted (I was new and did not have the knowledge to participate at that time). That was bad; this change is good. IMHO, this is restoring something that should, at the very least, have been an option for me. +1 for TRUE, regardless of the means by which I get there.

pasqualle’s picture

tstoeckler’s picture

Status: Needs work » Closed (duplicate)
Michael-IDA’s picture

Version: 7.x-dev » 6.13
Category: feature » bug
Status: Closed (duplicate) » Active

Removing duplicate status as that issue is in regards to Blocks only.

This ticket encompasses all regions on a 404 page. Which as of 6.13 is still broke. Specifically if one does the well known, and almost always done, core hack of #6 above, menus in D6.x still disappear. Although you do get your Blocks back.

[soapbox]
NO ONE should have to hack core. For Anything.
[/soapbox]

So, I don't care how it's done, but, this needs fixing, and has needed fixin' for a long time. Please make the Default 404 response to show ALL regions of the default theme. If you want to add opt out logic for the small percentage of high traffic sites, great, but please stop making the vast majority of Drupal installs do core hacks just to get a standard level of user friendliness.

Best Regards All,
Sam

[more soapbox]
PS: Why hasn't this been incorporated correctly in core? Why does just about everyone have to do Core Hacks just to have a user friendly site? Do I now have to instruct everyone who I work for not to use Menus at all, and that they're going to need to manually build 'menus' from Blocks? Let me tell you, that's a fun conversation. Which quickly leads to, "Why the F*, did you recommend this?" Yes, I'm bitching, and YES this is a Bug. Seriously, what percentage of Drupal sites have such high traffic that they even need to consider dumping regions for 404 pages?
[/soapbox]

webchick’s picture

Title: Show regions at 404 page » Show menus at 404/403 pages
Version: 6.13 » 7.x-dev

PS: Why hasn't this been incorporated correctly in core?

Mostly, because people spend their time soapboxing and not creating/reviewing patches, which is the only way changes get made to core.

It sounds like you're keenly interested in getting this resolved. That's great. To do so, here's what you need to do (or need to find someone to do, if you lack the technical background):

1. Get a copy of Drupal 7. We always fix bugs in the latest development release first and then backport. You can either download it from the main Drupal download page (7.x-dev), or else check it out from CVS (it's the HEAD branch).
2. Make the appropriate changes to core so the bug gets fixed.
3. Create an automated test that proves the change you made worked.
4. Create a patch, and upload it to the issue.
5. Mark the patch as "needs review." Probably also assign it to yourself if you're planning on following through with it.
6. Find some people (or be one yourself, if you don't write the patch) to give the patch a thorough review. Are the coding standards okay? Does the patch indeed fix the bug? Is it possible to break it with funny settings? Is there a more efficient/correct way it could be done?
7. Once problems are all resolved, the last person to review it marks it "reviewed & tested by community." It then goes into the core comitters' queue for possible inclusion into Drupal core.

There are lots of people in #drupal that can help you with any of these tasks. But no one can help you be less frustrated if you don't take active steps to resolve the problem. In fact, that kind of behaviour generally has the exact opposite of its intended effect. Everyone who works on Drupal core is a volunteer, and most volunteers would rather help fix problems for people who are keen to help, rather than fix problems people who are demanding to know why people haven't done work they want for free yet.

Hopefully you'll take the steps required to see this through!

tstoeckler’s picture

Title: Show menus at 404/403 pages » Show regions at 404 page
Category: bug » feature
Status: Active » Closed (duplicate)

This has been fixed in the other issue (see #80) and will therefore be part of Drupal 7.
If you want this functionality in Drupal 6 try the 404 Blocks module.

Reverting issue settings.

kenorb’s picture

I'm not sure, but probably this one is related
#595724: After core upgrade: Fatal error: Unsupported operand types ... /modules/system/system.module on line 627
Because system.module getting empty value from menu_get_item() for some reason.

xano’s picture

Assigned: xano » Unassigned