Hi all!
I'm proud to present this patch, it does some simple but yet very useful changes to the error reporting system. Basicly this is a version of Search404 (http://drupal.org/node/26010) on steroids, with some enchantments. All features from Search 404 are kept. It has been rewritten to be more stable, secure and fast than before. New features has been added:
- The error message is now configurable
- Three special paths has been added;
- <blank> - It's the classic Drupal 404 page, just an error message
- <front> - Invokes the frontpage - with an error message set to $message
- <search> - This is what Search 404 accomplishes and is also the standard 404 page if nothing else is specified. It performs a search on the words from the requested URL. The options to configure Search 404 remains:
- Display a link instead of the search results directly. This feature is particular useful when many bots is hitting the site.
- Jump directly to the search result when there is only one result.
- Use OR between keywords when searching.
- A list of words that should be ignored (php, html etc)
- Of course any Drupal path can still be specified
- There's now an option to send "404 Not found" header or not
- Brand new 403-handling:
- Can use <blank> and <front>, but also <login> which will display a login form.
- Configurable error message and option to send "403 Access denied" header
Care has been taken to not create loops, such as the user don't have access to the 404 page, and the 403 page itself isn't found.
One reason why this should be in core, is that error-handling surely is someting the CMS itself should take care of.
How to test:
1a) Visit: http://forngren.com/fantastic40x/
1b) Make a fresh Drupal HEAD install, and make sure that everything seems ok. Apply the patch attched
2) Fool around with the site
3) Report results back in this issue
Links
http://drupal.org/node/26010 Search 404
Posted by geokker - November 7, 2004 - 11:56 http://drupal.org/node/12668
I can't believe how many sites miss this trick. I was looking for a radio programme 'Loose Ends' on the BBC website. I tried www.bbc.co.uk/radio/looseends and got a 404.Why? How difficult would it be to put the non-matching part of the URL into a search and bring up 'Sorry, we couldn't find an exact match - did you mean...' then list trip_search results or something.
If anyone can figure this out, it should go into core Drupal functionality.
A good CMS has an excellent search facility.
A good site has this search facility on every page.
When I'm looking for something on a site, as soon as it renders I instantly judge it. If I can't quickly see a search box - I hate it. If there is a ton of content on the front page - I'm not a fan. If the search is slow, or doesn't yield coherent results - I'm dissapointed. If it's Flash - I'm not impressed.
http://drupal.org/node/7570 Diskussion about "Custom 404 Error Page"
Sideeffect
There is a tiny sideeffect, though. After using this for a few days on a test site with 6 nodes, I stopped using the navigation and just typed what I was looking for in the URL. I think that Bert Boerland can confirm this.
Final words, contact info and more
First of all, none of this would be possible if it wasn't for the work done by Lars Sehested Geisler/larssg, the maintainer of Search 404, and Bert Boerland, which has been a great support during the process of writing, testing and documenting this patch. Since I've summerholidays I can usually fix bugs/errors within a few hours (awake between 10-24 GTM +2). If you want to discuss this patch, please, do so. I'm in #drupal when I'm awake. You can also use my contact form (http://drupal.org/user/40238/contact) or skype me (forngren).
Cheers!
Comment | File | Size | Author |
---|---|---|---|
#37 | better_404-403[1].diff | 6.31 KB | Robin Monks |
#24 | Fantastic40x_episode1.patch_8.txt | 8.48 KB | forngren |
#22 | Fantastic40x_episode1.patch_7.txt | 8.35 KB | forngren |
#18 | Fantastic40x_episode1.patch_6.txt | 7.92 KB | forngren |
#16 | Fantastic40x_episode1.patch_5.txt | 8.46 KB | forngren |
Comments
Comment #1
forngren CreditAttribution: forngren commentedBtw, some errors appears:
I'm not sure if those is because of the patch or Drupal itself, probably the first.
Comment #2
moshe weitzman CreditAttribution: moshe weitzman commentedComment #3
forngren CreditAttribution: forngren commentedDue some recent discussions (e.g. http://drupal.org/node/76824), I've decided to split this patch in three smaller ones; the one attached, a 403 patch and 404 patch.
This patch does some improvements to the error reporting settings:
I think this code is pretty robust, so it's mainly spelling and coding standards that needs to be checked.
Cheers Johan Forngren
Comment #4
Dries CreditAttribution: Dries commented1.
if (empty($return) || $return == MENU_NOT_FOUND || $return = MENU_ACCESS_DENIED) {
Should that be '=='?
2. Incorrect use of t().
3.
+ $return = ' ';
is ugly.4. Why the various extra variable_set()s? I vote against these.
Comment #5
forngren CreditAttribution: forngren commentedFixed
Not quite sure what you mean.
Agreed, but I didn't wanna leave it blank/" " since I think that will breake some templates, how about setting the message as $return?
Not quite sure here either, do you mean the settings I added? In that case, I'm not sure howto remake them, or should some of them be removed?
(your opinons/review are greatly appreciated)
Comment #6
beginner CreditAttribution: beginner commentedabout the use of t():
you have hard coded html within your strings, and what's worse they are not compliant with xhtml standards (
<i>
is deprecated, you don't have any<p>
around your paragraphs, etc.).Check http://api.drupal.org/api/HEAD/function/t.
I am not sure what the complete solution would look like, but this gives you a small clue to know where to start.
Can someone more competent complete what I am saying?
Comment #7
forngren CreditAttribution: forngren commentedFinally, here's the updated patch. I still use some (x)html within the code;
But I think that's cleaner than:
Still not fixed
Switched to <em>, see above
Thanks to unconed, dries, killes and especially webchick for explaining things.
(sorry for the messy post)
Comment #8
forngren CreditAttribution: forngren commentedKeeping w/HEAD.
Because of http://drupal.org/node/76824 I think the need for this patch increased, so we don't just "blank" the page on every 404.
(I did also take down the test site cause noone visited/used it)
Comment #9
drummWhy would anyone want to disable the 404 HTTP header? I do not think that should be a setting.
Comment #10
forngren CreditAttribution: forngren commentedRerolled without the ability to toggle headers...
Comment #11
forngren CreditAttribution: forngren commentedTrying to fix html...
Comment #12
forngren CreditAttribution: forngren commentedKeeping w/HEAD. People, if you want to see all of the above stuff in core, please make a review of this patch so I can focus on the rest of it. Thanks!
Comment #13
Dries CreditAttribution: Dries commentedI think the default 404 message is too technical. People don't care whether URLs of a website are case-sensitive. Plus, it is not necessarily true.
Comment #14
Dries CreditAttribution: Dries commentedCan we create a new issue for this? The entire page is screwed up because invalid HTML usage. Thanks.
Comment #15
forngren CreditAttribution: forngren commentedI'm affraid that I've done it as user-friendly as could, I must ask someone else to step in and help me or point me in the right direction.
Try:
http://drupal.org/node/1
http://drupal.org/NODE/1
http://drupal.org/?q=node/1
http://drupal.org/?q=NODE/1
Can we wait until the first part of this patch gets in?
Cheers Johan Forngren
Comment #16
forngren CreditAttribution: forngren commentedUpdated 404 message to "We are sorry, the requested page (%path) was not found on this webserver. Either the URL does not exist or the page you were looking has been deleted."
Comment #17
Dries CreditAttribution: Dries commented- Please drop the %path functionality.
- In 'Enable error message', 'Enable' seems redundant. Or maybe it should be 'Display'.
- It is not clear how this works. What happens with the error message when I set an alternate 404 page. Even so, what is the added value? People can already customize the message through an alternate 404 page. The texteare is easier to use, but maybe we should eliminate the 'Alternate 404 page' feature then. Right now, it is not clear how the textarea and the alternate 404 page mix.
Comment #18
forngren CreditAttribution: forngren commentedDroped, changed, explained, postponed :p
Comment #19
Dries CreditAttribution: Dries commentedThere are still two mechanisms to configure these? :S
Comment #20
forngren CreditAttribution: forngren commentedOk, here's the deal;
There are 6 options in total, 3 for 403, and a copy of those for 404. The options are:
Page to display:
[TEXTFIELD] This page is displayed when a 403 error occurs. If you are not using clean URLs, specify the part after "?q=". If unsure, leave blank.
This is the 'classic' 403 setting, i.e. what page users should be redirected to when a 403 occurs.
Display error message
[Checkbox] Enabling this feature will display the message below when a 403 error occurs, even if "Page to display" is set.
When this is checked, the message below will be shown (drupal_set_message), independent if "Page to display" is set or not
Message:
[textarea]
The message, see above
(sorry for messing up the xhtml even more, but as you said; it's fubar. Hope you'd understand)
Comment #21
drummLooks okay, but I think the language could use some work.
- 404 and 403 are HTTP codes, not meant for people. Go ahead and leave '40x - ' out.
- There aren't really any 'files', should be 'Page not found.'
- 'web server' is not one word.
- Don't abbreviate 'admins'
'Other error reporting' has a weak name and groups two unrelated fields together. The fields can be left without a fieldset.
Comment #22
forngren CreditAttribution: forngren commentedFixed all of drumm's issues above.
I have also created new default messages, but I'm not sure about the t() use, see below. They have been discused in irc and some native English speakers (sorry, don't recall names) have aproved it.
Cheers Johan Forngren
If this t() use doesn't fulfill the the coding standards, please submit a corrected version or tell me what needs to be fixed. Thanks
Comment #23
forngren CreditAttribution: forngren commentedI'm not sure why #default_value jumped down a line in second example, but that's not the case in the patch.
Comment #24
forngren CreditAttribution: forngren commentedKeeping w/HEAD.
Comment #25
drummpatching file includes/common.inc
Hunk #1 FAILED at 318.
Hunk #2 succeeded at 360 (offset 13 lines).
Hunk #3 succeeded at 375 with fuzz 1 (offset 13 lines).
Hunk #4 succeeded at 402 (offset 13 lines).
1 out of 4 hunks FAILED -- saving rejects to file includes/common.inc.rej
patching file modules/system/system.module
Hunk #1 succeeded at 603 (offset 22 lines).
Hunk #2 succeeded at 673 (offset 22 lines).
This is a large patch. Please think about ways to logically break this up into quickly reviewable chunks which stand on their own.
Comment #26
forngren CreditAttribution: forngren commentedIndeed, I have been to busy with school to have time for this patch. However, when 5.x is released I'll correct and submit this again.
Comment #27
forngren CreditAttribution: forngren commentedI'll let this rest until chx's gigantic menu patches are i place.
Comment #28
BioALIEN CreditAttribution: BioALIEN commentedI hoped this would make its way into D5 but I guess we're all resting on chx's new menu magic.
Comment #29
moshe weitzman CreditAttribution: moshe weitzman commentedlets keep this one alive. i think the menu stuff has settled down now. only minor tweaks there left that don't affect 403/404.
Comment #30
Morbus IffSee also http://drupal.org/node/141162. forngren: if we're going to improve the 404s as much as your patch suggests, then I /support/ the addition of the checkboxes mentioned in that issue. My current "by design" suggestion is fine in the current state of Drupal - but would really force the user to lose a whole bunch of good stuff if one has to choose between the features in this patch vs. "merely" displaying blocks.
Comment #31
NancyDru+1 for me on that, since it was my patch.
I like most everything in this patch, but agree with the above comment that it is getting large (but then it is for a new release, not current application).
Instead of the default message idea, is there any way to have it just create a default page for each error and let the admins use that as a starting point for customization? That might reduce this patch's size, and encourage more admins to actually create such pages.
Comment #32
forngren CreditAttribution: forngren commentedPostponed until next hunting season, i.e. 7.x-dev. Thanks for all feedback so far.
Comment #33
forngren CreditAttribution: forngren commentedComment #34
forngren CreditAttribution: forngren commentedFree issue give-away ;)
Comment #35
lilou CreditAttribution: lilou commentedComment #36
NancyDruSee also #116895: Show regions at 404 page
Comment #37
Robin Monks CreditAttribution: Robin Monks commentedFree re-roll compliments of patch bingo!
Robin
Comment #38
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last submitted patch failed testing.
Comment #39
bsherwood CreditAttribution: bsherwood commentedIt's been a while, has anyone made any progress with this?
Comment #40
andypostIs there any duplicates of the issue?
Comment #43
dpiAlthough not technically a dupe, this issue offers a better alternative to this issue.