Austin, Texas
Account created on 7 May 2008, about 17 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States justcaldwell Austin, Texas

Adding a static patch based on MR 22.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

This would be a small but nice UX improvement.

🇺🇸United States justcaldwell Austin, Texas

The project page lists "Text filter for syntax highlighting" as one of the features.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

We use CodeMirror Editor to edit code-only blocks. Works great for our needs.

🇺🇸United States justcaldwell Austin, Texas

My mistake -- I should've used .once() instead of .on(). MR updated.

Thanks for testing!

🇺🇸United States justcaldwell Austin, Texas

Marked MR as ready. Given that the previous code remains, this should still work with previous versions.

🇺🇸United States justcaldwell Austin, Texas

Attaching a static patch.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

justcaldwell made their first commit to this issue’s fork.

🇺🇸United States justcaldwell Austin, Texas

Another +1 for RTBC: Drupal 10.4.1, Field Group 8.x.-3.6

🇺🇸United States justcaldwell Austin, Texas

Thanks for the info on wrappers. And I just wanted to say, Custom Field really is an excellent contribution — thanks for your work!

🇺🇸United States justcaldwell Austin, Texas

That works -- merged to dev. Thanks for contributing!

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas
🇺🇸United States justcaldwell Austin, Texas

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 .

🇺🇸United States justcaldwell Austin, Texas

Glad to help.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

Hi @fvd! What version of Drupal are you using? On Drupal 10.2+, it is under the "Date and time" field type.

🇺🇸United States justcaldwell Austin, Texas

Updated the MR with a second post-update hook to run after invalid permissions are removed. So the solution now:

  1. Implements logic to associate appropriate menu config dependencies with Menu Admin per Menu permissions.
  2. Removes any stale/invalid Menu Admin per Menu permissions (i.e. for menus that have been deleted).
  3. Re-calculates dependencies and updates roles that have existing Menu Admin per Menu permissions (in light of #1).

Back to NR.

🇺🇸United States justcaldwell Austin, Texas

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. :\

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

Post update hook added. Ready for review/testing.

🇺🇸United States justcaldwell Austin, Texas

Great, thanks @mahyarsbt!

I see the MR was approved, but it seems like it's not actually merged to 1.0.x?

🇺🇸United States justcaldwell Austin, Texas

justcaldwell changed the visibility of the branch 3457584-remove-keyvaluestore to hidden.

🇺🇸United States justcaldwell Austin, Texas

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

🇺🇸United States justcaldwell Austin, Texas

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 ;).

🇺🇸United States justcaldwell Austin, Texas

Just bumped into this again. Updated Steps to Reproduce to more reliably invoke the error.

🇺🇸United States justcaldwell Austin, Texas

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).

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

@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.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

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 🙂.

🇺🇸United States justcaldwell Austin, Texas

Added 'Administration Tools' -- seemed more accurate than 'Developer Tools', which I removed.

🇺🇸United States justcaldwell Austin, Texas

LGTM and mo issues found in manual testing against D10/11.

Merged to 1.x-dev. Thanks for contributing.

🇺🇸United States justcaldwell Austin, Texas

justcaldwell made their first commit to this issue’s fork.

🇺🇸United States justcaldwell Austin, Texas

justcaldwell created an issue.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

Released in 2.0.0-alpha9.

🇺🇸United States justcaldwell Austin, Texas

Released in 2.0.0-alpha9

🇺🇸United States justcaldwell Austin, Texas

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 .

🇺🇸United States justcaldwell Austin, Texas

justcaldwell created an issue.

🇺🇸United States justcaldwell Austin, Texas

This issue and MR is 'sub-optimal' at best! Closing.

🇺🇸United States justcaldwell Austin, Texas

Merged to dev. New release to follow soon.

🇺🇸United States justcaldwell Austin, Texas

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">
🇺🇸United States justcaldwell Austin, Texas

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!

🇺🇸United States justcaldwell Austin, Texas

Makes sense to fix Allow underscores in CSS classnames/style key field. Active while we're making these changes.

🇺🇸United States justcaldwell Austin, Texas

Fixed in dev. New release soon.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

Thanks for the update. Setting to NW.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

justcaldwell made their first commit to this issue’s fork.

🇺🇸United States justcaldwell Austin, Texas

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?

🇺🇸United States justcaldwell Austin, Texas

Previous patch looks like pretty obvious credit mining and @Pracheth never returned to correct the format. No credit.

🇺🇸United States justcaldwell Austin, Texas

Makes sense. Merged with a new release coming shortly.

Thanks for contributing!

🇺🇸United States justcaldwell Austin, Texas

justcaldwell made their first commit to this issue’s fork.

🇺🇸United States justcaldwell Austin, Texas

No problems with manual testing. Merged the minimal change and will tag a new release soon.

🇺🇸United States justcaldwell Austin, Texas

justcaldwell made their first commit to this issue’s fork.

🇺🇸United States justcaldwell Austin, Texas

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?

🇺🇸United States justcaldwell Austin, Texas

This is a good idea. Are you thinking layout id should be on the current report somewhere, or would this be a new report?

🇺🇸United States justcaldwell Austin, Texas

Did some manual testing on D11. All seems fine. Merged to dev, release to follow.

🇺🇸United States justcaldwell Austin, Texas

Thanks for the contribution! In general, I like the changes proposed here. I think there are some minor issues that need discussion/work:

  1. 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.
  2. 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?
  3. The 'Reset' button doesn't clear Inline Block Type if a value has been set.
🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

MR created as promised 🙂

🇺🇸United States justcaldwell Austin, Texas

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.

🇺🇸United States justcaldwell Austin, Texas

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 ).

🇺🇸United States justcaldwell Austin, Texas

Given no other input, I'm going to close this one. Thanks!

🇺🇸United States justcaldwell Austin, Texas

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 the name 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.

🇺🇸United States justcaldwell Austin, Texas

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.

Production build 0.71.5 2024