The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
Blocks incorrectly not displaying after upgrade from 2.0.3 to 2.0.4
Steps to reproduce
Set up a block with a block visibility group having 2 separate mutually exclusive conditions where only 1 must pass (ie. one condition based on content-type and one condition based on vocabulary type).
Block will not display when one condition passes because the other condition has a null context.
Proposed resolution
Check that $logic = 'and' before returning false from applyContexts() when context data is null for one condition.
Current code
Suggested code:
protected function applyContexts(ConditionPluginCollection &$conditions, $logic) {
$have_1_testable_condition = FALSE;
foreach ($conditions as $id => $condition) {
if ($condition instanceof ContextAwarePluginInterface) {
try {
$contexts = $this->contextRepository->getRuntimeContexts(array_values($condition->getContextMapping()));
// Skip when any of the contexts is not set.
$missingContextData = FALSE;
foreach ($contexts as $context) {
if ($context->getContextData()->getValue() === NULL) {
if ($logic == 'and') {
return FALSE;
}
else {
$missingContextData = TRUE;
break;
}
}
}
if ($missingContextData) {
continue;
}
$this->contextHandler->applyContextMapping($condition, $contexts);
$have_1_testable_condition = TRUE;
}
catch (ContextException $e) {
// Log a message about the error.
// \Drupal::logger('block_visibility_groups')->error($e);
// If the condition is negated, shouldn't refuse so quickly.
if ($logic == 'and' && !$condition->isNegated()) {
// Logic is all and found condition with contextException.
return FALSE;
}
$conditions->removeInstanceId($id);
}
}
else {
$have_1_testable_condition = TRUE;
}
}
if ($logic == 'or' && !$have_1_testable_condition) {
return FALSE;
}
return TRUE;
}
| Comment | File | Size | Author |
|---|---|---|---|
| #29 | or-logic-when-context-missing--3503194-29.patch | 3.22 KB | falco010 |
| #5 | or-logic-when-context-missing--3503194-5.patch | 1018 bytes | cobenash |
| Screenshot 2025-01-29 at 1.49.31 PM.png | 236.44 KB | debra-v |
Issue fork block_visibility_groups-3503194
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:
changes
Comments
Comment #2
debra-v commentedComment #3
debra-v commentedComment #4
damienmckennaThank you for reporting the bug, I'm sorry we left that bug in the module and didn't catch it.
FYI the correct status for a new issue is "Active"; "needs work" is for when a solution has been proposed via a patch or merge request but it needs further work.
Comment #5
cobenashHey Folks,
I have the same issue. My environment is as follows:
* Drupal 10.4.2
* Block visibility group 2.0.4
Thanks for the code provided by @debra-v
It works on my side, and I uploaded a patch regarding the proposed code.
So I changed the status from active to needs review. :)
Comment #6
proweb.ua commented#5 works
Comment #7
ccjjmartin commented#5 fixes broken logic conditions for me, +1 RTBC
Comment #8
joelpittetThe code differs significantly from the 2.1.x branch. Adding tests would help ensure this bug is fixed in both branches.
@cobenash, @proweb.ua, @ccjjmartin – would any of you be able to write a test to prevent regressions? A merge request would be preferred, but I appreciate the patch!
Comment #11
b_sharpe commentedMoved to MR, edited existing test to use two different contexts so that the AND/OR conditions are properly evaluated / tested.
Test fails without patch: https://git.drupalcode.org/issue/block_visibility_groups-3503194/-/jobs/...
Test passes with patch: https://git.drupalcode.org/issue/block_visibility_groups-3503194/-/jobs/...
Comment #12
joelpittetWow, that was quick! Were you already working on the test, @b_sharpe?
I always hesitate to modify an existing test instead of adding a new one since there’s a risk of losing valuable coverage. It’s a bit tricky to tell if anything important is lost here, but seeing the red/green tests is definitely encouraging! Can you alleviate that worry?
Comment #13
b_sharpe commentedHa, was actually just coincidence, I needed this to fix a prod instance today.
I would normally agree about a new test over editing; however, in this case the edit made more sense as the old test is for testing both AND/OR (
testMultipleConditions()) and would have caught this issue in the first place had there been more than one context so I figured the edit made more sense.Comment #14
ccjjmartin commentedThis issue is attempting to fix the broken logic in the 2.0.4 version of the module, which I can verify is a real bug. However, the proposed change here will be in conflict with the direction taken in the 2.1.x version of the module, see this issue: https://www.drupal.org/project/block_visibility_groups/issues/2864027
There is a backport patch to the 2.0.x version of the module available there that modifies the same lines of code the proposed patch here is modifying. I believe the other patch fixes this issue as well and knowing it was already merged in the 2.1.x version, that in theory is the direction the maintainers was to head. Ideally someone who understands the code in this patch would review the affected code in the already merged branch to verify if the issue here was fixed or not.
Comment #15
twodNot sure why that shows as user 0...Never mind, it fixed itself.I tested this with a condition that has two optional contexts, and if one or more of them didn't have a value it prevented the block from being displayed, which kinda defeats the purpose of the contexts being set as not required...
Maybe instead of FALSE it gets set to the inverse of the context definition's required flag?
Comment #16
greg boggsI can confirm, this bug made all my blocks disappear, hrm, never mind, still debugging. Could be a bug from the content of the blocks.Comment #17
greg boggsOk figured it out.
Once updating to the current release, I have a VG of
path = /example*
Path = /example/search (negated)
Content type = page (negated)
With content type = page (negated) is on the VG, the group does not display when it should on any page or content type or route.
If I use only path based rules, the VG displays as expected. Not sure it's related to this exactly.
Comment #18
greg boggsI ended up just switching entirely to path based rules, but found another bug.
/example* works
/example/* works
/example/*/* fails
Comment #19
eddylbsThanks !
MR !26 works for me on Block Visibility Groups 2.0.4 and Drupal core 10.4.7
Comment #20
falco010We had the same issues after updating this module to 2.0.4
The MR !26 solves it. Also bumping priority to Major as the latest release is breaking existing functionality.
Comment #21
liampower commentedI was facing this issue with webforms losing their page titles (Matched by URL pattern).
Config of the block visibility group had 2 conditions
The configuration for URL patterns was no longer working, after applying the patch from MR !26 it works again. Thanks!
Comment #22
damienmckennaThis needs to be rerolled for the 2.1.x branch.
Comment #26
damienmckennaI copied the test coverage to a new MR for 2.1.x, let's see how the tests perform.
Comment #27
damienmckennaThe tests currently pass with the test changes on their own. This suggests that the problem doesn't exist in the 2.1.x branch. Can someone please test out whether the problem still happens on their sites with the current 2.1.x branch? Thank you.
Comment #28
debra-v commentedI just tried to install the 2.1.x-dev branch to test and it's throwing errors in the block_visibility_groups_admin module
Comment #29
falco010Attached is a patch from the MR that can be added via Composer without including any further changes from the MR.
Comment #30
hitchshockI'm facing this issue on the User page.
Steps to reproduce:
- make the block that must be available for a certain content type and the user page (/user/*)
- configure the module:
- Only one condition must pass
- add Request Path condition
- add Inhoudstype condition
- Expected behaviour: block is available on the content page and user page
- Actual behaviour: block is available only on the content page
So the issue exists on 2.0. Also, I can confirm that the test in the MR fails without the fix on 2.0
But... Since the test passes for 2.1, then probably it won't be an issue there, and the root cause is fixed.
So, my suggestion is:
- merge the updated tests into 2.1
- to do a backport of the fix from 2.1 -> 2.0, cuz the issue is really a critical one
Comment #33
joelpittetCommitted thank you all!
2.0.x: fix + test coverage (MR !26)2.1.x: tests only (MR !41). The root cause was already addressed there by #2864027: Allow choosing more than one visibility group per block; the new tests confirm it and guard against regression.Your examples became the test suite almost one-for-one:
content type OR path(@hitchshock, @liampower), content type OR vocabulary (@debra-v), and the negated condition case (@greg-boggs). Thanks to @b_sharpe for the MR, @cobenash and @falco010 for patches, @damienmckenna for the 2.1.x reroll, @ccjjmartin for the #2864027: Allow choosing more than one visibility group per block pointer that stopped us double-fixing, and @proweb.ua, @eddylbs and @twod for testing and review.Still open: @twod's point in #15 about optional contexts being treated as required. That deserves its own issue rather than riding along here ... I'll leave this one for someone to file. And the 2.1.x branch has some fun errors that I need to dig into... and some random test failures this evening were quite a pain... but the 2.0.x merge 🚂 passed!