Attributes of a block content are applied to block itself

This is by design. If you are using bartik theme's templates you can pass in #content_attributes like what D7 had. We decided to remove some divs.

Ah did a bit of research and see kinda what you are trying to explain. There is a specific comment to that effect.

core/modules/block/src/BlockViewBuilder.php:212

      // Place the $content returned by the block plugin into a 'content' child
      // element, as a way to allow the plugin to have complete control of its
      // properties and rendering (e.g., its own #theme) without conflicting
      // with the properties used above, or alternate ones used by alternate
      // block rendering approaches in contrib (e.g., Panels). However, the use
      // of a child element is an implementation detail of this particular block
      // rendering approach. Semantically, the content returned by the plugin
      // "is the" block, and in particular, #attributes and #contextual_links is
      // information about the *entire* block. Therefore, we must move these
      // properties from $content and merge them into the top-level element.

I think you are write and we really should have been using #wrapper_attributes, to effect that outer element from our block plugin build() render arrary I think.

Am I following you @ivanjaros?

Here's a quick patch to see how many test fail.

There is also a good chance this won't get into release considering it's next week. Don't want to get your hopes up @ivanjaros.

Let's see if a framework manager can chime in here.

This is especially bad for blocks that represent forms since none of the form attributes is preserved.

I was curious how this affected SearchBlock, and found that most form attributes, such as method and action are ok, because they're stored as #method and #action on the render array, rather than in #attributes. However, I did find that in HEAD, $form['#attributes']['data-drupal-selector'] ends up getting moved from the <form> tag to the block's <div> wrapper, per the attached screenshot.

The code in #4 was introduced in #2171269: Handle block rendering's #attributes correctly (fixes in-place editing of custom blocks), so adding it as a related issue, and I'll post a comment there asking for people who worked on that to comment here.

Prior to giving framework manager review on this, I'd like input from block.module maintainers, so tagging for that.

I am having this issue with trying to use theme item list. In my block class I have:

public function build() {

    return array(
      '#title' => 'Title',
      '#theme' => 'item_list',
      '#items' => $items,
      '#attributes' => array(
        'class' => array('class'),
      ),
    );
  }

Instead of the class feeding through to the list it is being applied to the block. I am unsure on how to get around this to instead have the class applied to the list instead of the block. Will give the patch above in #5 a go however I am now up to Drupal 8.3 so I don't know if it is still going to apply.

Ah thanks! That worked perfectly.

public function build() {
    return [[
      '#title' => 'Title',
      '#theme' => 'item_list',
      '#items' => $items,
      '#attributes' => array(
        'class' => array('class'),
      ),
    ]];
}

I agree this is a bug. And at first I wasn't sure how best to fix it without breaking BC.
Except that render arrays are exempted from the BC policy!

I think a switch to #wrapper_attributes would be good.

Is anyone comfortable enough to provide a workaround for this while a fix isn't available?

I'm trying to add attributes to a form but they keep getting bubbled up to the block's div.

Hello,
I notice that the problem is still present, I have partially taken over the joelpittet patch. The latter solves the problem well

Sorry the comment #22 doesn't have the right file, it's #23

I've encountered the same issue on Drupal 9.0.14
Is there any working patch on Drupal 9 ?

Just pushed a new commit that applies the same fix to blocks placed via layout builder.

there doesn't appear to be anything to review the patch nor MR are passing tests.

And if this is a bug it will tests of it's own.

Is this a twig issue? Tried the MR and patch which both applied cleanly to 9.5.x but didn't do anything with the rendering.

Also is it really a bug ?

Current olivero block twig

<div{{ attributes.addClass(classes) }}>
  {{ title_prefix }}
  {% if label %}
    <h2{{ title_attributes.addClass('block__title') }}>{{ label }}</h2>
  {% endif %}
  {{ title_suffix }}
  {% block content %}
    <div{{ content_attributes.addClass('block__content') }}>
      {{ content }}
    </div>
  {% endblock %}
</div>

To match change in description

<div id="{{ attributes.id }}">
  <div{{ attributes.addClass(classes)|without('id') }}>
    {{ title_prefix }}
    {% if label %}
      <h2{{ title_attributes.addClass('block__title') }}>{{ label }}</h2>
    {% endif %}
    {{ title_suffix }}
    {% block content %}
      <div{{ content_attributes.addClass('block__content') }}>
        {{ content }}
      </div>
    {% endblock %}
  </div>
</div>

This is not a twig issue, and everyone agreed that this is an issue. Not sure what else is needed.

Maybe I don't fully understand the rendering but to get the output mentioned in the description don't you need to wrap in an additional div.

