remove bartik_page_alter() and extra block classes [#889100]

The $page array is supposed to be a renderable array, and the first level things in it are regions... but the second level is not necessarily blocks... it can be arrays that contain blocks (or really could be any renderable array). bartik_page_alter() makes some pretty inflexible assumptions about this, as discovered in #867256: Create render example demonstrating D7 render arrays and altering. Basically, when bartik_page_alter() is searching for blocks to set block ordering (IMO) it needs to search the region recursively, not just assume a block in the top-level element.

To demonstrate the error, use the patch in #867256: Create render example demonstrating D7 render arrays and altering and enable block array reporting.

Attached is a patch that would be more generous about its approach to the page array.

Comment	File	Size	Author
#5	bartik-remove-hook-page-889100.patch	1.78 KB	Jeff Burnz
	drupal.bartik_page_array_fix.patch	2.55 KB	rfay

Comments

Comment #1

Jeff Burnz commented 25 August 2010 at 23:43

I added the example module and without the patch I get a bunch of notices:

# Notice: Undefined property: stdClass::$position_first in bartik_preprocess_block() (line 112 of C:\Users\Jeff\Documents\Aptana Studio Workspace\d7\drupal\themes\bartik\template.php).
# Notice: Undefined property: stdClass::$position_last in bartik_preprocess_block() (line 115 of C:\Users\Jeff\Documents\Aptana Studio Workspace\d7\drupal\themes\bartik\template.php).
# Notice: Undefined property: stdClass::$position in bartik_preprocess_block() (line 119 of C:\Users\Jeff\Documents\Aptana Studio Workspace\d7\drupal\themes\bartik\template.php).

The page alters appear to working regardless - however after applying the patch http://drupal.org/files/issues/drupal.bartik_page_array_fix.patch the notices go away.

Consider this a friendly bump - certainly like to get some more eyes on this.

Comment #2

moshe weitzman commented 26 August 2010 at 10:01

Any chance we can just put these classes in for all themes (i.e. move to block module)?

Comment #3

Jeff Burnz commented 26 August 2010 at 11:25

I looked at this more in Bartik and from what I can tell these classes are not even being used. I suspect they may have been something to do with layout before we remodeled and simplified the entire layout CSS.

Frankly, as a themer, my experience with these first/last classes on blocks is that they sound good to have but in reality you hardly ever use them, as in almost never. Be interested what other themers say about these classes, I personally have not had a use for them in living memory and with modules like Skinr, blockclass and so on its really easy to add classes a specific block if you need it - its just so rare to have any need to style the first and/or last block in a region (layout aside, which can be useful for eliminating padding in grids layouts).