Refactor block-admin-display-form.tpl.php

I think that we need to look at condensing functionality that this TPL provides. @jessebeach, a renderable array is probably ideal. This would also make it easier to remove the table drupal_add_js and drupal_add_css' from the template.

Marking needs work.

I've started to refactor this system, as per the attached patch. This is the start of an effort to move logic out of the template for the block admin section. Essentially, this patch is the beginning of ripping the 'horribleness' out of the block module. This patch passed simpletests for the block system locally.

Tested it as well and all in favor

I think this needs more review before it is RTBC. I know I haven't had an opportunity to look at the patch, and many others may want / need to chime in first, including those involved in the D8 gates (should any apply). Let's give it a few days for some of us to have time to review.

@Everett: This merely moves code from a template file to the PHP module file, where it should theoretically be by definition anyway. There is no change involved otherwise, it is just a small cleanup. I think its fine to commit as-is.

Nice cleanup! Looks good to me too.

Let's use #attached on the form here, including 'tabledrag'. Ideally also merge the 'css' line into it.

We should also try to backport this.

sun, do you think that should be a separate issue, it seems to me that since there is a commitable patch here, we can do the render change in a seperate issue. I'm in favor of backporting and will roll patches for 6 and 7.

Let's do it in this issue. Although I added the tag myself, I'm not sure whether this can be backported.

No idea how to handle the suggestions. Anyone? I can roll if I get instructions.

This should (hopefully) work.

It doesn't

Warning: call_user_func_array() expects parameter 1 to be a valid callback, function 'tabledrag' not found or invalid function name in drupal_process_attached() (line 4498 of C:\xampp\htdocs\drupal8\core\includes\common.inc).

And I'm not sure the css gets loaded

oopsie, yeah, 'tabledrag' needs to be 'drupal_add_tabledrag'.

The short form is only supported for 'css', 'js', and 'library'.

Hmm, dragging in the disabled region does some funky things.
I think there is a problem with #attached and multiple tabledrag's.

I maybe have seen an issue somewhere... Can't find it :(

Ok, to see what doesn't work:

1) apply the patch
2) go to structure ==> blocks
3) go to disabled blocks area and try to drag it to the top

You'll see that you can drag but when you let go you get some funky results. It feels like the "end drop" doesn't get triggered somehow.

Ouch. Let's try to figure out what's going on here. Tagging.

Gosh, that took some time to figure out.

+++ b/core/modules/block/block.admin.incundefined
@@ -84,13 +84,22 @@ function block_admin_display_prepare_blocks($theme) {
+  // Add a last region for disabled blocks.
+  $block_regions_with_disabled = $block_regions + array(BLOCK_REGION_NONE => BLOCK_REGION_NONE);

If this is always set, can we just do array(BLOCK_REGION_NONE => t('Disabled')) and kill off those 3 lines in template_preprocess_block_admin_display_form?

Otherwise, this is the same conclusion I *just* came to during the HTML5 sprint.

Whoops.

Although I agree that it's very wonky, I intentionally did not want to open that can of worms. This form is re-used by the Dashboard module, which is one of the primary reasons for it being so wack.

This one is ready. tim.plunkett was working on the patch in the last HMTL5 meeting as we were discussing the problem, and was about to post the exact same patch.

Committed to 8.x. Moving to 7.x. Thanks all.

Uhhh. Is this actually backportable? If someone already has a copy of block-admin-display-form.tpl.php in their theme, don't they end up with two table headers?

Not rly two table headers but the css/jss gets loaded multiple times and I'm not sure thats a good thing. AND this is not broken in drupal7 it's just a D8 cleanup, so I don't think it's a good idea to backport this.

Marking fixed.

Cool; restoring old properties.

Thanks! Removing the sprint tag.