In an example block I'm using this is my build array

    return [
      '#attributes' => array(
        'class' => array(
          'foo-bar'
        ),
        'data-test' => 'This is just a test'
      ),
      '#markup' => 'This is block content'
    ];

With or without the patch I'm seeing class, id, and data-test appear on the same div. So maybe this needs an issue summary update.

I am running into this problem with exposed forms in blocks, but it only surfaced when I updated to D 9.5. I was using views_block_filter_block because my views were block displays and I needed the exposed filters in separate blocks. It seems like the D9 update superseded views_block_filter_block, changing where the attributes were attached.

I patched with the MR, but it doesn't seem to be working, exposed form class attributes are still being moved to the containing block. If you have a custom block template for blocks containing views exposed forms that don't output all of the attributes, it breaks the ajax as well as the CSS in my case.

I worked around the issue by refactoring my template structure, I was fortunate to find a relatively simple solution, unfortunate that it cost me half a day to discover the problem and fix it.

Based on comment #10 I checked to see if this is still an issue in Drupal 11. It is. The same problem with the Search block form's data-drupal-selector attribute being applied to the block wrapper still exists.

There was no need to make a new test for this. The test created for the related original issue, #2171269: Handle block rendering's #attributes correctly (fixes in-place editing of custom blocks), can be adapted to check the change.

MR 1402 was broken. I don't know why some of the additional changes in it were made because they aren't well documented. The change from #atrributes to #wrapper_attributes was lost. I think that's the reason for the confusion in the more recent comments. I'm sure that it stopped doing anything to fix the issue. Since it's difficult or impossible to rebase MRs for old Core versions anyway, I hid the branch and created a new one for D11 restarting from the #5 patch. We'll see if this causes other tests to fail.

I didn't expect it to pass. I assumed that something else was probably relying on the attributes as they were. Oh well.

I made a change record: https://www.drupal.org/node/3525287.

I worked on this issue over the weekend not realizing that I'd experience it first-hand two days later. I've been adding inline styles to the .views-exposed-form class. Then I navigated to a different page where the exposed form is in a separate block. The class was bubbled up to the block wrapper, causing the block heading to be displayed in-line with the form as shown in the image below.

This seems like it could be an immediate breaking change if I had css written for before.

I ran into this logic recently on another issue and the current behaviour is very confusing as #48 demonstrates. We can't assume that everything rendered via block module is never rendered in any other context - especially with layout builder / experience builder and etc.

If we aren’t concerned with #49 can add to my review list for tomorrow

I keep running into this error as I apply my new theme to more sites. But now I've had a WSoD crash because of it. I'm altering Views exposed forms to format them as USWDS Search components. It involves applying a few classes, changing the text input type to search, and setting role="search" on the form. For some reason, that last one, setting the role attribute, really caused a problem when the exposed form is in a block and that block has its title display on. template_preprocess_block() (this site is still in D10) tries to set a unique ID for the block with Html::getUniqueId(). When the role attribute is bubbled up to the block due to this issue, suddenly the preprocess function starts passing the title render array to getUniqueId(). Then the error is thrown because the expected input is a string, not an array.

I didn't bother trying to figure out what weird code path causes that to happen. I'm fortunate that I can simply turn the title display off in this one instance. I just want to point out that there are scenarios where this isn't harmless. I'm lucky that I knew about this issue so I could work out what was going on.

Sorry to take so long, rebased the MR and it's got pipeline failures.

Feel free to ping me and I'll try and get to it sooner.

With regards to my concern if no one else has it I'll ignore it.

I've seen that random Unit test error twice before on another issue. It's unrelated.

With regards to my concern if no one else has it I'll ignore it.

I'm biased because it's been directly impacting me. Maybe we need to tag it for framework or even release manager review.

Going to mark as this does fix the issue. Will tag but weird case I'm not sure needs framework manager sign off or not.

This needs frontend framework manager review rather than framework manager review.

If it was possible we'd probably want a bc layer for this in stable9 so that themes depending on that get attributes in the same way as they do now, but not sure how we'd implement that.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Post-bot-rebellion rebase

#57 needs addressing we've still not had a frontend framework manager review and a bc layer in stable9 would be good. I think we have to be concerned about breaking existing sites here.

I added backward compatibility to stable9. I even added a basic test for it.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

That works for me, CR is ok too.

OK this was a very tricky issue, the bc layer in stable9 and sign-off from @nod_ makes me a lot happier about committing it, thanks for sticking with it for 11 years everyone.

Committed/pushed to main and cherry-picked to 11.x, thanks!

Reword the CR to put the change first and publish.