Currently, the $show_blocks variable allows modules, 404 error pages, etc to turn off the left and right sidebars. This is supposed to be an optimization to reduce processing in certain circumstances. See #76824: Drupal should not handle 404 for certain files and #80201: Don't show blocks on bootstrapped 404 pages for the original discussion.
Unfortunately, Drupal has no way to know which regions a site administrator would consider essential to display on every page on the site. e.g. if the primary navigation is a block in the left region, that block must be shown on 404 pages, etc. So turning off just the left and right sidebars is pretty arbitrary.
So we have 2 options:
- Remove this functionality entirely or
- change the $show_blocks to a $hide_regions variable which holds names of regions to turn off when performance is crucial. So, $hide_regions would be a per-theme array that held the names of any regions that would need to be optionally hidden for that theme.
Thoughts?
Comment | File | Size | Author |
---|---|---|---|
#16 | show-blocks-423992-10.patch | 1.54 KB | davyvdb |
#14 | drupal.show-blocks-revert.patch | 9.96 KB | sun |
#9 | show-blocks-423992-9.patch | 9.96 KB | JohnAlbin |
#6 | system-404-regions-423992-2.patch | 3.9 KB | davyvdb |
#4 | system-404-regions-423992.patch | 2.59 KB | davyvdb |
Comments
Comment #1
JohnAlbinAfter talking to Moshe briefly in IRC, he recommended we remove it.
Changing issue title. Hopefully a patch should be easy to make in the next few days.
Comment #2
davyvdb CreditAttribution: davyvdb commentedI have documented this once on my blog. You can do this in the theme right now. I do this on every site now. It's also interesting when you use multiple page templates where you have different regions for each page template.
First I disable the show_blocks
http://www.drupalcoder.com/story/411-showing-left-and-right-region-on-pa...
Then I create my own block_list that does the checks using a mapping function
http://www.drupalcoder.com/story/313-improve-drupals-performance-by-not-...
http://www.drupalcoder.com/story/339-improve-drupals-performance-by-not-...
Do you think we need a UI for this? Throttle would have been nice here.
An idea for a UI : Add checkboxes for all regions to /admin/appearance/settings/garland (and other themes) asking... Which regions should never be hidden if Drupal decides to leave out a region for performance. Drupal tends to do this on pages like "page not found", "access denied".
Comment #3
JohnAlbinNote that when removing show_blocks, we'd need to remove the “Display blocks on default 404 (not found) page” checkbox on admin/settings/site-information as well.
@Davy, D7 already has the flexibility built-in that would allow a contrib module to have that kind of interface to those user who need that kind of performance boost. So I think we should remove the show_blocks functionality from core (meaning always show regions on 404/403) and let contrib handle a region/block hiding UI for 404/403 pages.
Comment #4
davyvdb CreditAttribution: davyvdb commentedI've removed the form stuff from Site information.
I've done a variable_del in system_update.
I've cleaned up drupal_not_found.
Comment #6
davyvdb CreditAttribution: davyvdb commentedComment #8
davyvdb CreditAttribution: davyvdb commentedThis patch actually works fine, but it keeps failing because of a bug in the block tests.
http://drupal.org/node/557024
Comment #9
JohnAlbinDavy, awesome start.
The changes you made to drupal_not_found() look too extreme because it removes the the calling of HOOK_element_info_alter on the page. I've reverted that bit. And combined your patch in #557024: Wrong URL in drupalGet results to wrong test results. Also, a global search for show_blocks shows we missed some spots.
Here's a new patch.
Comment #11
davyvdb CreditAttribution: davyvdb commentedAha, you want to remove the underlying show_block stuff too. Looks great now!
Comment #12
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!
Comment #13
JohnAlbinBah. That patch broke the System: Drupal Error Handlers simpletests in modules/simpletest/tests/error.test. :-p
Comment #14
sunStraight roll-back makes error tests pass again.
Comment #15
davyvdb CreditAttribution: davyvdb commentedHold on, I'm creating a patch to fix this (an not re-roll this).
Comment #16
davyvdb CreditAttribution: davyvdb commentedThis fixes this.
The cause was that show_blocks is removed from theme('maintenance_page')
Comment #17
JohnAlbinI looked at the patch in #16 and it is correctly removing the 2nd parameter from all the calls to theme_maintenance_page in D7.
RTBC from me.
Comment #18
Dries CreditAttribution: Dries commentedCommitted. Thanks.
Comment #19
jhodgdonNeeds to be added to the http://drupal.org/update/theme/6/7 theme update guide, and perhaps to the module update guide?
Comment #20
catch#653068: Update documentation is incomplete
Comment #21
effulgentsia CreditAttribution: effulgentsia commented.
Comment #22
jhodgdonIt's best to leave the title as it was. The "Needs documentation" tag takes care of notifying us what needs to be done.
Comment #23
jhodgdonchanging tagging scheme
Comment #24
jhodgdonAdded this to http://drupal.org/update/themes/6/7#show-blocks