I removed the getTexformatsUsingMentions()
method and updated the test to cover isFormatMentionable()
.
After reviewing issue
🌱
[policy] Standardize how we implement in-memory caches
Needs work
, I decided to replace the custom static cache implementation with BackendChain
to combine cache backends, similar to what jsonapi does. This removes the need for a custom resetCache()
method and allows us to fully delegate cache invalidation to the core.
I don't like using a class variable for caching, since the "caching" is only per PHP request. I'd rather use the Drupal caching system instead!
Regarding the in-memory caching, I see your point about per-request caching being limited. However, Drupal itself often uses memory caching to avoid repeatedly loading the cache from the database. In this case, it’s a similar situation: editor entities are already cached and likely loaded during text field save operations, and our calculations to determine which formats can be mentioned are not very heavy. So using a class variable for per-request caching still provides a small performance benefit without adding complexity.
Thank you for the feedback. I agree that a persistent cache would further improve performance, so I have added it.
What do you think about removing the getTextformatsUsingMentions()
method?
- Renamed the option to a more generic and clear name, making it easier to understand for new users unfamiliar with version 2.x.
- Improved caching behavior for access check results.
- Added a test that covers the new mode of operation.
Thanks for raising this point. We discussed it with @foxy-vikvik and decided to add a new markup option, but instead of a styled <div>
it will be implemented using <figure>
and <figcaption>
. Please see
📌
Create accessible formatter using and tags for captions (a11y compliance)
Active
for details.
I will start working on this now. This approach is semantically correct, meets accessibility requirements, and is already used in Drupal core by CKEditor5.
Thank you for reviving this discussion and for preparing a concrete proposal.
I believe the original maintainer's approach is still valid today: the module should provide only the minimal markup needed, leaving it up to site builders and themers to override the template in their own theme. With Twig it is straightforward to copy the template and replace <blockquote>
with <div>
(or any other element) if a project requires it.
Introducing predefined colors, shadows, and other opinionated styles into the module would reduce flexibility and could conflict with existing design systems. Since there is no truly universal look for captions, it seems better to keep the module neutral and let themes handle presentation.
If someone wants custom styling, it's also easy to attach it directly in the template with their own library, for example:
{{ attach_library('your_theme/your_image_caption_styles') }}
This way both the markup and the styles can be overridden cleanly and in one place.
@foxy-vikvik what do you think about this?
Hi,
Thanks for reporting this. The error occurs because your field configuration for default_image
is not in the expected format. Since default_image
has been stored as an array
starting with Drupal 8
📌
Alt, title, width and height for default images
Fixed
, and this module supports Drupal 10.3 and later, it assumes the configuration is already correct.
To fix this, please resave the field configuration via the UI or manually adjust the exported configuration so that default_image
is an array. The module cannot handle configurations that are inconsistent with core expectations.
Thanks for understanding!
I removed unused dependency injections and replaced the use of getTexformatsUsingMentions
for each entity with a lazier and cacheable approach in isFormatMentionable
.
I also replaced my own check for inheritance from TextItemBase::class
with a call to the _editor_get_formatted_text_fields
function from core. This removes from us the responsibility of deciding what should be considered a text field, but adds a few extra iterations, since first the function prepares the fields, and then we iterate over them. However, these are very small overhead costs, and in the future the core might make more active use of generators for such functions, and this drawback may disappear entirely.
Definitely makes sense! I wonder if we could specify this even more through checking whether the field widget actually uses CKEditor.
We are already doing this, check the getTexformatsUsingMentions
method, it will return only the formats that use CKEditor5 and have the mentions plugin enabled.
But I think loading all formats when saving any entity with fields is also a bit of overhead. I would like to add one more optimization.
kksandr → changed the visibility of the branch 3541318-allowed-widgets-option to hidden.
Thanks for the report. To investigate this on 4.0.0-rc2 we need the exact configuration so we can reproduce.
Please attach or paste:
core.entity_form_display.*
for the bundle where the field should appearfield.field.*
for the affected field(s)- Drupal core version
- Any errors in Recent log messages or the browser console
You can get the config with:
drush cget core.entity_form_display.<entity>.<bundle>.default
drush cget field.field.<entity>.<bundle>.<field_name>
Clearing caches after changing the option also helps confirm whether it is a display issue or configuration.
@heavystonehead you can now configure which widgets show the caption field in your image field settings. Look for the "Caption field allowed widgets" option and select the widgets where you want captions enabled.
kksandr → changed the visibility of the branch 3541318-improve-widget to hidden.
kksandr → made their first commit to this issue’s fork.
kksandr → made their first commit to this issue’s fork.
kksandr → changed the visibility of the branch 3255807-request-option-to to hidden.
1. Apologies — due to a rushed release, the update was not properly tested and unintentionally reset the field settings. This issue is now fixed and won't overwrite existing settings anymore.
2. Thanks for pointing that out. I'll pass this to the maintainers for possible inclusion on the release page or a change record.
kksandr → changed the visibility of the branch 3472997-fixes-3 to hidden.
kksandr → made their first commit to this issue’s fork.
kksandr → changed the visibility of the branch 3472997-fixes-2 to hidden.
Thank you for the feedback.
Regarding the issue:
Warning: Undefined array key "caption_allowed_formats" in image_field_caption_field_widget_single_element_image_image_form_alter() (line 42 of modules\contrib\image_field_caption\image_field_caption.module).
we've released version 4.0.0-beta3
which should resolve it. We're aiming to keep the configuration safe to use, avoiding excessive isset
/empty
/??
checks across the codebase. This release includes a post-update hook that applies default settings for fields. Please remember to export your configuration after updating to avoid losing changes on the next import.
As for the validation error:
Error message
The submitted value plain_text in the Text format element is not allowed.
Unfortunately, we don't have a proper fix for this yet. plain_text
is a valid format, and forcing its change just because it's a fallback would be a bad approach. All we can suggest is enabling its usage by editing the configuration. This will allow it to be used and will avoid the validation error:
drush config:set filter.settings always_show_fallback_choice TRUE
kksandr → changed the visibility of the branch 3472997-fixes to hidden.
Thanks for the follow-up. Yes, I meant 4.0.x. Despite the "beta" label, it's the stable and recommended version for testing and production. The 4.0.x branch is a full rewrite with proper test coverage, and I'm confident in its stability.
All further development will continue in 4.0.x. Older versions (like 3.x) will be deprecated soon due to architectural limitations.
If the issue still occurs in 4.0@beta feel free to follow up.
That's a good idea, let's do it that way.
kksandr → changed the visibility of the branch 3472997-rewrite-module-to to hidden.
This has been fixed in version 4.0.0-beta1. The caption
and caption_format
are now field properties, so they are available as tokens.
Thanks for the report. This issue refers to an outdated version of the module (8.x-1.x-dev), which is no longer supported. The 4.0.x release includes significant changes, and this problem may already be resolved.
Please check if the issue still occurs with the latest version (4.0.0-beta1). If it does, feel free to update this issue or open a new one with details.
Thanks for the report. This issue refers to an outdated version of the module (8.x-1.2), which is no longer supported. The 4.0.x release includes significant changes, and this problem may already be resolved.
Please check if the issue still occurs with the latest version (4.0.0-beta1). If it does, feel free to update this issue or open a new one with details.
Thanks for the report. This issue refers to an outdated version of the module (8.x-1.x-dev), which is no longer supported. The 4.0.x release includes significant changes, and this problem may already be resolved.
Please check if the issue still occurs with the latest version (4.0.0-beta1). If it does, feel free to update this issue or open a new one with details.
This has been fixed in version 4.0.0-beta1. The caption
and caption_format
are now field properties, so you can work with them just like alt
or title
.
Thanks for the report. This issue refers to an outdated version of the module (8.x-1.x-dev), which is no longer supported. The 4.0.x release includes significant changes, and this problem may already be resolved.
Please check if the issue still occurs with the latest version (4.0.0-beta1). If it does, feel free to update this issue or open a new one with details.
Thanks for the report. This issue refers to an outdated version of the module (8.x-1.2), which is no longer supported. The 4.0.x release includes significant changes, and this problem may already be resolved.
Please check if the issue still occurs with the latest version (4.0.0-beta1). If it does, feel free to update this issue or open a new one with details.
This has been fixed in version 4.0.0-beta1. Please try the latest release and feel free to reopen or create a new issue if needed.
Thanks for the report. This issue refers to an outdated version of the module (2.0.x), which is no longer supported. The 4.0.x release includes significant changes, and this problem may already be resolved.
Please check if the issue still occurs with the latest stable version (4.0.x). If it does, feel free to update this issue or open a new one with details.
Thanks for the report. This issue refers to an outdated version of the module (2.0.x), which is no longer supported. The 4.0.x release includes significant changes, and this problem may already be resolved.
Please check if the issue still occurs with the latest stable version (4.0.x). If it does, feel free to update this issue or open a new one with details.
Thanks for the report. This issue refers to an outdated version of the module (2.0.x), which is no longer supported. The 4.0.x release includes significant changes, and this problem may already be resolved.
Please check if the issue still occurs with the latest stable version (4.0.x). If it does, feel free to update this issue or open a new one with details.
Thanks for the report. This issue refers to an outdated version of the module (2.0.x), which is no longer supported. The 4.0.x release includes significant changes, and this problem may already be resolved.
Please check if the issue still occurs with the latest stable version (4.0.x). If it does, feel free to update this issue or open a new one with details.
Was fixed here: 🐛 Issue with first time saving the image caption of an image field. ( Paragraph ) Active .
Alright, I’ve closed the MR that only included the test to avoid any confusion. I’ve also changed the target branch to 11.x.
I’ve added two branches:
3537774-validation-test
- contains only a test to demonstrate the issue.3537774-validation-fix
- includes both the test and the fix.
kksandr → created an issue.
Funny enough, the order of operations between && and = was also the problem behind #3403999: Exposed filter values ignored when using batch.
@godotislate These are two completely different cases.
kksandr → changed the visibility of the branch 10.3.x to hidden.
kksandr → changed the visibility of the branch 11.x to hidden.
The issue has been fixed in 2.x: #3265219: Fix coding style issues and achieve phpstan lvl6 → , 📌 Drupal 11 compatibility Needs work
Will be fixed here: 🐛 Issue with first time saving the image caption of an image field. ( Paragraph ) Active .
Will be fixed here: 🐛 Issue with first time saving the image caption of an image field. ( Paragraph ) Active .
Will be fixed here: 🐛 Issue with first time saving the image caption of an image field. ( Paragraph ) Active .
Will be fixed here: 🐛 Issue with first time saving the image caption of an image field. ( Paragraph ) Active .
I confirm that the handling of the absence of $value['image_field_caption']['value']
is correctly implemented here, and it resolves the issue of the value disappearing during repeated programmatic entity saves (not through forms).
I am also redirecting this MR to the 2.0.x branch, as a completely new implementation of the module is planned for the 3.0.x branch.
kksandr → made their first commit to this issue’s fork.
Yes, I will add such processing for images that are corrected by this module.
But always checking all images, loading them to get the real resolution will be too resource-intensive task and is far beyond the scope of what this module works with.
So if you want to have such processing for all images, I think you should look for a solution elsewhere.
I thought that an access denied would be sufficient to redirect to (or show subrequest of) the next path in the dynamic link
You understand everything correctly. All links in the specified order are checked for access and if there is an accessible one, it will be shown. If all links return access denied then the result will be an access denied. Are you sure that the next link in the list is accessible for the user you are testing?
If you are sure that the configuration is correct, then perhaps there is an error in the module, in which case it is necessary that you provide an example of the view configuration and dynamic link to investigate your case.
The edit has been corrected. Also, the test coverage has been updated to detect such cases.
I find this fix useless because I can't reproduce this issue on 7.0.0-beta1 or 6.0.6 following the steps in the issue. The patch looks like it disables this and makes this code dead, so yes it does solve the problem, but not very well. It's very strange that there is no test case here and the fix is already merged and released.
Same problem, pager is broken after this fix. Rolling back to 7.0.0-beta1 also solves the problem.
@hmdnawaz I use phpredis, so it's class: Redis
, which is the global class provided by that PHP extension.
This event was added to editors in version 8.0-alpha3~680 11 years ago, so it is compatible with Drupal 8 which the module claims to support.
kksandr → changed the visibility of the branch 2.1.x to hidden.
kksandr → created an issue. See original summary → .
This issue is still happening, I have added steps to reproduce it.