Thank you @berdir!
I understand we cannot undo released changes, but I was thinking of doing a new release so that people who already faced this problem can resolve it by updating, and to ensure other Drupal 9 projects do not face this problem.
We can remove the line from composer.json, it should work, but what about the change in the info file? will we keep Drupal 9 unsupported?
We're trying our best to upgrade our project, but for some reasons we can't do it now, but thanks for mentioning it.
Uploading a static patch file to use with composer-patches...
Based on the changes in the below commit, I think the correct version requirement should be ^9.3 || ^10.1 || ^11
.
https://git.drupalcode.org/project/ultimate_cron/-/commit/c003090f0c7352...
I also updated drush versions in composer.json based on Drupal X Drush compatibility table .
redwan jamous → created an issue.
Also, this wouldn't affect 7.0.x
since processDate()
method was removed in Drupal 10. I didn't test it though.
Reopening as I believe the
linked issue
🐛
Warning: Undefined array key "#type" in Drupal\\better_exposed_filters\\Plugin\\better_exposed_filters\\filterDatePickers->exposedFormAlter() (line 65 of /var/www/html/web/modules/contrib/better_exposed_filters/src/Plugin/better_exposed_filters/filter/Dat
Active
was originally referring to a different warning (Undefined array key "#type"
not "type"
) which I did not encounter btw.
I was able to confirm that this was caused by Remove dependency on jquery_ui_datepicker 📌 Remove dependency on jquery_ui_datepicker Needs work .
In the above issue, we started setting '#type' = 'date'
, doing so will run the processDate()
method since it's a process function defined in core/lib/Drupal/Core/Render/Element/Date.php::getInfo()
.
However, since the info provided by that method is altered by DatePickers::exposedFormAlter()
and because we're manipulating the '#attributes'
key, the default $element['#attributes']['type'] = ['date']
that is defined in getInfo()
is lost.
Setting it whenever we set the '#type'
to 'date'
fixes the issue.
I understand that we may not be interested in fixing this in 6.0.x
, but I believe we should since we commited the reason behind this issue to 6.0.x
.
I'm uploading a patch anyways which can be used by sites that cannot upgrade to Drupal 10 yet.
Please ignore the patch in #2.
I have encountered the same issue, the attached patch fixes it for me. Although the patch is logically correct as it prevents `hook_entity_view()` from running on non content entities (like form modes), I think the actual root cause for the circular reference should also be resolved.
Steps to reproduce:
- Clear the plugin cache
drush cc plugin
. - Visit any page.
hook_entity_type_alter()
will get invoked and will reach to a point wherehook_entity_view()
is invoked as well, resulting in the mentioned error.
Full stack trace:
The website encountered an unexpected error. Please try again later.
Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException: Circular reference detected for service "business_rules.processor", path: "business_rules.event_listener -> business_rules.processor -> business_rules.util -> eca.service.token -> eca.token_data.current_user". in Drupal\Component\DependencyInjection\Container->get() (line 146 of core/lib/Drupal/Component/DependencyInjection/Container.php).
Drupal::service('business_rules.processor') (Line: 347)
business_rules_entity_load(Array, 'entity_form_mode') (Line: 431)
Drupal\Core\Entity\EntityStorageBase->Drupal\Core\Entity\{closure}(Object, 'business_rules') (Line: 405)
Drupal\Core\Extension\ModuleHandler->invokeAllWith('entity_load', Object) (Line: 432)
Drupal\Core\Entity\EntityStorageBase->postLoad(Array) (Line: 353)
Drupal\Core\Entity\EntityStorageBase->loadMultiple() (Line: 114)
Drupal\Core\Entity\EntityDisplayRepository->getAllDisplayModesByEntityType('form_mode') (Line: 85)
Drupal\Core\Entity\EntityDisplayRepository->getAllFormModes() (Line: 516)
business_rules_entity_type_alter(Array, NULL, NULL) (Line: 562)
Drupal\Core\Extension\ModuleHandler->alter('entity_type', Array) (Line: 334)
Drupal\Core\Plugin\DefaultPluginManager->alterDefinitions(Array) (Line: 123)
Drupal\Core\Entity\EntityTypeManager->findDefinitions() (Line: 175)
Drupal\Core\Plugin\DefaultPluginManager->getDefinitions() (Line: 22)
Drupal\Core\Plugin\DefaultPluginManager->getDefinition('user', ) (Line: 132)
Drupal\Core\Entity\EntityTypeManager->getDefinition('user') (Line: 253)
Drupal\Core\Entity\EntityTypeManager->getHandler('user', 'storage') (Line: 192)
Drupal\Core\Entity\EntityTypeManager->getStorage('user') (Line: 41)
Drupal\eca\Token\CurrentUserDataProvider->__construct(Object, Object) (Line: 262)
Drupal\Component\DependencyInjection\Container->createService(Array, 'eca.token_data.current_user') (Line: 176)
Drupal\Component\DependencyInjection\Container->get('eca.token_data.current_user', 1) (Line: 437)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array) (Line: 276)
Drupal\Component\DependencyInjection\Container->createService(Array, 'eca.service.token') (Line: 176)
Drupal\Component\DependencyInjection\Container->get('eca.service.token') (Line: 176)
Drupal\business_rules\Util\BusinessRulesUtil->__construct(Object) (Line: 262)
Drupal\Component\DependencyInjection\Container->createService(Array, 'business_rules.util') (Line: 176)
Drupal\Component\DependencyInjection\Container->get('business_rules.util') (Line: 159)
Drupal\business_rules\Util\BusinessRulesProcessor->__construct(Object) (Line: 262)
Drupal\Component\DependencyInjection\Container->createService(Array, 'business_rules.processor') (Line: 176)
Drupal\Component\DependencyInjection\Container->get('business_rules.processor', 1) (Line: 437)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters(Array) (Line: 240)
Drupal\Component\DependencyInjection\Container->createService(Array, 'business_rules.event_listener') (Line: 176)
Drupal\Component\DependencyInjection\Container->get('business_rules.event_listener') (Line: 136)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, Object) (Line: 52)
Drupal\business_rules\EventSubscriber\KernelRequestListener->onKernelRequest(Object, 'kernel.request', Object)
call_user_func(Array, Object, 'kernel.request', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'kernel.request') (Line: 135)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 709)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
Thanks @vensires!
While your patch removes the extra field, it doesn't actually apply the selected orientation because we're passing it to Mpdf under the key "default_page_orientation", while Mpdf expects it to be "orientation".
Updated the MR to fix this.
Adding a patch to use with composer...
I think we need one more review on this.
This may also apply to 10.1.x
but I didn't check it.
Adding a patch file to use with composer...
Redwan Jamous → created an issue.
Redwan Jamous → created an issue.
Created a MR...
Attaching a patch file to use with composer...
Redwan Jamous → made their first commit to this issue’s fork.
Redwan Jamous → created an issue.
Thank you, @smustgrave! I'm updating the issue summary...
Unfortunately, the previous fix doesn't work well while source editing mode in CKEditor is active.
To overcome the issue, the container can be the parent element (.ck.ck-editor
) instead of .ck.ck-editor__top
.
However, changing the css from:
.ck.ck-editor__top {
container-type: inline-size;
container-name: ck-editor;
}
@container ck-editor (width > 0) {
.ck-dropdown__panel {
--ck-toolbar-dropdown-max-width: 90cqw;
}
}
To:
.ck.ck-editor {
container-type: inline-size;
container-name: ck-editor;
}
@container ck-editor (width > 0) {
.ck-dropdown__panel {
--ck-toolbar-dropdown-max-width: 90cqw;
}
}
isn't enough because off-canvas reset (core/misc/dialog/off-canvas/css/reset.pcss.css
) is applying all: revert;
with higher specificity which will overwrite our container-type
and container-name
.
Adding .ck-editor
to the list of elements excluded from the off-canvas reset rules would fix the issue, but I'm not sure about this change.
We currently have the following selector in off-canvas reset:
#drupal-off-canvas-wrapper *:where(:not(svg, svg *, .ck-reset *, [data-drupal-ck-style-fence] *, .ui-resizable-handle))
We're excluding all children elements of .ck-reset
but not .ck-reset
itself (unlike svg).
The new proposed selector:
#drupal-off-canvas-wrapper *:where(:not(svg, svg *, .ck-reset, .ck-reset *, [data-drupal-ck-style-fence] *, .ui-resizable-handle))
Also, CSS linting checks are failing because of the cqw
unit, so I will be creating a separate issue to add container query units to the allowed units since they're now supported in all major browsers.
Thank you, Rajab!
I cherry-picked the commit into a new branch and opened a new PR for 10.0.x
, leaving the old one for 10.1.x.
@Rajab Natshah, yes we do as this is not specifically related to varbase_layout_builder/claro10
and the same issue will happen with any admin theme.
Redwan Jamous → changed the visibility of the branch 11.x to hidden.
Redwan Jamous → changed the visibility of the branch 3328095-ckeditor-5-toolbar to hidden.
Created a MR that fixes the issue using CSS container queries.
The --ck-toolbar-dropdown-max-width
variable will be set to 90% of the editor's width instead of 60% of the viewport's width.
Adding a patch file to use with composer...
Redwan Jamous → made their first commit to this issue’s fork.
Adding a patch file to use with composer...
Redwan Jamous → created an issue.
Adding a patch file to use with composer...
Redwan Jamous → created an issue.
Thanks for the fix!
Adding a patch file to use with composer...
Adding a patch file to use with composer...
Redwan Jamous → created an issue.
Attaching a patch file from the MR to use with composer...
Attaching a patch to use with composer...
Marking the issue as needs work since we still need to figure out how to handle existing content...
Redwan Jamous → changed the visibility of the branch 2.0.x to hidden.
Redwan Jamous → created an issue.
Redwan Jamous → changed the visibility of the branch 3063998-mpdf-plugin to hidden.
@vensires Yes, #18 is the one to go with.
Redwan Jamous → changed the visibility of the branch 3424034-add-3422610-4.patch-to to hidden.
Redwan Jamous → created an issue.
Adding a patch file to use with composer...
Redwan Jamous → created an issue.
Adding a patch to be used with composer...
Redwan Jamous → created an issue.
Adding a patch to be used with composer...
Redwan Jamous → created an issue.
Thank you for the feedback, @John.
Attached is an updated patch that utilizes batch processing.
Adding a patch from the open MR to be used with composer...
Adding a patch file to use with composer...
RedwanJamous → created an issue.
Corrected the mentioned naming conventions by using more appropriate terms; PascalCase instead of UpperCamel, and camelCase instead of lowerCamel.
Corrected the mentioned naming convention; class name should be in PascalCase not in camelCase.
Corrected the mentioned naming convention for service provider class; the prefix (i.e. the module name) must be in PascalCase not in camelCase.
Corrected the mentioned naming conventions; module names are in snake_case, while class names are in PascalCase not in camelCase.
No need for loadForUpdate
here since we're not saving the order itself.
LoadUnchanged
works fine.
Adding a patch file to be used with composer.
RedwanJamous → created an issue.
Hi @droprocker,
Thank you for your feedback.
The patch follows the general implementation of webform config/settings.
Referring to
When the global setting is set to content language, the webform-specific setting should be disabled.
In fact, this is the case as shown in #20 📌 Webform rendering should use the content language, not interface Needs review , you have the following options:
- If you have multiple webforms and you need all of them to use the interface language, you can just keep both checkboxes unchecked (global and webform-specific).
- If you have multiple webforms and you need all of them to use the content language, you will need to check the global checkbox and as a result, the checkbox inside each webform-specific settings will be disabled.
- If you have multiple webforms and you need only some of them to use the content language instead of the interface language, then you will need to keep the checkbox in the global settings unchecked, and check the checkboxes for the webforms you desire to render in the content language inside the settings of each of them.
Since the implementation of the current patch covers all use cases, I don't think there is a need for what you suggested lastly:
Or to have a third option in the webform-specific setting like this (which I prefer):
- Use global setting - Default
- Use Interface language
- Use content language
Plus, going with such suggestion will create an inconsistency in the UX since other settings work exactly this way (the way that the current patch is implemented).
On a side note, neither the webform-specific checkbox nor the global checkbox will be available unless the site is multilingual.
I'm not updating 20 subscriptions only, I'm updating all subscriptions but loading them in chunks of size 20 (loading each group of 20 subscriptions at once) to avoid memory issues.
Setting the value to the current time is an option, but setting it to match "created" field is more sensible since it will probably be much closer to the real last update.
Not setting a value at all is problematic as I explained before; if you performed any operation on the subscription other than saving the form and "changed" field didn't have a value at that time, it will not get updated and will remain empty.
Prevented access check when updating existing subscriptions.
Loaded them in chunks of size 20, please let me know if this should be changed.
My bad, on it.
Hi @jsacksick, thanks for the reply.
I fixed the points you mentioned, and added tests.
I also modified the update hook implementation to update all existing subscriptions to set the initial value of 'changed' field to match the value of 'created' field; this is needed because we need to have a value for 'changed' field in order to update it when doing operations other than saving the form (e.g. cancel operation). Using setInitialValueFromField('created')
when defining the field won't work since the field types don't match.
RedwanJamous → created an issue.
Updated forms settings and schema for tests to pass.
Next step is to add a test for this new functionality.
The attached patch adds a new checkbox to both the global configuration and the webform entity settings form under a new group called "Multilingual".
This adds the ability to force all webforms to be rendered using the content language, or to select specific webforms to be rendered using the content language.
This is an old issue and I really hope this gets merged soon.
Webform entity settings
Global configuration
RedwanJamous → created an issue.
Needs work as we're facing double order placement with this fix.
RedwanJamous → created an issue.