Closed (fixed)
Project:
Drupal core
Version:
x.y.z
Component:
block.module
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
29 Oct 2006 at 19:49 UTC
Updated:
22 Nov 2006 at 19:32 UTC
Jump to comment: Most recent file

Comments
Comment #1
moshe weitzman commentedgreat idea ... block admin with multiple themes is quite tricky so we should test this patch in that scenario. of course, i have a feeling that scenario behaves unexpectedly with or without this patch.
Comment #2
RobRoy commentedTested this out. Works well. A very nice improvement. +1
There's still some work to be done with block administration and this is a step forward.
Comment #3
RobRoy commentedChanged title.
Cross referencing http://drupal.org/node/92630
Comment #4
moshe weitzman commentedRobRoy - if possible, mark this as RTBC or say what more is needed.
Comment #5
RobRoy commentedOkay, marking this RTBC. I gave it a +1 in my first comment above and the patch is still great. Shortly I will submit a patch in the issue I referenced in #3 to add the region drop down to the block add/edit page as well.
Comment #6
RobRoy commentedI'm modifying this patch so that we use a new constant BLOCK_REGION_NONE instead of just -1, but I had one question: On line 324 I was going to write a comment for why we are using !== instead of != (like core is doing for all ==='s). Why are we using a !== instead of != there?
Comment #7
RobRoy commentedAlso, is the (int) on 289 necessary since we are passing that through %d which would ensure that a boolean is converted to an integer value? Just want to make this clean.
Here is a change with ONLY the addition of the constant. Still think maybe we should change !== to != and remove the (int). Thoughts?
Comment #8
kkaefer commentedRobRoy is right. We don't need (int) and !==.
Comment #9
profix898 commentedCode looks good and works as expected.
Really nice improvement :)
Comment #10
dries commentedI like. Good job. Committed.
Comment #11
(not verified) commented