Impossible to set attributes for all entities

Okay, #1982024: Lazy-load Attribute objects later in the rendering process only if needed is in. Have fun! :-)

Looks superb, just applying the pattern of #attributes turning into useful things as found elsewhere in Drupal core.

+++ b/core/includes/theme.inc
@@ -2682,7 +2682,7 @@ function template_preprocess(&$variables, $hook) {
-    'attributes' => array(),
+    'attributes' => isset($variables['elements']['#attributes']) ? $variables['elements']['#attributes'] : array(),

Excellent bug fix. Let's add a test.

+++ b/core/modules/block/block.module
@@ -545,6 +545,11 @@ function template_preprocess_block(&$variables) {
+  // Inherit the attributes set on the content of the block.
+  if (isset($variables['elements']['content']['#attributes'])) {
+    $variables['attributes'] = array_merge_recursive($variables['attributes'], $variables['elements']['content']['#attributes']);
+  }

We need a @todo here to fix this to use $variables['content_attributes']. That requires $variables['content'] to be a render array rather than a string, and that's being fixed in #1927608: Remove the tight coupling between Block Plugins and Block Entities.

+++ b/core/includes/theme.incundefined
@@ -2682,7 +2682,7 @@ function template_preprocess(&$variables, $hook) {
   $variables += $default_variables + array(
-    'attributes' => array(),
+    'attributes' => isset($variables['elements']['#attributes']) ? $variables['elements']['#attributes'] : array(),
     'title_attributes' => array(),
     'content_attributes' => array(),
   );

This made me go a little WTF, since obviously we need this same fix for title_attributes and content_attributes. Should we make separate related issues or try and fix them here as well?

Can we write
'attributes' => isset($variables['elements']['#attributes']) ? $variables['elements']['#attributes'] : array(),
as
'attributes' => isset($variables['elements']['#attributes']) ?: array()

?

Just a reroll for HEAD changes. Will follow up with a separate patch and interdiff that incorporates feedback.

This still needs a test added, but CNR for bot.

we need this same fix for title_attributes and content_attributes

I don't think so. There are no core use cases of a #title_attributes or #content_attributes property on render elements. It's possible that in some cases, like for 'block' in this patch, that we want to merge the #attributes of a child element into $variables['title_attributes'] or $variables['content_attributes'], but I think that's always hook-specific, not something that can be done generically in template_preprocess().

In manual testing of this patch, the 'data-edit-entity' attribute is output correctly for custom blocks, but I don't get a "Quick Edit" contextual link for them.

+  // When theming a render element, merge its #attributes into
+  // $variables['attributes'].
+  $hooks = theme_get_registry(FALSE);
+  if (isset($hooks[$hook]['render element'])) {
+    $key = $hooks[$hook]['render element'];
+    if (isset($variables[$key]['#attributes'])) {
+      $variables['attributes'] = NestedArray::mergeDeep($variables['attributes'], $variables[$key]['#attributes']);
+    }
+  }

Please inject the $info into the preprocess function instead. This completely break encapsulation and is most likely very inefficient.

I mean, transform the signature of those functions (preprocess, process) into:

function xxxx(&$variables, $hook, $info) {
}

@Wim: $info is the name given to $hooks[$hook] inside theme(). This should be passed to the preprocess/process functions by theme(), instead of those functions breaking encapsulation by getting them from the global theme_get_registry().

@Damien Tournoud: I agree with you, but I think that's out of scope for this issue. This is copying the same pattern already being used in HEAD for contextual_preprocess(). Can we have a separate issue for changing the signature of preprocess functions?

Given that it's a simple and backward compatible change, we should do it in here. If you don't like the performance hit, contextual links you can disable, template_preprocess() you cannot.

Ok. I also removed the cloning of $hook, because the reasoning in the code comment for it was faulty. I added that code and comment in D7 in a moment of paranoia, but we don't institutionalize that level of distrust about the functions we call in any other Drupal code, and I didn't want to expand that antipattern to $info or treat $hook and $info differently in that regard.

Working on this at Drupalcon Portland contrib sprint. Filling out the Issue Summary.

#31: attributes_entity-1972514-31.patch queued for re-testing.

Looks nifty, thanks for the reroll. As far as I see this addresses concerns brought up above. Let's get it in!

Yep, looks sweet.

Needs reroll

curl https://drupal.org/files/attributes_entity-1972514-37.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  9992  100  9992    0     0   8484      0  0:00:01  0:00:01 --:--:--  9952
error: patch failed: core/modules/edit/edit.module:14
error: core/modules/edit/edit.module: patch does not apply

Assigning to self to reroll patch for mentoring office hours.

Here is a quick reroll. The patch still applied with some fuzz.

@ekl1773: ups, sorry for stepping on toes! Was not intended.

Added the twig template from patch at #37 to patch at #42. Maybe this works?

Thanks. Doing a diff of #37 vs. #46 shows no lines of code that differ, so back to RTBC.

By the way, no need for a "Needs review" tag, since we have that as a status. Tags are useful for things not captured by the other issue attributes.

Committed 71a4ab1 and pushed to 8.x. Thanks!

+++ b/core/includes/theme.incundefined
@@ -2582,11 +2581,16 @@ function template_preprocess(&$variables, $hook) {
+  // When theming a render element, merge its #attributes into
+  // $variables['attributes'].
+  if (isset($info['render element'])) {
+    $key = $info['render element'];
+    if (isset($variables[$key]['#attributes'])) {
+      $variables['attributes'] = NestedArray::mergeDeep($variables['attributes'], $variables[$key]['#attributes']);
+    }
+  }

I think requires a change notice to describe this functionality...

Small follow-up to fix call to template_preprocess() in theme():
#2030457: Warning: Missing argument 3 for template_preprocess() for theme functions overridden by templates

Updated Issue Summary.