Hi Guys

I noticed that this module is not using drupal's API to send 403 and 404 errors. I reckon it should, that's what they are there for.

I'm attaching patches with that issue fixed.

One for 6.x-2.0-dev version
one for 6.x-2.0-beta10

CommentFileSizeAuthor
#1 imagecache-6.x-2.0-beta10-928646_1.patch1.09 KBjm.federico
#1 imagecache-6.x-2.0-dev-928646_1.patch1.09 KBjm.federico
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jm.federico’s picture

jm.federico CreditAttribution: jm.federico commented
Version: 6.x-2.0-beta9 » 6.x-2.x-dev
FileSize
imagecache-6.x-2.0-dev-928646_1.patch1.09 KB
imagecache-6.x-2.0-beta10-928646_1.patch1.09 KB
jm.federico’s picture

jm.federico CreditAttribution: jm.federico commented
Priority: Normal » Major

Changing to major, usage of Drupals API is a must for modules.
Without this, there are modules that won't report things correctly. Like statistics module (we won't know a 403/404 was sent). Or 404 Blocks module, well, becase no page is shown, no blocks are shown.

Also, if viewing an image in full page, user will just get a blank page and will have no idea what happened. It even confused me for a second, and I knew what was going on. There is no intuitive way of knowing what happened if you are not a developer. A normal user just gets nothing and thinks, hell, page is broken. And it is not, he just got to a 403 or 404 page.

sandrewj’s picture

sandrewj CreditAttribution: sandrewj commented
Status: Needs review » Reviewed & tested by the community

May need to be re-rolled now, but works fine.

mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented
Status: Reviewed & tested by the community » Needs work

This would be a major performance hit. Fast 404/403 is a much better option.

fizk’s picture

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

Need good reason to use Drupal core if there would be a big performance hit as per #4.

mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented
Title: use drupal_access_denied() and drupal_not_found() » Issue a fast 404/403 with some text instead of no text (looks like WSOD)
Category: bug » feature
Priority: Major » Normal
Status: Closed (won't fix) » Active

httprl_fast403() and advagg_missing_fast404() are 2 examples of a simple fast 40* call that provides some context to the user. Right now you get a blank screen with no idea what module created it. As it is, you can sometimes think its a WSOD.