Problem/Motivation

We recently noticed that block_class is creating dependencies on itself even if no class has been configured on a block.

While this is not necessarily an immediate problem, if the module is uninstalled in the future, any block configs with a dependency on block_class (valid or not) will be deleted. Thus blocks that don't actually have a block class, id or attribute configured will be deleted.

Steps to reproduce

  1. Open a block configuration form for any placed block that does not have a block class, id or attribute configured.
  2. Click 'Save block' without making any changes.
  3. Export configuration and note that the block config now has a dependency on block_class (example below), even though no class was set.
  4. Visit '/admin/modules/uninstall', choose Block Class and click 'Uninstall' — the block above will be listed in the configuration to be deleted if you proceed with the uninstall.

Below is a 'Powered by Drupal' block config created with the steps above. I simply opened the block config form and clicked 'Save block' — you can see that dependency on block_class was created, but no class or other block_class settings were recorded.

langcode: en
status: true
dependencies:
  module:
    - block_class
    - system
  theme:
    - olivero
_core:
  default_config_hash: eYL19CLDyinGTWYQfBD1DswWzglEotE_kHnHx3AxTXM
id: olivero_powered
theme: olivero
region: footer_bottom
weight: 0
provider: null
plugin: system_powered_by_block
settings:
  id: system_powered_by_block
  label: 'Powered by Drupal'
  label_display: '0'
  provider: system
visibility: {  }

Proposed resolution

  • ✅ Recalculate dependencies in the blockClassPreSave() to ensure empty values do not create invalid dependency.
  • ✅ Include an update hook to remove exisiting invalid dependencies.

Remaining tasks

  • ✅ Update/patch
  • Review and test
  • Merge
  • Release

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork block_class-3419337

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

justcaldwell created an issue. See original summary.

renatog’s picture

renatog commented

Definitely a good catch! Thanks for reporting that @justcaldwell

justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented

Glad to help. I don't have time to dig too deep, but it looks to me like line 358 of blockClassPreSave will always execute and save config, even if all block_class config is empty. Maybe that's where the dependency is created? EDIT: Disregard. That is module config getting saved, not block-specific config.

justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented
Issue summary: View changes
pooja saraah’s picture

pooja saraah commented

One approach to resolve this could be to add a condition before saving the configuration to check if any `block_class` settings are present. If not, we could skip the save operation to prevent unnecessary dependencies.

justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented

Agreed @pooja saraah, but it seems like the current code is already attempting to do that. At the top of BlockClassHelperService::blockClassPreSave(), each possible setting is checked and unset if empty. I'm not sure why that doesn't address the issue.

justcaldwell opened merge request !31

justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented
Priority: Major » Normal
Status: Active » Needs review

Okay, seems the issue is:

  1. blockClassFormValidate() is setting ThirdPartySettings for block_class, even if the values are empty (e.g., no classes or id were set). This creates the potential for a dependency.
  2. Core's Block::preSave() method runs and calculates dependencies, creating the dependency on block_class based on the (possibly empty) settings above.
  3. blockClassPreSave() unsets any empty values (see #6), BUT it's executed by a hook after the preSave method, and the dependencies don't get recalculated.

We can either 1) refactor blockClassFormValidate() to avoid setting empty values, or 2) just recalculate the dependencies in the pre-save hook. The MR implements #2 as it's the simplest, and includes an update hook to remove any invalid values.

This wasn't an issue previously. I don't know what changed to make it start occurring now.

justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented
Issue summary: View changes

Updating IS.

justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented
Version: 2.0.x-dev » 4.0.x-dev
Status: Needs review » Needs work

This is still an issue in 4.x.

The fix proposed in the MR still works in 4.x. The MR will need a re-roll for the update hook.

dydave made their first commit to this issue’s fork.

dydave’s picture

dydave commented
Status: Needs work » Needs review

Thanks a lot Michael (@justcaldwell) for the great work on this!

I've noticed the problem a while ago on a project and never actually took the time to look into the necessary changes.

Just rebased the merge request and fixed a conflict with the number of the hook_install.

I'll give this MR a round of manual testing in priority so it could hopefully get merged very soon. 👌

Back to Needs review for now.

Thanks in advance! 🙂

  • dydave committed 2eb6e253 on 4.0.x authored by justcaldwell 
    Issue #3419337 by justcaldwell, dydave: Prevent and remove invalid '...
dydave’s picture

dydave commented
Status: Needs review » Fixed

Great job Michael (@justcaldwell), as usual! 🙏
Thanks a lot for taking the time to look into this issue and document the logic so clearly with the different possible options.

Sorry for the late reply on this, but I had no idea nobody was actually maintaining this module at the moment...

I'm not very familiar with the 4.x code branch and features yet, but from what I've seen I would definitely agree there would be a lot of work refactoring, cleaning, reorganizing code files, etc...

At least the module has a base of PHPUNIT tests, so this should greatly help with any future refactoring or evolutions of its existing logic.
 

However, for the time being, refactoring the module does not seem to be in the list of priorities: 😅
There are still urgent issues to be addressed in the queue, to stabilize certain features or for Drupal Core compatibility.

