Thanks for reporting the issues in #34!
I am able to establish that on Drupal 10.4.8 (i.e., CKEditor < v45) with Editor Advanced Link 2.2.6, the problem described of populating link attribute values in an new link from an old link is also present. Therefore I conclude it shouldn't be considered in the scope of this issue.
I think we need to define that in a separate issue (potentially redefining 🐛 Creating multiple link, the Title field is REUSED Active to include the larger scope described here), and provide specific steps to reproduce. (For me, it seemed like I needed some of the fields in the first link to be blank for the values to populate the second link.)
Separately, in my testing on Drupal 11.2 with this branch, I was unable to reproduce a scenario where the attributes get lost. Could you provide more specific steps to reproduce, ideally with just Editor Advanced Link (not Linkit or CKEditor Plugin Pack v1.4.1 with the Link Attribute), to help determine whether this is related to the CKEditor v45+ update or not?
Okay, based on discussion in 293-294, I've reworked the logic **not** to use the entity: prefix in the href. I also updated test expectations in this MR to match the new syntax. Those tests are passing, but I'm not clear why some low level Unit tests seem now to be failing?
Thanks for asking about this. My general sense of the situation is that machine-based authentication methods have changed significantly since the time that this module first advertised the ability to embed content from Facebook.
I only became a maintainer of this module in the past couple years, so I can't speak to what extent the module _previously_ supported Facebook embeds. What I can say, having worked with Meta-based Instagram integrations elsewhere, is that Meta's current methodology for allowing embeds requires a pretty sophisticated implementation of their API, with a client secret that generates an authorization session that provides an authentication handshake that results in a token for authentication, which then periodically must be refreshed/replaced to stay valid.
So, that's all to say that I really doubt it's as simple as inputting credentials into the existing provided interface in URL Embed.
We should probably do some deeper investigation that would result in either identifying the currently supported "simple" way to embed Facebook/Instagram, or identify that there is no "simple" way anymore and remove legacy indication of integration.
I found the same re #284 - the issue is the href loses the entity: prefix. I have no idea how this is happening.
Okay, so yes, this comes from filterXss(), as originally called out by Wim in #91. The design indicated there is that core.services.yml adds a protocol allowance for entity, along with an update to the default.services.yml. But if an existing site has its own services.yml file that does not include this, then the site will strip out entity: and we have this problem, which is the scenario under which @acbramley presumably found it. If adoption of this new functionality requires existing sites to update their services.yml file, this seems like it could lead to a lot of headaches. At a minimum we need a big mention of this in the change record.
However, I'd like to double-check whether we even should be populating the href with entity: protocols in the first place. Doing so means that the markup stored will include non-functional href values that require the new text format filter (core/modules/filter/src/Plugin/Filter/EntityLinks.php) for them to be "fixed" so that they are functional. Consider what happens if a content editor switches existing markup from a text format that has enabled the entity_links filter to one that doesn't? A bunch of broken links.
It would seem safer to populate a functional relative link (so, instead of entity:/node/1, just /node/1), basically as a fallback. For rendering that does use core/modules/filter/src/Plugin/Filter/EntityLinks.php, that code will update the href anyway from the metadata in the data attributes.
Accomplishing this change would involve apparently trivial modifications as shown below. What I'm not sure about is whether there is some other important reason to use the entity: protocol in the href? Security?
Diff of involved changes
diff --git a/core/modules/ckeditor5/src/Controller/EntityLinkSuggestionsController.php b/core/modules/ckeditor5/src/Controller/EntityLinkSuggestionsController.php
index 37fab1b00a2..41cf678207d 100644
--- a/core/modules/ckeditor5/src/Controller/EntityLinkSuggestionsController.php
+++ b/core/modules/ckeditor5/src/Controller/EntityLinkSuggestionsController.php
@@ -238,7 +238,7 @@ protected function createSuggestion(EntityInterface $entity): array {
// @see \Drupal\Core\Url::fromEntityUri()
// @see \Drupal\Core\Entity\EntityLinkTargetInterface()
// @see \Drupal\filter\Plugin\Filter\EntityLinks
- 'path' => sprintf('entity:%s/%s', $entity->getEntityTypeId(), $entity->id()),
+ 'path' => sprintf('/%s/%s', $entity->getEntityTypeId(), $entity->id()),
];
}
@@ -309,7 +309,7 @@ protected function computeGroup(EntityInterface $entity): string {
* An entity ID parsed from the user input, otherwise NULL.
*/
protected static function findEntityIdByUrl(string $target_entity_type_id, string $user_input): ?string {
- $expected_url_prefix = "entity:$target_entity_type_id/";
+ $expected_url_prefix = "/$target_entity_type_id/";
if (str_starts_with($user_input, $expected_url_prefix)) {
return substr($user_input, strlen($expected_url_prefix));
}
Thanks for explaining the root problem in #10, @mstrelan -- which explains why the MR only shows changes to the build artifact. Setting to RTBC!
Thanks for the further information. I'm able to reproduce it in the context described in #4.
I've suggested some revisions to the proposed change; there was already a position: relative; declaration in place for the target, so I don't think we need/want to add another one with an !importnat flag; we just need to change the scoping of the CSS. The important thing to verify is that this works "correctly" in 3 different contexts:
1. Linkit autocomplete as used within a CKEditor field within the Layout Builder settings tray (it
2. Linkit autocomplete as used with CKEditor outside of Layout Builder -- e.g., on a normal "node" field.
3. Linkit autocomplete as used with a Link field configured to use the Linkit widget.
The updated MR meets these scenarios in my testing. I'd appreciate a confirmation from someone.
Our custom matcher for Linkit completely broke after updating from 7.0.4 to 7.0.5. We eventually traced it back to seemingly undocumented significant structural changes to the constructor and create methods between these two minor patch releases. Please don't do that.
I'm sorry to hear this. As a fellow developer, I feel for you having to sort through something that broke that wasn't your fault, and that was seemingly released in an irresponsible way. I do want to clarify that the changes referred to between the 7.0.4 to 7.0.5 release were only understood to be code syntax rule fixes. In reviewing that work, it simply did not occur to me that switching to dependency injection would affect implementers of the Linkit Matcher class -- and this is the first I've heard of it being a problem. It was not our intent to introduce compatibility-breaking changes to implementers of Linkit classes, so providing the changes (as we understood them) in a patch-level release was semantically correct. In other words, our intentions were pure :) .
Thanks for sharing, and thereby empowering me to be a better maintainer of community-powered code!
Thanks for creating this issue. I am unable to reproduce the problem using Drupal's "Standard" installation profile, after enabling Layout Builder and configuring CKEditor/Linkit, using the default theme provided by core. Can you provide more specific steps to reproduce? Is it possible that you're using a custom theme that may be involved here, for instance? Thanks!
Having given this thought, and speaking only to what I'm going to do for Linkit, I'm going to change the version constraint in the info.yml file from >=10.1 to ^10.1 || ^11 as originally proposed by @nicxvan.
My reasoning: as stated in #4, I was originally adopting the open-ended future-major-version >=10.1 because I was also vouching that as a maintainer, I would always ensure that the latest release of Linkit was compatible with the latest stable release of Drupal core (i.e., before Drupal 12's release date, the latest release of Linkit would have API compatibility with Drupal 12, and so on). But if I'm vowing to be a maintainer that does that, then it's hardly much more work on my end to update the version constraint as part of module maintenance.
Thanks, everyone!
Is there functionality from linkit that won't be covered by this change? Just to help evaluate using this later.
Good point. I'd created a list of functional differences between this and Linkit awhile back shown below. This list is probably outdated at this point, but it's a start. We should do a comprehensive catalog.
- Ability to configure which types of links will be shown in autocomplete suggestions, and how to group them
- Ability to create multiple Linkit "profiles" to be used on different text formats
- Linkit-styled autocomplete suggestions for link fields (core already has this, but it's entity suggestions don't have the configuration options below)
- Ability to configure metadata (with token support) for autcomplete suggestions; for example, showing the authoring information, published state for each suggestion
- Ability to configure the number of suggestions shown
- Ability to toggle published/unpublished entities in suggestions
- An extendable plugin type for "Matchers," which determine what types of links can be suggested. Linkit includes a front page matcher, external link matcher, email address matcher, as well as matchers for user, taxonomy, media, and contact form entities.
- An extendable plugin type for "Substitutions" to customize what a match should render as
- For media entities, the ability to configure whether to link to the media entity URL or direct download
Thanks for raising this for discussion. I'm happy to have a conversation with the community about this. The version constraint indications in Linkit are by design. I'll explain my reasoning, below; I don't expect everyone to agree with the judgment call, but I would appreciate it if people participating in the conversation stay open to understanding it!
As of right now, Drupal 11.x is the latest supported major version of Drupal core. Linkit is compatible with this version. So as regards Linkit's version constraint of >=10.1, this is technically accurate for the available full major version releases of Drupal core.
An analogy to the version constraint methodology I'm adopting could be an "innocent until proven guilty" approach, rather than a "guilty till proven innocent" approach.
Okay, but why? In my experience, as a site maintainer depending on many Drupal contributed modules, the greatest impediment to being able to update sites to the latest supported version of Drupal core is that many contributed modules do not receive timely action for updating their module to be compatible with the major version release of Drupal core. Often that compatibility is easily resolved through a patch. But if there isn't a tagged release of the module that is compatible with the latest version of Drupal core, as a site maintainer, you then have to use a hack such as mglaman's drupal-composer-lenient project, or fork the contrib module.
In my experience, this reality is more common and more problematic than a scenario where a site tries to update to the next major version of Drupal core and runs into a contrib module that has code that is incompatible; in this case, the site maintainer can take an action of waiting to update to Drupal core, or better, provide a patch to make the contrib module compatible.
In other words, the control is owned by the site maintainers, rather than with the contrib module maintainer.
So that's my judgment call: I'm trying to empower the community. The position that "Linkit can't know that this current version supports 15" seems squarely a philosophical problem rather than a functional problem. As a maintainer, I am proactively addressing API changes as they come for Drupal 12 and Drupal 13, as in 📌 Add support for PHP Attributes in Matcher and Substitution plugin types Active . I vouch that I will continue to do so, and that Linkit will have a release that is compatible with future major versions of Drupal core prior to their release. As a result, there should never be a functional untruth about the version constraint.
Thanks again for reading!
Could this be everyone's old friend #2544110: XSS attribute filtering is inconsistent and strips valid attributes ?
Indeed. Manual experimentation shows that with "Limit allowed HTML tags and correct faulty HTML" disabled, this href stripping does not occur.
Well, kudos to @acbramley: bugs in #284 and #285 appear to be completely separate from the bug in #276 -- I confirmed that both problems existed prior to the most recent round of code changes -- so Bug Bounty x2 for you!
#285 is resolved with commit 44cbfae0; we need to account for the scenario where the autocomplete is triggered, but no change is made to the selection, in which case the data attributes need to be retrieved from state, rather than the event.
#284 is more weird! Data such as href="entity:node/1" is saved to the database, but upon load -- apparently prior to any JS code executing from our plugin -- CKEditor converts this to href="node/1", and things fall apart from there. Not sure if this is CKEditor validation?
At step 6 the link has now changed to foo so all data-* attributes were stripped, and the href is busted (but the link text did get updated)
Hrm! I'm unable to reproduce this using (I think!) the same steps from #276. Here's a screenshare of what I'm doing, so we can triangulate what the trigger is: https://www.drupal.org/files/issues/2025-10-21/data-attributes.gif →
happy for you to push that change to the main MR
Done. Thanks so much for work on the test cleanup. I'm working on remaining eslint violations now...
Any chance we can get a patch file that applies to version 2.3.2
Sure, uploaded here: https://www.drupal.org/files/issues/2025-10-21/editor_advanced_link_cked... →
mark_fullmer → changed the visibility of the branch 3317769-fix-link-api-ckeditor45 to hidden.
As the maintainer for Linkit, and a contributor to Editor Advanced Link, I want to draw attention to an API change introduced in CKEditor v45+, which is used on Drupal 10.5+ and 11.2+: the API now includes a "displayedText" input, as shown in https://ckeditor.com/blog/ckeditor-45-0-0-release-highlights/#smarter-li... . As a result, the way that link attributes are stored must change in the args object. Now, all attributes must be stored in args[1], namespaced, while the displayedText will go in the final args slot.
This resulted in multiple refactors required in the Linkit module and Editor Advanced Link to avoid breakage: 🐛 Editing Displayed text generates multiple links Active and 📌 Refactor custom JS for CKEditor5 v45+ Active
This problem is present in the current implementation staged for this issue, demonstrated in the video below:
Since there are substantial API changes, I've opted to present remediation as a separate commit on a separate branch/MR, at https://git.drupalcode.org/issue/drupal-3317769/-/merge_requests/1 , so that it can be reviewed in isolation. Ideally, it can be merged into the main MR.
This implementation is modeled after CKEditor5's own logic for links. See packages/ckeditor5-style/src/integrations/link.ts
Steps to reproduce problem shown above
- Create an autocomplete-based link in a node using a Drupal text format that has entity linking autocomplete enabled.
- Now click on the created link (do not select all link text, just click on it! this is called a "collapsed selection" in CKEditor parlance)
- Click the "Pencil" icons to edit the link
- Delete the existing displayed text in the link interface and enter replacement text
- Click "Update"
- Inspecting the inserted text (or switching to "Source" mode) shows that multiple adjacent links are present when there should be one.
Changing the priority from "Major" to "Critical," since Bug No. 2 in the issue description represents an easily reproducible scenario that results in data corruption that cannot be reversed without manual intervention.
mark_fullmer → changed the visibility of the branch 3534699-minimal to active.
mark_fullmer → changed the visibility of the branch 3534699-minimal to hidden.
I tested the MR with CKEditor5 v45+ (Drupal 10.5+) and could no longer recreate the problem
Woohoo!
We may want someone to test this in a 10.4 or earlier site before marking as fully "reviewed and tested" though.
I did test it in that context, myself. It would be nice to get another verification of this, though...
… and coincidentally there is now a module for that: Autoupdate Link Text.
Thanks for sharing! Created just today, huh?! It looks like currently that module only supports this autoupdating for link _fields_, as opposed to the more common usage of Linkit for links within CKEditor rich text areas (developing a methodology for updating those would presumably be a big task!). That should probably be called out in that module's project page more clearly.
Thanks for logging this integration consideration. One thing we should maybe consider is whether Drupal core's own solution for autocomplete-linking, in ✨ Drastically improve Drupal's default linking experience in text fields Needs work , is the best place for fully supporting Canvas. If Canvas was not supported in Linkit, that might nudge people to adopt the Drupal core solution, which would be good in the long run for everyone. Obviously, that issue hasn't been fixed yet, but per Laurii's comment here, it is a release target for Drupal CMS: https://www.drupal.org/project/drupal/issues/3317769#comment-16309183 ✨ Drastically improve Drupal's default linking experience in text fields Needs work
Thoughts?
Thanks! I've updated the release notes for 7.0.10. https://www.drupal.org/project/linkit/releases/7.0.10 →
Okay, merge request added that resolves this issue for both CKEditor5 v45+ (Drupal 10.5+, Drupal 11.2+) and CKEditor5 < v45 (older versions of Drupal).
On older versions where the CKEditor link interface does not include a "displayedText" input, this just restores the previous behavior:
- if a link is inserted from "scratch," the entity href is used as the link text, and the data attributes are present
- if there is preexisting text, that text is used as the displayedText and the data attributes are present
On versions using the newer CKEditor5 interface, with the "displayedText" input:
- now thanks to ✨ Display node title (a text) in by default when creating link in ckeditor5 Closed: duplicate , if a link is inserted from "scratch," the displayedText is prepopulated with the entity label, and data attributes are present; if the displayedText is then manually removed (comment #5 above), the link text uses the link href, and data attributes are present
- if there is preexisting text, that text is used as the displayedText and the data attributes are present
- In a weirder scenario, if there is preexisting text, and then the displayedText is manually removed, the previous displayedText is just kept; this seems like the safer behavior to me than replacing the text with the href value.
Thanks, @ericgsmith , for the follow-up MR. This does resolve the breakage on older versions of Drupal (with CKEditor5 < v45) but the data attributes aren't populated on those versions. I've committed this as a partial fix, and have staged what I think completes the work -- ensuring those attributes are present -- in 🐛 Data attributes no longer added to new link when displayed text empty Active
Thanks for reporting this, and for the forensics. This was introduced in https://git.drupalcode.org/project/linkit/-/commit/4a6ca22dc18beb8d232ae... , which was included in the 7.0.10 release. It fixed the critical bug in 🐛 Editing Displayed text generates multiple links Active , but it looks like it did not account for this edge case where the displayed text is not present.
The addition in ✨ Display node title (a text) in by default when creating link in ckeditor5 Closed: duplicate does partially fix this by supplying displayedText when it's not present, but based on comment #5 above, it's not quite robust enough.
One resolution would be to retain the previous logic for calculating the "range" when the selection is collapsed AND there is no displayedText. This would presumably work for sites using CKEditor5 < v45 and for those on v45+.
The other resolution would be to go a bit further with the approach in ✨ Display node title (a text) in by default when creating link in ckeditor5 Closed: duplicate and populate the displayedText later on in the pipeline, when it's writing the attributes; that way, the range could be calculated consistently using the new methodology introduced in 🐛 Editing Displayed text generates multiple links Active . That also should, in theory, work for all versions of CKEditor 5, and would be simpler in code...
Thanks for confirming! Setting to "Works as designed"!
Setting to "Needs work" based on user testing in #7.
I created an issue in the Drupal core queue for the remaining problem with the Styles dropdown: 🐛 [CKEditor v45+] Using the Styles dropdown for a link results in multiple links in markup Active . Setting this back to "Fixed," as there is no known issue originating in Linkit.
The narrow changes based on the refactor in #31 make good sense for the CKEditor version 45 API. I'm happy to merge this in, with the understanding that it will be applicable for sites running Drupal 10.5+ or 11.2+, and will not introduce backwards-compatibility-breaking problems for sites on older versions of Drupal. Thanks, everyone!
Our error trace shows either "layout_builder_at" or "layout_builder_restrictions" is the source of the error.
I can report that I probably tested this in conjunction with Layout Builder Restrictions, when I added Steps to Reproduce in #3. I'll try to test it in isolation and report back...
It sounds like what is being reported in comment #7 and #8 is actually the issue 🐛 Link attributes don't save Needs review . Based on this, it sounds like this issue should be set to "Closed (works as designed)" and attention should be focused on the other issue, which is already marked "Critical."
Okay, I've updated the merge request to incorporate the changes from the latest 2.3.x. Tests are green! Setting to "Needs review." Since this is a complicated issue history, to summarize, the things to review are:
- [ ] Bugfix: Clicking a link, updating the displayed text, then saving no longer results in multiple links being created in the markup (see full Steps to Reproduce in issue description, above; video demonstrating problem at https://www.drupal.org/files/issues/2025-10-09/editor-advanced-link-dupl... →
- [ ] Bugfig: Store Editor Advanced Link attributes in a dedicated key in args[1]. This is important so that Editor Advanced Link doesn't overwrite other arguments coming from other providers, and was the thing originally identified in this issue as a problem: https://www.drupal.org/project/editor_advanced_link/issues/3534044#comme... 🐛 Adding "Open in new window" removes all other attributes Active . See also problem described in 🐛 Initialize advanced attributes (`id`, `rel`) on link dialog open for new links in CKEditor 5 Active
- [ ] The "Title" attribute was erroneously omitted from the "Advanced" grouping in the UI. With CKEditor version 45 now having a "Displayed Text" field present, to reduce confusion, the "Title" attribute should be moved into the "Advanced" grouping
The problem reported in #10 could be due to the user using other tools that also modify link attributes, tools that have not updated to reflect saving logic in CKEditor version 45. The MR in 📌 Refactor custom JS for CKEditor5 v45+ Active should resolve this by storing link attributes in a keyed element within the other arguments. Specifically, it changes the storage from:
evt.editorAdvancedAttributes
to
args[1]['editorAdvancedLinkAttributes']
Compare https://git.drupalcode.org/project/editor_advanced_link/-/blob/2.3.x/js/... to https://git.drupalcode.org/project/editor_advanced_link/-/merge_requests...
So it seems that the changes made here do not actually fix the issue. I Initially thought they did because visually there was no issue for me and when inspecting the source code there was only 1 link instead of 3....If I disabled linkit in the format settings the problem is fixed.
So the default link functionality from Drupal core does work correctly.
Thanks for the due diligence here! I am able to reproduce the problem without Linkit installed on a generic Drupal site, as shown in the screencast below, and the steps to reproduce. If someone else can confirm that they can reproduce the issue using Drupal core without any other appurtenances, we should report this in the Drupal core issue queue.
Screencast of problem
https://www.drupal.org/files/issues/2025-10-08/drupal-core-link-collapse... →
Steps to reproduce
1. Install Drupal core (11.2.x) using the "Standard" installation profile
2. Go to admin/config/content/formats and choose the "Basic HTML" text format
3. Add the "Style" dropdown to the CKEditor 5 toolbar
4. Under the CKEditor plugin settings for "Style" add a.btn|Button
5. Create a Basic page and add a link using the Link toolbar (text: "text", link: "https://drupal.org"), and insert the link into the content.
6. Use the "Styles" dropdown to apply the "Button" class to the existing link.
7. Important here!: Click somewhere within the text of the link (do not highlight the link).
8. Click the "Link" icon in the toolbar.
9. Use the link "Pencil" icon to change the displayed text of the link.
10. Press the "Update" button in the link balloon.
11. Inspect the page source. There are now two links, divided by the text, when there should be one.
The fix in the merge request on Linkit's issue should be a good model for Drupal core to fix the problem. This is a problem introduced in CKEditor 5 due to the new presence of the "displayedText" input element. All implementers that are modifying link attributes now must use a different range calculation for a "collapsed" selection (i.e., when someone just clicks the link (Step 7, above).
Based on the multiple RTBCs above and my own testing with Drupal < 10.5 to confirm backwards compatiblity, I'm going to proceed to merge this and cut a release. Thanks, everyone!
I am on Drupal 10.5.3 with the modules "Linkit" in version 7.0.9 and "Editor Advanced Link" in version 2.3.1. I had to install both the patch from this issue as well as the patch from the companion Editor Advanced Link issue
Yes, thanks for pointing that out. Confirming that for people using the Linkit and Editor Advanced Link modules, the same problem is present in both module and both need to be patched/fixed to avoid the multiple links problem.
Working on the other issue is a better long term approach I think.
I tend to agree. In the meantime, here's a patch that is compatible with the 8.x-3.8 release (October 2025).
I agree that the change introduced in 🐛 Comments stopped working after updating to Mime Mail 2.0.0 combined with Comment Notify Active is most likely the root cause. Could you provide more specific steps to reproduce triggering an email via the Comment Notify module? If so, I think this should be a pretty straightforward fix.
Okay, I was able to figure out the issue described in #20!
The approach in the existing merge request, as well as recent patches, include a large number of naming changes that are not required for providing compatibility with CKEditor version 45. To simplify the maintainers' review process, and to expedite the hopeful fixing of this issue, I've created a new merge request only with the changes required for compability with CKEditor v45. I did not update the automated tests since that work is happening in ✨ Fix CI issues Active .
The merge request https://git.drupalcode.org/project/editor_advanced_link/-/merge_requests/43 accomplishes the following specific goals:
- [ ] Relocates where the Editor Advanced Link attributes are stored and passed (now in a dedicated key in args[1]). This is important so that Editor Advanced Link doesn't overwrite other arguments coming from other providers, and was the thing originally identified in this issue as a problem.
- [ ] Relocates the "title" attribute form input into within the "Advanced" group.
- [ ] Fixes the problem described in #14 and #20, where clicking a link and then editing it results in multiple links being created in the markup
Note that Drupal 10.5/11.2 updated to CKEditor version 45, which has a redesigned link interface that includes a new input element for the displayed text: https://ckeditor.com/blog/ckeditor-45-0-0-release-highlights/#smarter-li...
The work here will need to be updated to accommodate this version of CKEditor's interface
I've refactored the relevant code to handle the scenario below. I would greatly appreciate followers of this issue to test the merge request with their sites to confirm that the following is fixed:
- Create a link using a CKEditor-enabled rich text area with Linkit.
- Now click on the created link (do not select all link text, just click somewhere inside it)
- Click on Edit link
- Update the link source using Linkit's autocomplete functionality
- Delete the existing displayed text in the link interface and enter replacement text
- Click "Update"
- Inspecting the inserted text (or switching to "Source" mode) shows that only a single link is present, not multiple adjacent links are present when there should be one.
mark_fullmer → changed the visibility of the branch 3540235-ckeditor-v45-editing to hidden.
After this change I've been getting the following AJAX error upon adding a new section using VLSuite module.
Thanks for pointing this out. I've added the proposed check!
Thanks for reporting this. The root cause has to do with a change in Symfony from version 6 to 7, where Normalizers expect a getSupportedTypes() method to be present which specifies what things the given normalizer is eligible to normalize. It defaults to *, so what was happening was that the ReferenceNormalizer was being considered eligible for denormalization when Bibcite was trying to denormalize Contributor entities. The merge request resolves this issue; I need to double-check that this doesn't introduce backwards compatibility issues for earlier versions of Drupal...
Quoting an issue from comment #14 that still is present in with the current changes:
When I click on it and edit an attribute (e.g. "Open in new window"), the link is temporarily split up into 3 links: one for the part to the left of the cursor, one invisible filler link (containing only 7 ⁠ characters), and one for the part to the right of the cursor.
I'm investigating a similar regression in the Linkit module 🐛 Editing Displayed text generates multiple links Active and have isolated the problem to how the code determines the "range" for applying attributes, specifically when the link is selected by "clicking"; in CKEditor terminology this is referred as a "collapsed selection."
In Editor Advanced Link, the problematic code is here: https://git.drupalcode.org/project/editor_advanced_link/-/blob/2.2.x/js/... . When the selection is not collapsed (i.e., the link was selected by highlighting the entire link), the "range" is calculated correctly.
I haven't yet figured out the appropriate refactor for Linkit, but wanted to provide a heads up for this gnarly problem to others working on this refactor.
mark_fullmer → changed the visibility of the branch 3536202-add-a-option to hidden.
Thanks for the creation of this issue, and for the maintainer's input and solicitation for feedback. I think this is a high-value issue, since it has to do with regulating the ability to use Drupal local login. This matters because local login has security implications. A site manager may create new accounts intended solely to be accessed with SSO authentication, and may create weak placeholder local passwords. The current design of the module means that until a user has successfully authenticated with SSO, those local credentials are still a valid method for sign in.
"...If you create single new Drupal accounts one by one, maybe there needs to be a new drush command
As a developer supporting a large organization, I would posit that creating single new Drupal accounts one by one -- performed by non-developers -- is by far the most common scenario. Therefore, I do not believe a Drush command is scalable enough, as a Drush command would assume that the person creating accounts has access to the command line.
my current opinion is that it should NOT be possible to edit auth-mappings through the Drupal UI; there only is an already-existing UI to view and delete them. So I believe that just this drush command without any UI equivalent, is good. But that's just my opinion; maybe other people want a form to add these.
I agree with the first statement: auth mappings, themselves, should not be directly editable. However, I think that a module that is providing an alternate sign in methodology for Drupal local login *is* implicitly responsible for allowing site managers to individually control whether or not user accounts can still use Drupal local login. Since the SAML Authentication module design currently determines whether or not local login is possible based on the presence/absence of auth mappings, in effect, it needs to either (a) allow site managers the ability to perform an action that adds that auth mapping or (b) allow site managers to use a different mechanism to limit local login.
Based on the above, I offer two resolutions that do not change the existing design of the SAML Authentication module, but do allow site managers the ability to control local login. Both of these approaches could be adopted, if desired -- they are not mutually exclusive.
Option 1: Add a setting to the user registration form to associate newly created accounts with SAML authentication
This setting allows site managers to choose whether new accounts are SAML-based, rather than "waiting to see" whether someone tries to sign in with SAML. In conjunction with the existing "Roles allowed to use Drupal login also when associated with a SAML login," this would allow site managers to create new accounts that are only eligible for SAML authentication, or create accounts that are eligible for both Drupal and SAML authentication. Stated as a use case, this approach would be most appropriate for sites that want basically all users to only be able to use SAML authentication but still want to retain some exceptions, using Drupal roles to control those exceptions. This is the approach that the SimpleSAMLphp Authentication module takes. Note that as shown in https://git.drupalcode.org/project/simplesamlphp_auth/-/blob/8.x-3.x/sim... , it uses the External Auth module's linkExistingAccount method to perform the action.
Limitations:
1. This does not provide a way to update existing accounts (Option 2 does).
2. This does not provide a programmatic way to link new (or existing) accounts to SAML authentication. Therefore, a Drush command would still be a valuable addition to the External Auth module.
This is implemented in https://git.drupalcode.org/project/samlauth/-/merge_requests/34
Option 2: Add a setting to limit Drupal login to specified roles, regardless of SAML association
This setting would allow sites to effectively tweak the existing setting for "Roles allowed to use Drupal login" to apply to all accounts. In other words, if desired, sites could set local login capability to no longer be dependent on whether a record exists in the authmap table. This new setting, disabled by default, would not change the behavior of existing sites. It would allow existing sites immediately to "lock down" Drupal local login to specified roles as described in this issue's original writeup.
This is implemented in https://git.drupalcode.org/project/samlauth/-/merge_requests/35
mark_fullmer → made their first commit to this issue’s fork.
mark_fullmer → made their first commit to this issue’s fork.
Yep, these can be removed at this point in time. Thanks, folks!
Okay, thanks! Setting this to "Needs review," now that we have a merge request that shows tests passing. What we need is clear steps to reproduce in the UI, which I understand by the original description are hard to define, since the problem seems intermittent and inconsistent. Let me know how else I can help!
We are also observing this error where autocomplete is not shown or executed at all. We upgraded from drupal/core 10.4.3 and linkit 6.14 to Drupal core 10.5.3 and linkit 7.0.9 version. The issue still persist even if we just downgrade the linkit version and keep Drupal core 10.5.3, so I presume it has something to do with the Drupal update as well.
The console is showing a JS error, after clicking the linkit button:
Uncaught CKEditorError: Cannot read properties of undefined (reading '_items')
A few things that might help disentangle this:
1. Drupal 10.5+ upgraded to CKEditor version 45, which introduced changes to the Link plugin. The only version of Linkit that is compatible with Drupal 10.5+ (and Drupal 11.2+) is Linkit version 7.0.7 or higher.
2. We have automated test coverage that shows that Drupal 10.5.3 + Linkit 7.0.9 works, and I can personally confirm that it is working on a fresh installation. If the site you're working on is having issues, then I think it's got to be something about the site configuration, rather than the module. You can confirm this by trying Linkit with a fresh installation of Drupal. You could also try creating a new text format using Linkit and seeing if that works on your site, just as a debugging step.
As a maintainer, I maintain that the change that makes Linkit only active if it has been enabled is *by design* and follows Drupal expectations for text format filters, and the steps outlined in https://www.drupal.org/project/linkit/issues/3546851#comment-16279928 🐛 Linkit module broken in CKeditor 5 after updating to 7.x Active should allow this to work on existing sites that had previously installed Linkit.
Thanks for reporting this. I'm thinking that the appropriate fix here, based on what you've outlined, would be to add a cache context to the block so that the render caching is able to vary by user. Something along the lines of:
diff --git a/src/Plugin/Block/GoogleSearchBlock.php b/src/Plugin/Block/GoogleSearchBlock.php
index 39e7029..56c8f6a 100644
--- a/src/Plugin/Block/GoogleSearchBlock.php
+++ b/src/Plugin/Block/GoogleSearchBlock.php
@@ -178,6 +178,7 @@ class GoogleSearchBlock extends BlockBase implements ContainerFactoryPluginInter
// This conditional handles search configurations that were deleted.
return;
}
+ $form['#cache']['contexts'][] = 'user.roles';
$plugin = $entity->getPlugin();
$google_markup = $plugin->buildResults();
$config = $this->configFactory->get('search.page.' . $search_id);
Would you be able to see if the above method works for your use case?
Great idea, and awesome start on the documentation. I've created the project page here: https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... → . I'll continue to work on building out the documentation.
Thanks for the logo! This has been added to the 8.x-3.x branch (still the active one!).
We've decided to drop the transpilation -- 📌 Update build dependencies to work with a supported Node version Active -- so I'm going to close this as outdated. Thanks for raising it as a concern, though!
The issue 🐛 Widget empty if the default value is an integer Active has been merged. The other maintainers above asked for what scenario triggers the problem, and the issue referenced supplies at least one context -- usage with the Webform module. Given that the merged change now supports arrays of IDs, I'd like to set this issue to "Postponed" and ask if the original reporter can confirm if the issue is now fixed for them, with the latest changes in the latest release. Thanks!
Interesting. I'm not sure how common this module is used with Webform (though there is at least one other issue in the queue about its usage -- 🐛 media_library element inside webform_multiple error Needs review -- but in any case, yes, the rendering of the description here should definitely support merging with other descriptions that are render arrays. The code implementation is fine, and doesn't negatively affect the existing implementation, so it makes sense to add this. Merged, to be included in the next release!
Tests are now passing in HEAD. Marking this as outdated.
mark_fullmer → made their first commit to this issue’s fork.
Okay, this makes sense to me, and I'm fine with adding support for default value being an integer and array of IDs. The code change proposed doesn't negatively affect the existing business logic, so this doesn't pose a risk for backward compatibility. Merging, and including in the next release. Thanks!
mark_fullmer → made their first commit to this issue’s fork.
Updating to include the latest commits on the 8.x-3.x branch -- let's see how test coverage looks....
mark_fullmer → made their first commit to this issue’s fork.
There is no active development happening in the 5.0.x branch. The current active branch, 8.x-3.x, has description properties for the info.yml files. Closing as outdated.
This change is long overdue, and when this MR is updated to include the latest commits on 2.x, tests are passing. The code changes look great. Merging!
After updating to the latest commits from 8.x-3.x, tests are passing. This call to a non-defined object property is indeed no longer allowed in PHP 8.2, so this change makes good sense. Thanks!
is there any other way to overcome with this issue?
Can someone please clarify.
This has been removed and will be included in the forthcoming 2.1.3 release!
mark_fullmer → made their first commit to this issue’s fork.
Yes -- good call! At this point, we should use $address->getDisplayName()
Merged & fixed!
mark_fullmer → made their first commit to this issue’s fork.