Adding a static patch based on MR 22.
Agreed, this would be a nice enhancement!
Media Library Edit → also allows editing media items in a modal (via a form widget). That module's code might offer some ideas for implementation.
MR 13 allows any valid `$text-container` element (e.g. headers) to be indented.
It also updates the CSS to remove the element targets (p, ol, ul). List element are indented by nesting a new list container, so the class is not used in that case anyway. I also changed the units from `em` to `rem` to ensure consistent indents where font-sizes might vary (again, e.g. headings).
Static patch attached.
This would be a small but nice UX improvement.
The project page lists "Text filter for syntax highlighting" as one of the features.
Maybe it does mean we could make that a recommendation for this particular feature need.
Makes sense to me. That's definitely how we're using it.
I think the two fill different needs. This module (greatly) improves the user experience when using ckeditor's view/edit source feature, with the ckeditor wysiwyg still intended to be the primary editing mode.
CodeMirror Editor is a dedicated editor for code and would be associated with it's own text format.
We use CodeMirror Editor → to edit code-only blocks. Works great for our needs.
My mistake -- I should've used .once()
instead of .on()
. MR updated.
Thanks for testing!
Marked MR as ready. Given that the previous code remains, this should still work with previous versions.
Attaching a static patch.
I opened an MR with an update that's working for us. I've only done initial testing, and I'm not in a position to easily test with ckeditor < 44.0.0 -- hence the 'draft' MR status.
Please test/review if you're able.
justcaldwell → made their first commit to this issue’s fork.
Another +1 for RTBC: Drupal 10.4.1, Field Group 8.x.-3.6
Thanks for the info on wrappers. And I just wanted to say, Custom Field really is an excellent contribution — thanks for your work!
That works -- merged to dev. Thanks for contributing!
Okay, the patch did resolve the warnings. Thanks for the fast turnaround!
I also see the new config item after actually editing/saving the formatter settings per your instructions in the other issue 🐛 Errors when displaying Entity Reference field as a block in block layout Active .
We also use fences, so I'm curious to see if/how these new wrappers might interact with that module.
Hello! Just here to confirm that we're seeing the wrapper/offset warnings described in #4 after updating to 3.1.0. Re-saving manage display settings didn't help.
I haven't had time to investigate further. We've reverted to 3.0.x for now.
justcaldwell → created an issue.
Confirming that the CKEditor 5 Bookmark plugin does not support bookmarks/anchors on links. It my testing it also appears to strip id attributes from existing links.
If you're on Drupal 10.4 or 11.1, you can test for yourself with CKEditor 5 Bookmark → .
justcaldwell → created an issue.
Glad to help.
Drupal ^10.4 || ^11.1 now ship version 44.0.0 of ckeditor5 and the Bookmark plugin. I created CKEditor 5 Bookmark → to enable the plugin for use/testing.
Actually, as of Year Only 9.1.1, that's true for any supported version of Drupal. The image above shows the Drupal 10.2+ UI.
Hi @fvd! What version of Drupal are you using? On Drupal 10.2+, it is under the "Date and time" field type.
Updated static patch attached.
Updated the MR with a second post-update hook to run after invalid permissions are removed. So the solution now:
- Implements logic to associate appropriate menu config dependencies with Menu Admin per Menu permissions.
- Removes any stale/invalid Menu Admin per Menu permissions (i.e. for menus that have been deleted).
- Re-calculates dependencies and updates roles that have existing Menu Admin per Menu permissions (in light of #1).
Back to NR.
Hmm. Looks like the update hook should also cause an update of any role with any menu_admin_per_menu permission, so the dependencies for existing permissions are applied. Back to NW. :\
Posting a static patch. Also bumping to Major as this can render the Permissions page unusable unless/until you do something like the suggested workaround in #5.
Post update hook added. Ready for review/testing.
Great, thanks @mahyarsbt!
I see the MR was approved, but it seems like it's not actually merged to 1.0.x?
justcaldwell → changed the visibility of the branch 3457584-remove-keyvaluestore to hidden.
Until someone provides an MR for that feature, I recommend the approach in MR 8 for folks who need a resolution to the issue in the meantime. To that end, I'm attaching a static patch based on the MR.
Updated IS
The update hook from a patch for the same issue in entityqueue → looks almost cut-and-paste-able.
MR 20 adds the required dependencies. Ideally, we'd also provide an update hook to clean up any stale invalid permissions hanging around (hence the draft MR). I'll try to take a pass that hook soonish, if no one beats me to it ;).
Just bumped into this again. Updated Steps to Reproduce to more reliably invoke the error.
MR 8 is an alternative solution that makes fewer changes to the module, retains the KeyValueStore approach and the role specific form/route handling. It just discards any stored settings after each use — settings are re-loaded/updated as usual on submitting the filter. I kept the store as expirable in case something unpredictable occurs after the form is submitted, but reduced the time to 5 minutes.
Either approach ensures the permissions page is immediately usable if the user returns to it after attempting to load too many values (or within 5 minutes if something else went wrong).
Hi @nsciacca. Thanks for starting the MR! Yeah, I ran into the same issue this morning. I added the fix to the MR, as well as some additional cleanup of KeyValueStore-related code.
Also, PermissionsRoleSpecificForm.php depends on saveFilterSettings()
, so that needed to be addressed. Since a permission filter has been added to Drupal's core form, I don't think it's necessary for filter_perms to override the role-specific form any longer — so I removed it and the route subscriber.
We'll have to see how the module owner feels about all that 🙂. Setting to NR.
@cyu: Some initial testing of this approach worked for me. If we $form_state->setRebuild()
in the submit handler, the submitted values will be still be available on subsequent loads, so $filter
can just draw from those values and drop the KeyValueStore entirely. The submit handler would become:
public function submitFormFilter(array &$form, FormStateInterface $form_state) {
$form_state->setRebuild();
}
Then $filter becomes:
$filter = [
'roles' => $form_state->getValue('roles', []),
'modules' => $form_state->getValue('modules', []),
];
See: FormState::$rebuild and FormState::setRebuild
The form 1) would no longer "remember" the previous filter if you navigate away from the permissions page AND 2) the render will still fail if too many permissions/modules are selected, BUT the page will be immediately usable if you navigate away and come back to it.
The solution for #2 is user training, unless we add some generic warning text about selecting too many permissions/modules.
Hi @nsciacca - I think remembering the filter settings between submissions is essential to the way the module functions.
@cyu - regarding your question in #5:
Out of curiosity, what kind of numbers for roles/permissions and memory limits are causing this problem?
We have 865 permissions and 54 roles (yikes!).
I just tried 'All modules / All permissions' again on my local with a memory limit = 1024M and it looks like the form actually begins to render for me (after quite a long time). It's the browser (Chrome in this case) that chokes on the results and ends up giving the 'Oh snap, something when wrong' message after partially rendering the page.
On our actual web servers, the memory limit on admin routes is 512M, and the page exhausts the memory and fails quite a bit sooner with no attempt to render.
Noting that the latest release of CKEditor 5 (44.0.0) now includes the Bookmark plugin — CKEditor's native UI for creating anchor links.
Additionally, work on link UI enhancements to ease linking to "on page" anchors/bookmarks is being tracked in this github issue. See the second video on that page for a demo.
I think that modules should ensure compatibility with popular admin themes, so it's probably more likely that this becomes part of (or sub-module of) Moderation Sidebar. That's probably no going to happen, so we'll just settle for the status quo 🙂.
Added 'Administration Tools' -- seemed more accurate than 'Developer Tools', which I removed.
LGTM and mo issues found in manual testing against D10/11.
Merged to 1.x-dev. Thanks for contributing.
justcaldwell → made their first commit to this issue’s fork.
justcaldwell → created an issue.
justcaldwell → created an issue.
Merged to 2.x, release to follow.
Re #3: Thanks, but the patch only addresses comment #2, not the full issue as described in the summary.
Released in 2.0.0-alpha9.
Released in 2.0.0-alpha9
The signature for the __construct() method of ConfigFromBase has changed in Drupal 11 such that we'll need to drop support for Drupal 9. See https://www.drupal.org/node/3404140 → .
justcaldwell → created an issue.
This issue and MR is 'sub-optimal' at best! Closing.
Merged to dev. New release to follow soon.
Just looking at the examples from the core ckeditor5 module, your example seems close, but maybe it should be something more like:
my_plugin:
ckeditor5:
plugins: [ htmlSupport.GeneralHtmlSupport ]
config:
htmlSupport:
allow:
- name: ~
classes: [my, whitelisted, classes]
drupal:
label: Whitelisted elements
library: core/ckeditor5.htmlSupport
elements:
- <* class="my whitelisted classes">
I'm working on 📌 Add support for new field selection UI Active , and it makes sense to go ahead and fix this in those changes. Closing and adding credit there. Thanks!
Makes sense to fix ✨ Allow underscores in CSS classnames/style key field. Active while we're making these changes.
Released in 1.0.4
Fixed in dev. New release soon.
justcaldwell → created an issue.
Thanks for posting this @gravelpot! Confirming same.
The offending commit was reverted in https://github.com/robrichards/xmlseclibs/releases/tag/3.1.3 -- updating to that release resolves the problem for us.
This is still an issue in 4.x.
The fix proposed in the MR still works in 4.x. The MR will need a re-roll for the update hook.
Thanks for the update. Setting to NW.
Thanks for raising this @alphex! This is now fixed in the 2.0.x-dev branch, or you could apply the patch from the MR.
I hope to tag a new release sometime in the next week or so.
justcaldwell → made their first commit to this issue’s fork.
Thanks for raising this!
Some steps to reproduce would be helpful here. Also, do we need an update/post update hook to clean up possible improperly stored values?
Previous patch looks like pretty obvious credit mining and @Pracheth never returned to correct the format. No credit.
Makes sense. Merged with a new release coming shortly.
Thanks for contributing!
justcaldwell → made their first commit to this issue’s fork.
No problems with manual testing. Merged the minimal change and will tag a new release soon.
justcaldwell → made their first commit to this issue’s fork.
Having a hard time replicating the scenario, but I can see how it happens. I suppose we could query for distinct values and populate the bundle filter with the results. Couldn't this be an issue for other fields as well?
This is a good idea. Are you thinking layout id should be on the current report somewhere, or would this be a new report?
justcaldwell → created an issue.
Did some manual testing on D11. All seems fine. Merged to dev, release to follow.
justcaldwell → made their first commit to this issue’s fork.
Thanks for the contribution! In general, I like the changes proposed here. I think there are some minor issues that need discussion/work:
- In the filters, what was labeled 'Block Type' is now 'Inline Block Type' and 'Block Type' now refers to other or reusable blocks, is that correct? This was confusing to me. Maybe 'Other Block Type' or 'Reusable Block Type' would be a better label here? I'm not sure.
- If I'm correct above, then those two filters are mutually exclusive, right? That can lead to further confusion. Maybe throw an error on validation, or add some help text or additional organization to the filter area?
- The 'Reset' button doesn't clear Inline Block Type if a value has been set.
Static patch of the current MR.
I opened PR 14 on northernco/ckeditor5-anchor-drupal in support of the MR here.
Just wanted to note that, when last I checked, ckeditor's upcoming 'Bookmarks' plugin will not support editing/creating bookmarks (anchors) on links.
I also don't see a way to resolve the edge case in #25. I think the changes so far are an improvement on stripping ids from links, so setting back to NR.
As I indicate in the MR, this still requires minimal changes in northernco/ckeditor5-anchor-drupal plugin. I can open a PR there if there's any interest in this.
At this point, we're just using a custom build of the plugin that resolves this and a few other issues in the queue to try to ensure a smooth transition from cke4 -> cke5.
MR created as promised 🙂
I haven't actually tried this, but I think a "quick-and-dirty" approach would be to just manually replace the northernco version of the plugin in your /libraries folder:
- Visit https://asset-packagist.org/package/npm-asset/justcaldwell--ckeditor5-an..., and click 'Get Zip'
- Decompress the zip file, and rename the resulting 'package' directory to 'ckeditor5-anchor-drupal'
- Replace /libraries/ckeditor5-anchor-drupal with the directory above
I assume this would get overwritten next time you update anchor_link with composer, so you'd probably want to re-install anchor_link using the modified "Option 2" outlined in #21 and #26 above when possible.
Adding CKEditor Anchor Link → , where it's currently a dependency of the 8.x-2.x branch, and might be added back to the 3.x branch (see: 🐛 Support concurrent use of CKEditor 4 and 5 Active ).
Given no other input, I'm going to close this one. Thanks!
Here to eat my words 😬.
Given that there's still a large number of sites that haven't moved from CKEditor 4 (1.x and 2.x branch usage is still at ~18K), there may be a case for supporting an improved upgrade path that allows testing both CKE4 and CKE5 concurrently (see 🐛 Support concurrent use of CKEditor 4 and 5 Active ).
This would require allowing the name
to continue to appear in the final source code. As I wrote in that issue:
The plugin currently upcasts anchors with
name
attributes, but does not include thename
attribute in the resulting page source. As a result, anchor links that have been edited in CKEditor 5 will no longer be present if the text format is subsequently switched back to one using CKEditor 4.
Even though name
is deprecated, it might be preferable to continue full support for now, and remove it in a subsequent release or branch.
I'm attaching a patch with the necessary changes. I'll open an issue branch/MR soon.
I opted to not reintroduce the hard dependency on the CKEditor FakeObjects → module now, but to use these changes you would need to include it in your project:
composer require 'drupal/fakeobjects:^1.2'
As I said above, this is probably a good idea anyway if you're updating from 8.x-2.x, as composer may well remove the module (if it's not required elsewhere) without uninstalling it. FakeObjects can be uninstalled and removed when it's no longer needed.
The patch also corrects broken paths to the CKEditor 4 toolbar icons.