Block css classes not being applied to DS Field

Hi guys,

Following up with the issue #2328103: Block class not working when used with Display Suite reported in the Block Class module issue tracker, where a user encountered the following problem:

Block custom css classes are not being applied to block container when used with Display Suite Fields.

For those who wouldn't know what the Block Class module does, basically, it adds a column to the Core Block DB table to store extra custom CSS classes. These classes get added to the $classes_array in [MODULE]_preprocess_block, when the block is being rendered.

Symptoms diagnosis:
Alright, so I have tested and managed reproducing the issue:
I installed Block Class and Display Suite (dev versions) normally, added CSS classes to the Powered by Drupal block and added it as a DS Block field. Then I went to configure the Page content type full content view mode and published the Powered by DS block field.
Indeed, when displaying the page with the Default DS Block field Layout, Block's container would display correctly but not the CSS classes that were added through Block Class.

Note that there are currently 3 Layout types available for displaying the DS block field:

• Default
• Show block title + content
• Show only block content

Default is actually the only Layout that doesn't get completely restyled by DS, in other words, it still keeps the original block markup (displaying div tags, contextual links, etc... so just like any other standard block displayed in a region). Selecting anything else other than Default will not get any markup or styling that could be obtained with the display of any standard block.

In other words, what I'm saying is that Show block title + content and Show only block content are probably lost causes (to support Block Class), that could only be controlled by DS, not even the Block theme, so we will try focusing on fixing the issue for Default only.

Problem analysis:
Now, obviously, it appears that when DS renders the block through one of its fields... some things might be missing, such as the css_class property, added by Block Class.
Currently, it appears the DS Block Field rendering process goes through the function ds_render_block_field($field), see ds.module, line 877.

If you look carefully at this function, you may see that DS actually reloads completely the full block (SELECT SQL Query, line 906) to fill in some missing properties from its invocation of the block_view hook, line 880.
So $block['subject'] and $block['content'] get filled in, then entering in the Default block rendering layout (DS_BLOCK_TEMPLATE, line 945), more properties get filled, such as region, module or delta, but no other property is provided.... therefore, the data from the css_class field for the block, which is present in the $full_block object, is lost and can't be passed on to the theme.
For those interested in this issue and would like to look at this closer, I would suggest you try comparing the dpms of the 2 objects: $full_block and $block, after line 948.

