Closing. Already fixed in https://www.drupal.org/project/fences/issues/3464653 📌 Do we miss explode() calls for fences_field_classes, fences_label_classes, fences_field_items_wrapper_classes Active
The patch in #6 tried to split the text at the form level, attempting to follow what config_translation did (albeit very specific to Views config and not using the \Drupal\Component\Gettext\PoItem::DELIMITER that's actually used to join the values).
But there's another issue: The delimiter separating the strings can also get lost after translation (e.g. Google Translate API via tmgmt_google). So while you can check the source text for \Drupal\Component\Gettext\PoItem::DELIMITER and split it for the source, the translation provider might come back with just one string without the delimiters - not something you can split and map 1:1 with the source with the same amount of textfields/textareas.
An alternative solution would be to split the source string to an array and translate the singular and plurals independently. However, the job item data's #text property is expected in many places to just be a single string. Switching this to also support an array, or have a different key altogether requires refactoring in many places (many spots in the code specifically look for #text or #file).
Skipping plural_label fields from translation altogether as a workaround will just produce a value of NULL, which also causes the same error in the same spot.
The workaround is to translate the config with plurals manually via config_translation.
avpaderno → credited fskreuz → .
- The "use strict" on the jQuery UI scripts are inside IIFEs, so those should be safe when aggregated.
- ckeditor5-check-plugins.js is only used in build tooling, i.e. Node.js (it has a note in the script), so its file-level "use strict" is irrelevant.
The problem might be elsewhere. Check your custom and contrib modules and themes.
Sent up an MR. Also decided to drop the rendering of occurrences in the message, feels like that's a technical detail a regular user doesn't really need.
Recreated the MR for 3.0.x
Rerolled against the latest 1.x, converted to MR while I was at it.
fskreuz → made their first commit to this issue’s fork.
Added a separate MR for BEF 7.0. Wasn't sure if adding the additional changes to the existing MR would affect anyone referencing the MR directly in some way.
Did a quick test of this MR with the changes from 3437860. Works for me (Drupal 10.3.1, Flood Control 2.3.4, 3437860's MR, 3437875's MR).
I did see the note about the timestamp and expiration in the MR. But out of curiosity, I checked the entries in Redis. They do seem to have a couple of values that resemble timestamps, and appear to be about 6 hours apart (one's an integer, the other has 4 decimal places). Haven't looked any deeper yet, but at least I can unblock now.
I'm able to reproduce the issue on Drupal 10.3.1:
1. Install Drupal
2. Enable Languages and Content Translation modules
3. Add a language besides the default language (e.g default: English, other: French)
4. Add a node in English and translate it to French.
5. Delete the French language itself (not the French translation of the node).
6. View the node that had been created.
Notes:
- The entity doesn't have to be nodes. It can happen on other entities. In my case, it's Blocks.
- It doesn't matter if the language deleted has a region/script extension (e.g. zh-hant, pt-pt).
- Step 6 doesn't need to be a full view. Anything that pulls up an entity to check its language will also do (e.g. view nodes in content admin).
- Step 5 is not limited to deleting the language via admin. The same effect happens with drush cim (e.g. delete language entity config, deploy code to environment, and do drush deploy on that environment).
Now for the fun part:
- The potential cause of the issue is that there's content in the database that's still using the deleted language. This content is not cleaned up or at least converted into a representation that still works after deleting its language.
- The patch in #4 only works for the workflow that hits EntityAccessControlHandler.php. But in general, anything that pulls up the language entity from any entity and calls a method of that language entity will have this issue (i.e. any code that does $entity->language()->ANYMETHOD()). I just did a quick search in both core and contrib of my project, and there a lot of this $entity->language()->ANYMETHOD() pattern.
- \Drupal\Core\Entity\EntityInterface->language() is NOT type-hinted as nullable but the implementation can return a NULL.
Potential fixes:
- Declare $entity->language() to return NULL. This is true to the current behavior but would require everyone to null-check their language entities and would break API signatures.
- Have $entity->language() to always return an object of \Drupal\Core\Language\LanguageInterface (possibly represent "und", or however other APIs are representing "no language"). This keeps the signature and avoid having everyone null-check their language entities. But then this would also break everything that does check for NULL.
The MR already covers the replacing of ModuleHandler to ModuleHandlerInterface, and more. Hiding this patch to avoid confusion.
Updated #125 for 10.3.x, take 2.
Updated #125 for 10.3.x
Updated #25 against 10.3.x
Although it's easy to have a configurable redirect destination replace the hardcoded redirect to entity.user.edit_form, I am leaning towards #4's reasoning.
As far as password_policy (and any other module) is concerned, the password edit fields are in entity.user.edit_form as implemented in core. The change_pwd_page module replaced that location, so change_pwd_page should be in-charge of redirecting everyone to this new location, not the other way around where everyone has to adapt to change_pwd_page.
From a dependency perspective, this approach would have both modules not know of each other. password_policy only knows to send the user to entity.user.edit_form, while change_pwd_page only knows that it needs to redirect requests from entity.user.edit_form to its new page. This is kinda like how change_pwd_page is currently replacing user.reset from core's original path to its own. Maybe something similar can be done for entity.user.edit_form. But this work would have to be on change_pwd_page's side.
My 2c.
#8 also works for me (Password Separate Form 2.0.0, Password Policy 4.0.1)
#15 solves a related issue regarding Password Policy's password_username constraint.
Because change_pwd_page separates the password form from the user edit form, the name field is not present. Password Policy treats the missing name field as a blank string, causing it to fail the password_username constraint. Adding the name field as a hidden field works.
Verified patch works and still applies to 2.x.
fskreuz → created an issue.
Closing as duplicate of https://www.drupal.org/project/session_limit/issues/3308506 🐛 Services\SessionLimit should type-hint dependencies using interfaces Needs review .
Re-opening with updated patch. https://www.drupal.org/project/session_limit/issues/3361326 📌 Fix the issues reported by phpcs Needs review missed a few things from this issue.
VladimirAus → credited fskreuz → .
Patch in #31 works, with one minor issue: If you're indexing a field more than once (same Path, different Machine Name and Type), since the options are keyed by path, only the label of the last duplicate is used.
The use case I have is that we use a single index for multiple search views.
- A location-only search:
- A geofield field is indexed as a "Latitude/Longitude" type for proximity search.
- The same geofield field is also indexed as "Storage Only" for rendering in Leaflet maps (Leaflet doesn't render the previous type).
- A multi-type site search with location search in the advanced section:
- Aggregates several geofields into one "Latitude/Longitude" type for proximity search.
When choosing which fields to aggregate, the aggregate options only show the "Storage Only" geofield because it's the last one found.
Could just be a presentation/help text thing. Otherwise, looks good to me.
Yup, will do. Thanks!
fskreuz → created an issue.
Reroll of #19
This file appears to be from a patch. There is an updated version in the related issue.
There are two patches in this issue:
- #2 is for when you're using CKEditor 4.
- #4 is for when you're using CKEditor 5.
And so...
- If content_lock decides to go with CKEditor 4, then patch #2 is the one to merge.
- If content_lock decides to go with CKEditor 5, then patch #4 is the one to merge.
- If content_lock decides to support both versions, a new patch with both changes would probably be needed.
This will need a maintainer's input.
The patch in #64 🐛 Image derivative generation does not work if effect "Convert" in use and file stored in private filesystem Fixed contains the fix from https://www.drupal.org/project/drupal/issues/2786763 🐛 Incorrect headers for image derivative if file stored in private filesystem Needs work
But according to #49 🐛 Image derivative generation does not work if effect "Convert" in use and file stored in private filesystem Fixed , #50 🐛 Image derivative generation does not work if effect "Convert" in use and file stored in private filesystem Fixed , and #51 🐛 Image derivative generation does not work if effect "Convert" in use and file stored in private filesystem Fixed , 2786763's fix should not be included.
Closing in favor of https://www.drupal.org/project/exclude_node_title/issues/3346319 🐛 Fix accessibility issues by using visually-hidden on hide and unsetting title element on remove Needs review to address both issues in one patch.
Combining this patch with a fix for https://www.drupal.org/project/exclude_node_title/issues/3247751 → to address a related accessibility issue. Having two separate patches for each issue will conflict, since the changes are in close proximity. Combining them should expedite the fix. Combined patch now:
- Applies the visually-hidden class to the title element when hidden.
- Removes the title element entirely when removed.
Reroll against 3.0.x
Reroll of #21 for 10.2.x.
Added both indent and no-indent versions, since the files involved are frequently modified by updates. The indented version is the one meant to be merged, while the no-indent version is for easy composer patching.
Reroll for 10.2.x
Reroll for 10.2.x
Here's a version of the patch for CKEditor 5, and the relevant documentation:
- https://ckeditor.com/docs/ckeditor5/latest/features/read-only.html
- https://ckeditor.com/docs/ckeditor5/latest/api/module_core_editor_editor...
Rerolled against 10.1.x
Closing in favor of https://www.drupal.org/project/session_limit/issues/3308506 🐛 Services\SessionLimit should type-hint dependencies using interfaces Needs review
Updated patch to change all the dependencies to use interfaces. Problems will occur eventually with the others, makes sense to fix them all in one go.
Remove stray console.log()
Undoing #33. Misread the other issue.
Roll in the similar one-liner change from 3192991 🐛 Contrib module classes are overwritten Needs review since it was closed in favor of this issue.
Reroll #11 against 9.5.x and 10.1.x once more (#16 rerolled against older versions of those branches).
Reroll of #11 for 9.5.x and 10.1.x
Just adding some context since it's not the first time I've bumped into this issue and tried to recall how this worked:
2158003 →
added the "the exact magical spot" referred to in
#33
🐛
Allow field blocks to display the configuration label when set in Layout Builder
Fixed
. It's a way for the rendered block content to override the configured title (e.g. views blocks). It appears that the intent is to have the implementing block place #title to override the block title (or at least it's
a change to accommodate how Views did block title overrides →
). Patch fixes the issue by avoiding the field's #title property from being in the magical spot by placing it in an array, nesting it a level down.
2942623 → exists, but might be closed. Although it brings up an interesting point of why the title override comes after the block's configured label. You would expect that block configuration overrides whatever the block content supplier (be it Views or Field Block) supplies.
Can confirm #31.
In our case, we have compiled JS in modules/themes whose output automatically gets a file-level "use strict";. This makes sense if the compiled JS is evaluated on its own (e.g. added via its own script tag, or eval'ed on its own).
But since that compiled JS is added as a library, it is subject to aggregation. Drupal aggregation is just a simple "concat a bunch of JS" operation, which means ANY JS that's aggregated together with one that has a file-level "use strict"; is evaluated in strict mode entirely - and will break some arbitrary library that wasn't expecting it.
Also, for regular scripts, "sloppy mode" is the browser's default execution mode while strict mode is opt-in. Although strict mode is encouraged, one cannot assume all scripts are written in strict mode. This may be the case for loadjs.
- https://developer.mozilla.org/en-US/docs/Glossary/Sloppy_mode
- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict...
So to sum it up:
- Aggregation can cause scripts declaring strict mode to be concatenated with ones that don't expect strict mode, breaking the latter.
- Therefore JS included in libraries SHOULD NOT have a file-level "use strict"; to avoid breakage.
- Any JS that is explicitly enforcing strict mode should move "use strict"; from a file-level to a function-level (e.g. wrap your script in an IIFE with "use strict";)
- No changes are needed for loadjs. Instead, fix the code introducing "use strict"; with the suggestion above.
fskreuz → created an issue.
Updated patch #5 for 3.x
Here's a reworked #106 based on comments from #105. Added a new configuration field label_display_type separate from the existing label_display checkbox that lets one select how one wants to hide the title. The hope would be that in a future version, label_display will just roll into the select box as the "visible" option, just like the patches prior. Name, label, and description could be better, open to suggestions.
Tests removed in this patch, I wasn't sure if they'd still be applicable (I also don't have a test-capable setup at the moment). Hopefully someone can pick up the baton from here.
Reroll
Reroll against 1.x (after 1.3)
@Anybody The theme I use is based on stable9. I am not able to write tests at the moment.
But to add more info, it's this bit of code in `fences_preprocess_field()` that's causing the issue. Two separate issues (linked, see sidebar), possibly not aware of each other and solving separate issues, merged and duplicated functionality. I hope this helps someone.
// Prevent adding empty classes:
if (!empty($fences_config['fences_field_classes'])) {
$fences_field_classes = explode(' ', $fences_config['fences_field_classes']);
foreach ($fences_field_classes as $class) {
// Add classes from setting:
$vars['attributes']['class'][] = $class;
}
}
if (!empty($fences_config['fences_label_classes'])) {
$fences_field_classes = explode(' ', $fences_config['fences_label_classes']);
foreach ($fences_field_classes as $class) {
// Add classes from setting:
$vars['title_attributes']['class'][] = $class;
}
}
if (!empty($fences_config['fences_field_classes'])) {
$vars['attributes']['class'][] = $fences_config['fences_field_classes'];
}
if (!empty($fences_config['fences_label_classes'])) {
$vars['title_attributes']['class'][] = $fences_config['fences_label_classes'];
}