Therefore, going with the least disruptive changes (solution 2 from #8) for now seems like a very acceptable compromise.👍

I actually did a manual round of testing of this locally:
1 - Enabled the module on 4.0.x and saved a block without classes and a block with classes, then 'drush cex'.
The dependency was there for the first block, although I had submitted the form with no change.
2 - Switched to repo branch 2.0.x (from the MR), ran 'drush updb', then 'drush cex'
==> The dependency in the first block was removed properly.

I then tried saving several blocks with and without classes and the dependency seemed to be updated as expected each time.

After updating slightly the inline comment in the merge request, based on comment #8, I went ahead and merged the changes above at #13. 🥳
This is a great step towards improving the quality of the configuration storage of the module!

Lastly, I have created a follow-up ticket to circle back on this issue if any refactoring of the module is planned at a later point:
#3555504: Refactor blockClassFormValidate() to avoid setting empty values

Once again, always a great pleasure working with you Michael! 🙏
Let us know if you spot anything else in the module, or would have any advice or ideas of improvement, your help and feedback would surely be greatly appreciated!

Thanks in advance! 😊

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented

Happy to help @dydave, and I'm glad to see you've been added as a maintainer!

dydave’s picture

dydave commented

Ha! 😅

I've been a maintainer since 2013 😅 (#1922252: Request to take over maintainership/co-maintainership for Block Class) .... But had no clue nobody was actually looking at this module anymore 😅
I rewrote the module entirely for the D7-2.x branch... which worked great for years and years.

I've kept maintaining the 3.0.x branch, legacy from D7.... but have not really been involved in the latest 4.0.x developments of which I'm personally not a big fan 😅 (It's doing too many things and should be broken down into smaller pieces/sub-modules)...

I'm still quite reluctant to use the 4.0.x branch in our projects and feel it's more of an alpha or beta than a real stable ...

I'll keep maintaining the 3.0.x branch in any case and try answering tickets for the 4.0.x branch as they come, but I'm not really planning on a full rewrite of the 4.0.x branch ... use at your own risks 😅

Thanks again very much Michael for all your great help with these modules! 🙏

justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented

the latest 4.0.x developments of which I'm personally not a big fan 😅 (It's doing too many things and should be broken down into smaller pieces/sub-modules)...

I 100% agree with you on that, and it's good to know your thinking! Unfortunately, we moved to 4.0 without realizing how significant the differences are. I'll probably look to move us back to 3.0 when I have a bit of time on my hands.

dydave’s picture

dydave commented

Super nice feedback Michael (@justcaldwell), once again! 🙏

Glad to hear I'm not the only one finding the 4.0.x branch a bit too much, as it is currently 😅

We're trying to clarify a bit the different versions in:
#3367869-5: Update Project description, clarify difference between version 3 and 4
(See the part about stable vs experimental and details on tests coverage, etc... 3.x vs 4.x)

I've done a big clean-up already of the Project page, removed all the D7 stuffs, added several sections, TOC, etc...

But I'll keep updating this week, the section about versions:
https://www.drupal.org/project/block_class#matrix
to provide more information on the different branches 👌

I'll probably look to move us back to 3.0 when I have a bit of time on my hands.

It would be awesome if you could share that with us 🙏
I think it might be of interest to other users as well 😅

Just to give you an idea, see:
https://www.drupal.org/project/usage/block_class
A reasonable amount of users still think it is enough to stick with an upgrade of the D7 branch/legacy features and use the 3.0.x releases (October 26, 2025: 9K reported installs).... which is currently also our case in our projects. 😅

We'll try improving Project page's documentation as much as possible, and see with time, if we could work on tickets to improve 4.0.x 👌

In any case, we're certainly very happy we can count on your help keeping module's code base well maintained.
Thanks again very much! 😊

ressa’s picture

ressa
he/him
commented

Thanks for working on a solution for this task @justcaldwell and @dydave! I can confirm this bug.

I only use the Block Class module to add classes, so version 3 would work well for me, and after understanding the current V3 vs. V4 situation, I decided to downgrade manually from version 4 to version 3. Basically just uninstalling, and reconfiguring all blocks.

I hadn't taken a closer look at this issue, so did not understand what it was about. So I ran into an "interesting" situation, where some blocks without any Block Class configuration got deleted, because they had a dependency -- for some strange reason -- on the Block Class module.

I corrected it manually, by restoring everything, deleting the erroneous occurrences of - block_class in the innocent block config-files, and imported the configuration. And then I could uninstall the module, and proceed with the process, and am now on version 3 :)

dydave’s picture

dydave commented

Great choice @ressa! 👍

I mean, in any case, if all you need is the ability to add custom CSS classes to blocks, you should definitely try to stick with the 3.0.x releases. The code has not changed much for over a decade making developers happy with the simple CSS classes feature. 😊

The module is so small and simple that pretty much any developer should be able to maintain it properly, for Drupal core compatibility upgrades, or API changes, for example. So the chances for this piece of code to sustain over a long period of time are probably much higher than something with lots of features or bells and whistles (AJAX autocomplete and such 😅).
Additionally, the UI (added field) and features are super simple to understand allowing any type of user (front/back developer, content editor, etc...) to immediately understand everything the module does.

Thanks for taking the time to document the steps you followed to restore/roll-back your block config and downgrade to 3.0.x.
Hopefully this will help others who might encounter the same type of issues with their configuration, until they are able to upgrade to the upcoming 4.0.2 release which should include this patch 👌
(No more risk of losing your config 😅)

Thanks again everyone for the great help keeping this module well maintained! 🙏

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.