The code in the _block_rehash() function is capable of disabling all blocks for a particular theme, if it ever tries to rehash blocks for a theme it doesn't recognize.
We have seen this happen occasionally on some Drupal 7 sites.
Although I'm trying to look a little deeper into the root cause of the bug, it also occurred to me that even inside _block_rehash(), there is perhaps no good reason ever to do this. The intention of the code that disables the blocks is that if (for example) a theme is changed so that it no longer contains certain regions, the blocks that were in that region should be disabled so they can be placed somewhere else. That's all fine, but a theme without any regions doesn't make sense, so if _block_rehash() comes across a theme that appears to be in that situation, it seems reasonable for it to catch that case and not blindly disable every single one of the theme's blocks.
The attached patch makes that change.
Comment | File | Size | Author |
---|---|---|---|
#13 | prevent-disable-all-blocks-theme-925360-13-D7-do-not-test.patch | 1.25 KB | roderik |
#7 | prevent-disable-all-blocks-925360-7.patch | 1.25 KB | dcam |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedLooks ok
Comment #2
effulgentsia CreditAttribution: effulgentsia commentedThe patch looks reasonable, and I don't believe it would hurt HEAD, so no harm in committing it. But I don't like the mystery of it all. What actually causes $regions to be empty? A theme .info file without any 'regions' lines would do it, but like you said, that's a rare condition, so I doubt that's what led to this being observed on real sites. I'd rather a test be created that identifies a real condition of this occurring. Perhaps we need to exit out of the function earlier if we're passed a questionable $theme variable?
Alternate wording suggestion:
A block assigned to a region that isn't implemented by the theme is effectively disabled, so set the status accordingly, rather than leaving it orphaned. This commonly occurs during development, when a theme is updated to no longer implement a region that it used to. If the theme doesn't implement any regions, it's indicative of some error, such as a corrupt .info file. In this case, do not disable every block.
Bonus if we also dsm() and/or watchdog() the condition.
Powered by Dreditor.
Comment #3
effulgentsia CreditAttribution: effulgentsia commentedComment #4
David_Rothstein CreditAttribution: David_Rothstein commentedIn my opinion the root cause is probably #925490: list_themes() should not call file_exists() on each theme's info file which would be difficult to test.
Comment #5
nagba CreditAttribution: nagba commentedRerolling the patch against Drupal 7.16-dev.
Comment #6
effulgentsia CreditAttribution: effulgentsia commentedThanks, but bug fixes need to get into the version of Drupal that's in development first, then backported to released versions. Otherwise, it's too easy to forget to forward port, and then we end up with regressions when people upgrade from 7 to 8.
Comment #7
dcam CreditAttribution: dcam commentedPorted #5 to D8.
Comment #8
dcam CreditAttribution: dcam commented#7: prevent-disable-all-blocks-925360-7.patch queued for re-testing.
Comment #13
roderikFWIW: reroll for 7.56 (and 7.55). Only the diff context changed.
Also: I believe the backport tag stays on the issue until the D7 patch is actually committed.
Comment #14
dcam CreditAttribution: dcam commentedIt does stay. I wouldn't normally have removed that tag arbitrarily, even 5 years ago. It probably happened accidentally.
Comment #20
pameeela CreditAttribution: pameeela commentedI do recall seeing this in D7 at times but have never seen it in D8. Anyone know if it's still relevant?
Comment #23
quietone CreditAttribution: quietone at PreviousNext commented@David_Rothstein, Thank you for reporting this problem.
There has been no discussion of the problem stated in the IS for 5 years and no reports of this happening on Drupal 8+
If you are experiencing this problem on a supported version of Drupal, provide complete steps to reproduce the issue (starting from "Install Drupal core").
Since we need more information to move forward with this issue, I am keeping the status at Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #25
quietone CreditAttribution: quietone at PreviousNext commentedThere are no reports of this in Drupal 8+, so time to close this one.