I had an issue with a context having two conditions:
- Content type (ctools): one content type selected
- Vocabulary (ctools): one vocabulary selected
In the node pages of the content type selected, the first condition is being met and the context fires correctly, but in the taxonomy term page of a term of the selected vocabulary, the context doesn't fires.
I debugged and noticed that in the taxonomy term page i was having a MissingValueContextException applying the context mapping to the "content type" condition, resulting in returning FALSE without applying contexts to he rest of conditions.
# modules/contrib/context/src/ContextManager.php
protected function applyContexts(ConditionPluginCollection &$conditions) {
foreach ($conditions as $condition) {
if ($condition instanceof ContextAwarePluginInterface) {
try {
$contexts = $this->contextRepository->getRuntimeContexts(array_values($condition->getContextMapping()));
$this->contextHandler->applyContextMapping($condition, $contexts);
}
catch (ContextException $e) {
return FALSE;
}
}
}
return TRUE;
}
I don't know exactly why applying context mapping to the "content type" condition is failing, but i think that that doesn't have to abort applynig contexts to the other conditions. Indeed, when i replaced the return FALSE; with a continue; the context fired and everything worked ok in the taxonomy term page
¿Anybody with a better Drupal insights understanding can review this?
Thanks!
Note: I applied a patch for this issue in order to have the Taxonomy ter route context, it may be neccesary to reproduce #2998826: Term route context.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | remaining_conditions_not_being_evaluated-3045666-10.patch | 824 bytes | loze |
| #8 | 3045666-8.patch | 752 bytes | bircher |
| #5 | remaining_conditions_not_being_evaluated-3045666-5.patch | 393 bytes | bzoks |
| #3 | remaining_conditions_not_being_evaluated-3045666-3.patch | 544 bytes | vidorado |
Comments
Comment #2
vidorado commentedComment #3
vidorado commentedComment #4
vidorado commentedComment #5
bzoks commentedPatch #3 was not created in module root path, so I had to recreate it to be able to apply it.
It works correctly, tested on 8.x-4.x-dev on Drupal 8.8.
Comment #6
nace_fr commentedTested the patch and it works as expected. Changing status to "Reviewed".
Comment #7
ronaldmulero commentedThanks @vidorado
Patch
#3#5 works for me to fix Vocabulary (ctools) context Conditional only when combined with Patch #38 from https://www.drupal.org/project/drupal/issues/2998826#comment-13385527Drupal core 8.8.1
Context 8.x-4.0-beta2
Comment #8
bircherThe patch from #5 Makes the method always return
TRUE. Even though the return value is not used anywhere the in this module the method is not private so we should maintain the same return value.Attached is patch that solves the problem but also still returns
FALSEwhen one of the contexts has a missing dependency etc.Comment #9
loze commentedThe patch in #5 worked for me. The one in #8 did not.
Comment #10
loze commentedHere is what I ended up needing to get this working.
Comment #12
loze commentedComment #13
deaom commentedI can confirm that the patch under #10 is working, the patch under #5 did always return true, which is now corrected with the patch #10. And the patch under #8 did not work as it was returning FALSE if one of the conditions failed, which meant public function evaluateContextConditions(ContextInterface $context) returned false. So marking this as RTBC.
Comment #14
paulocsThanks! I'll fix on dev branch.
Comment #16
paulocsComment #18
bircherI think the patch from #10 is not a good solution.
It will return true if the last context doesn't throw an exception and false if the last context throws an exception.
If returning false when one of the contexts throws an exception doesn't work, then maybe we should return true if at least one of them doesn't throw an exception.
So false if all throw an exception and true if one of them in the loop doesn't.
Comment #19
birchersorry for the noise!
It does what I suggest already.
Comment #21
jonmcl commentedChanging version to dev since that is where the patch was committed. It is not yet available in beta2 (or beta5)