Now, this appears to be a rather complicated problem, unrelated with Block Class (which doesn't seem to do anything wrong in this case), to be fixed in DS.

Potential solutions:
As a potential general solution, I would suggest that perhaps the $full_block and $block arrays could be merged at some point. This would ensure that any other properties added by third party modules would be able to make it through to the theme in the rendering process.
Please find attached to this comment a patch against ds-7.x-2.x at 1640acd, which merges $full_block in $block.
File attached as: ds-7.x-2.x-merge-block-with-full-block-2327879-1.patch.

This approach and patch should provide a more general solution, not only for the Block Class module, but also for any other module that would be adding properties in the same way.
However, given the amount of changes for this patch and modification of the $block array, the approach would need to be discussed with module's maintainers and the patch definitely further tested and reviewed.

As an immediate solution for @yalegria's needs, I have attached to this comment another patch (shorter) against ds-7.x-2.x at 1640acd, that adds the css_class property to the renderable $block object. But this solution is very specific and I guess it would be more satisfying to find a more generic way of solving the problem for any module adding properties to blocks.
However, this patch is certainly harmless, safe and works as expected.
File attached as: ds-7.x-2.x-block_class-compatibility-2327879-1.patch.

I would greatly appreciate to have more feedbacks, comments, reviews, testing and reporting on any of the attached patches (generic solution, fix Block Class) or the suggested more generic approach.

Feel free to let me know if you would have any questions, comments, objections, ideas, concerns or suggestions about the code changes, the attached patches, suggested approach or any other aspects discussed in this ticket, I would be glad to explain in more details.

Thanks in advance to everyone for your testing, reporting, reviews, comments and feedbacks.
Cheers!

Hi @yalegria,

Thank you very much for your prompt reply and taking some more time to investigate the issue.

Actually, the reason why DS is working with block_class-7.x-1.x is because Block Class will make an additional DB query to get the block classes at the theme level, which is part of the most important changes in the 7.x-2.x branch, see #1868586: Alternate structural approach: modifying the block table?: this additional DB query was removed, the core block DB table altered to load the css classes at the same time the block is loaded.
See the code of block_class-7.x-1.3, in block_class.module, line 63, more particularly the function block_class($block) which contains the query.

However, in terms of how DS handles the rendering of the block, the issue stays the same whether 7.x-1.x or 7.x-2.x.
In other words, unfortunately, case 1 and case 2 don't really lead anywhere or bring any tangible evidence that could help resolving the problem.

I think I have already described the issue at length, from a coding stand point and it would actually greatly help if you could perhaps try testing the patches from #1 and report back.

Ideally, it would be great if we could also stick with modules' latest versions and if you could test with the dev versions it would be even better.

Any more comments, questions, ideas, suggestions, or concerns would certainly be greatly appreciated.
Once again thank you very much for your feedback, comments, testing and reporting.
Cheers!

Hi @yalegria,

Sorry for this late reply, but I have been kept very busy working on other modules in the past few weeks.

I wanted to thank you very much for your feedback, testing and reporting on this issue, especially for the patches.

I definitely understand your point, related to working with stable versions and your approach appeared much clearer once I understood you actually came from various upgrades of modules through different branches. I would assume it would still be possible for you to have the patches applied to the stable DS version of the 7.x-2.x branch, but it would have to be manually.

Now, what is going to happen with these patches in the future greatly depends on the feedback from the DS users' community and maintainers. As a heads up, the DS Block field rendering process (specifically the function ds_render_block_field) seems to be undergoing a major (if not complete) rewrite, see the patch at #1936128-36: Block field Doesn't respect block visibility settings. Once this patch is committed, maybe this issue will become obsolete (this problem might not exist anymore), or maybe the patches will have to be completely rewritten... We will have to wait and see what happens there first.

In any case, thanks again very much for your help, testing/reporting and detailed feedback, including versions, it is certainly greatly appreciated.

Thanks in advance to everyone for your testing, reporting, reviews, comments and feedbacks.
Cheers!

The second patch in #1 still applies, and still does what it's supposed to do -- making DS compatible with the Block Class module.

@DYdave I like the idea of your first general solution, to merge the blocks, as done in the first patch in #1. But as this issue has sat in the queue for nearly two years without even any feedback from the module maintainers, I'm hoping we can at least get some attention and movement to fix the specific issue with the smaller patch.

Wouldn't it be better if Display Suite used the _block_render_blocks function to render the blocks instead of doing a lot of code itself?

This still appears to be an issue in Drupal 8

Here's a stab at a fix. The problem seems to be coming from the fact that the block is not being passed through as a proper render array. Taking a leaf out of the block_field module, I've implemented something similar to what they do and it seems to fix the issue. I've attached some comparison markup with twig debug so it's easier to see what's happening.

Your patch looks good but I wonder how many sites it will destroy cause of the extra wrappers...

@aspilicious yeah, I agree, would probably need a version bump with some release notes. Not sure it would even be viable for 3.x...

We need a new checkbox on the block field config pages "Add block wrappers and classes" and disable it by default.

Alsof need a test before I can commit this.

Added checkbox to BlockFieldForm and added tests.

Awesome!! Looks great! I'll review this again in a few days!

See how fast things can go with the proper patch :)

Is the D7 patch supposed to make it into the contrib module at some stage? Spent some time debugging today until I hit this page here. :-)

I applied the patch from # 1 manually, but the concept still applies to latest versions of DS and Block Class.

Thank you.