Tucson
Account created on 3 June 2013, about 12 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States mark_fullmer Tucson

Thanks for the rework! Assigning myself for review...

🇺🇸United States mark_fullmer Tucson

example of two modules that are removing LinkIt attributes when used:

Okay, thanks for helping connect the dots, @chrisla. Per https://www.drupal.org/project/editor_advanced_link/issues/3534044#comme... 🐛 Adding "Open in new window" removes all other attributes Active , another developer suggests that, contrary to intuition, this is not the responsibility of those modules to not remove other attributes, but rather, the responsibility of LinkIt to ensure the "persistence" of its attributes. I'll start looking into that.

🇺🇸United States mark_fullmer Tucson

As this is an opt-feature, I feel comfortable merging this in; should there need to be refinements, we can work those out subsequently.

🇺🇸United States mark_fullmer Tucson

Noting that this library now uses seboettg/citeproc-php, which declares a dependency on citation-style-language/locales, which has tagged releases. I can confirm that this module can be installed when the composer minimum-stability is set to beta; it still doesn't support stable since there is no full release of Bibcite -- we should remedy that!

🇺🇸United States mark_fullmer Tucson

After thinking about this further, I think that this is a common, reasonable request that should not require a developer in order to accomplish. I therefore propose adding a new, global setting to Bibcite for "Convert URLs into hyperlinks".

When enabled, this will pass the already-processed citation through Drupal core's _filter_url() function, which is robust and designed to handled a wide range of HTML content.

This approach does not give sites the ability to have some URLs be hyperlinks and others not be hyperlinks -- that, I feel, still should be the purlieu of developers and custom code.

🇺🇸United States mark_fullmer Tucson

This should no longer be an issue in 2025, using the standard Composer installation process. Closing as outdated.

🇺🇸United States mark_fullmer Tucson

Link updated in project page. Closing as complete!

🇺🇸United States mark_fullmer Tucson

I've confirmed that I can reproduce the problem and that the original proposed solution works sufficiently. Committed and included in today's 3.0.1 release of the module: https://www.drupal.org/project/iframe_title_filter/releases/3.0.1

🇺🇸United States mark_fullmer Tucson

Thanks for revisiting this, and the companion 🐛 title attribute still not added to oEmbed iframe Postponed . Associating this issue with the latest version branch of this module...

🇺🇸United States mark_fullmer Tucson

mark_fullmer changed the visibility of the branch 3073895-blocks-unique-id to hidden.

🇺🇸United States mark_fullmer Tucson

mark_fullmer changed the visibility of the branch 3073895-system-menu-blocks to hidden.

🇺🇸United States mark_fullmer Tucson

Maintainer of Responsive Tables Filter here, chiming in. I think the problem stems from the fact that the reponsive tables logic wraps the "Reset" link in a span tag, as shown below, and the JavaScript in Editoria11y assumes that the <a> is the immediate sibling of the <td> element here: https://git.drupalcode.org/project/editoria11y/-/blob/2.2.x/js/editoria1...

<span class="tablesaw-cell-content">
  <a role="button" title="Reset this dismissal" class="ed11y-reset-this-dismissal action-link action-link--icon-trash">
    <span hidden="" class="ed11y-api-by">1</span>
    <span hidden="" class="ed11y-api-path">/node/1</span>
   </a>
</span>

Ideally, the targeting JS in Editoria11y can be more tractable and the two modules can play nice together.

🇺🇸United States mark_fullmer Tucson

Thanks for reporting this. I was able to reproduce the conflict described. My review of the code suggests that the adjustment should happen on the `editoria11y` front. The problem stems from the fact that the reponsive tables logic wraps the "Reset" link in a span tag, as shown below, and the JavaScript in Editoria11y assumes that the <a> is the immediate sibling of the <td> element here: https://git.drupalcode.org/project/editoria11y/-/blob/2.2.x/js/editoria1...

<span class="tablesaw-cell-content">
  <a role="button" title="Reset this dismissal" class="ed11y-reset-this-dismissal action-link action-link--icon-trash">
    <span hidden="" class="ed11y-api-by">1</span>
    <span hidden="" class="ed11y-api-path">/node/1</span>
   </a>
</span>

I'll leave a similar comment in 💬 Conflict with Responsive Tables Filter & clearing dismissals Active

🇺🇸United States mark_fullmer Tucson

Thanks for filing this report! I may need some help understanding the intent here, as my first analysis suggests this works as designed.

The description above states that "In Drupal 11, using CKEditor 5, libraries attached via $result->setAttachments() in a filter plugin are not automatically rendered on the page unless they are explicitly declared in the text format’s configuration (filter.format.*.yml)."

Would it be more accurate to say that the library is not attached unless the text format filter is executed as part of the page's rendering process?

If not, can you point to a Drupal change record that spells this out, and/or an example in Drupal core or contrib where this is the case? I find it very plausible to expect that `$result->setAttachments()` won't execute on a page if the the filter format plugin isn't executed on the page, but I don't follow the logic that a Drupal library would need to be explicitly declared in a text format configuration in order to allow `$result->setAttachments()` to execute.

Regardless, I don't find the Steps to Reproduce above to be true. When I create a text format with filter_responsive_tables_filter enabled and create a page that renders an HTML table with a header, I see the Tablesaw library loaded and working as designed.

Furthermore, the proposed code change doesn't seem like the expected logic. By adding a hook_preprocess_page() hook, as indicated in the comments, if the Responsive Tables Filter plugin is added to *any* text format, it will add the library to *every* page, regardless of whether that page renders any content using the text format. This almost seems like using a "back door" of a text format filter to add the Tablesaw library to the entire site, which is not the design of the module.

If sites do want to add the Tablesaw library to every single page, that's up to the developer I guess, and they could simply do the following, or add responsive_tables_filter/tablesaw-filter as a dependency to the site theme's library.

function mymodule_preprocess_page(&$variables): void {
    $variables['#attached']['library'][] = 'responsive_tables_filter/tablesaw-filter';
}
🇺🇸United States mark_fullmer Tucson

Thanks for the suggestion, and sorry about the headache. Callout added to https://www.drupal.org/project/linkit/releases/7.0.7

🇺🇸United States mark_fullmer Tucson

It would be nice to have a new release to get this bug fix without needing to apply a patch.

Thanks for the nudge! Done: https://www.drupal.org/project/layout_builder_restrictions/releases/3.0.4

🇺🇸United States mark_fullmer Tucson

I believe this is an issue with the Drupal module Editor Advanced Link, not Linkit, which only provides the autocomplete functionality. See https://drupal.org/project/editor_advanced_link

🇺🇸United States mark_fullmer Tucson

Will this fix be backported to 6.x? If not, then 6.x should be updated to require <10.5

Thanks. This will not be backported to 6.1.x. I've reflected this in the 6.1.8 core version compatibility: https://www.drupal.org/project/linkit/releases/6.1.8

🇺🇸United States mark_fullmer Tucson

This is due to a design change coming from CKEditor version 45 (see https://ckeditor.com/blog/ckeditor-45-0-0-release-highlights/#smarter-li... ). Basically, some types of link decorators are moved into a "Link properties" sub menu that is only visible and available **after** the original link has been added. This is not a change coming from the Drupal module, per se, but from CKEditor version 45. The "Open in new tab" option behaves differently than other options in Editor Advanced Link because it is defined differently, as a link decorator, in https://git.drupalcode.org/project/editor_advanced_link/-/blob/2.2.x/src...

🇺🇸United States mark_fullmer Tucson

I've merged a number of changes related to CKEditor v45 compatibility to reduce the surface area here. The originally reported problem still remains, so this is "Needs work."

🇺🇸United States mark_fullmer Tucson

Adding a max-width makes good sense here! Merging as-is. Tests are failing for unrelated reasons -- CKEditor v45 changes, to be addressed in 🐛 Cannot replace 'Displayed text' of existing links Active .

🇺🇸United States mark_fullmer Tucson

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

🇺🇸United States mark_fullmer Tucson

Hi Jannahka,

Thanks so much for digging into the CKEditor v45 changes. These are certainly a priority. I'd begun some similar work, refactoring tests and refactoring for the problematic behavior in 🐛 Cannot replace 'Displayed text' of existing links Active . Ideally, I'd like to make only the minimal changes necessary and see if it is possible to retain backward compatibility with previous versions of CKEditor so that we don't have to change the core_version_requirement in a new release.

To that end, it will be important to document exactly what is known to be breaking, with steps to reproduce, so that we can ensure that we're only changing what is necessary and not breaking other things.

🇺🇸United States mark_fullmer Tucson

Thanks for reporting this. I am able to reproduce. Specifically, using a generic installation of the Standard profile, adding Linkit to the Basic HTML text format:

1. Click the "Link" icon.
2. Enter a "Displayed text" value.
3. Enter the first character of an existing node and use Linkit's autocomplete to choose a result from the dropdown. Confirm that the internal path to the node is populated in the "Link URL" field.
4. Quickly(!) Remove the contents from the "Displayed text" input and quickly enter new text and press "Insert."
5. Link text will not be updated and the console will show the Uncaught CKEditorError: Cannot read properties of undefined (reading 'attributes') error.

Additionally, this is something else that doesn't work correctly.
1. Click the "Link" icon.
2. Enter a "Displayed text" value.
3. Enter the first character of an existing node and use Linkit's autocomplete to choose a result from the dropdown. Confirm that the internal path to the node is populated in the "Link URL" field.
4. Now click "Link URL" field again and enter the first character of another node and use Linkit's autocomplete to choose a different result.
5. The link field will NOT correctly populate the internal path to the node but will simply populate the character you entered.

🇺🇸United States mark_fullmer Tucson

This is the first time I am posting; unsure how to proceed. If appropriate I can supply some code snippets related to the info_alter option.

Code snippets in a comment would be a great start. I can then move them into a new page under the documentation at https://www.drupal.org/docs/contributed-modules/bibliography-citation

🇺🇸United States mark_fullmer Tucson

Seems a few other block--system-menu-block.html.twig in core. Is there a place we can add an assertion to?

Agreed that modifying the Twig templates is not the best approach here. Instead, we can supply the ID attribute to all templates through the SystemMenuBlock::build() method. The rendering logic for a unique ID can be modeled exactly on block/src/Hook/BlockHooks::preprocessBlock(), per below:

    // Create a valid HTML ID and make sure it is unique.
    if (!empty($variables['elements']['#id'])) {
      $variables['attributes']['id'] = Html::getUniqueId('block-' . $variables['elements']['#id']);
    }

I've created an alternate merge request, at https://git.drupalcode.org/project/drupal/-/merge_requests/12664

🇺🇸United States mark_fullmer Tucson

My previous comment was wrong. This is still an issue in Drupal core, though it only manifests in the context of placing a menu block using Layout Builder.

🇺🇸United States mark_fullmer Tucson

This appears no longer to be an issue as of 11.3.x. Placing a menu block on the page successfully generates unique ID attributes; placing a second instance of the same menu block confirms that the ID attributes and corresponding aria-labelledby attributes are unique. I haven't tracked down the core commit responsible for this yet, but will set this to "Postponed" given the above.

Example markup of first instance of menu block:

<nav id="block-olivero-mainnavigation" class="contextual-region primary-nav block block-menu navigation menu--main" aria-labelledby="block-olivero-mainnavigation-menu" role="navigation">
      
  <h2 class="block__title" id="block-olivero-mainnavigation-menu">Main navigation</h2>

Example markup of second instance of menu block:

<nav id="block-olivero-mainnavigation-2" class="contextual-region primary-nav block block-menu navigation menu--main" aria-labelledby="block-olivero-mainnavigation-2-menu" role="navigation">
      
  <h2 class="block__title" id="block-olivero-mainnavigation-2-menu">Main navigation</h2>
🇺🇸United States mark_fullmer Tucson

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

🇺🇸United States mark_fullmer Tucson

This analysis is comprehensive and logical, and the proposed resolution of placing the executing code in a try/catch statement to avoid a fatal error, modeled after the behavior in Drupal core, is safe and uncontroversial. Thanks so much for the report, the analysis, and the code change fix!

🇺🇸United States mark_fullmer Tucson

As of Drupal 10.5 and Drupal 11.2, version 45 of CKEditor is used, which places the Editor Advanced Link items in its own collapsible fieldset within the CKEditor Link editor. Given this, it is is likely that this proposed change is outdated. I'll not close the issue just yet, but I'm going to set its status to "Postponed" to evaluate the impact of version 45 of CKEditor.

🇺🇸United States mark_fullmer Tucson

Okay, changing this to a "Support request." Let's try to add some documentation that shows example usage of both HOOK__preprocess_bibcite_citation() and HOOK_bibcite_bibcite_processor_info_alter()

🇺🇸United States mark_fullmer Tucson

I believe the new version of CKEditor that is included with Drupal 10.5 caused some changes to the buttons.

Okay, thanks! That was the missing piece for me. I've added a change that should make the link editor width fit the available content.

🇺🇸United States mark_fullmer Tucson

Reviewing the use of the Gulp build process in this module and the resulting output, I conclude that it does so little that it is more of a liability to keep the build process in the codebase than it is an asset: there is a a fair amount of boilerplate code for SCSS that doesn't exist, and the JS is only very slightly modified. I propose that we work directly from the relatively small JS file in /assets.

If others in the community feel strongly that the build process should continue to be used, it can be reverted -- I'm not strictly opposed to that, but I will proceed to do the removal here in the absence of other strong opinions. Thanks!

🇺🇸United States mark_fullmer Tucson

This change creates a dependency on the presence of the media module for loading the form options, so we need to check if that module is active...

🇺🇸United States mark_fullmer Tucson

mark_fullmer changed the visibility of the branch 3381766-linkit-dialog-list to hidden.

🇺🇸United States mark_fullmer Tucson

I've confirmed that the recent series of patches do not correctly handle both scenarios that need to be supported. They fix the dialog when used in CKEditor, but break the dialog when it is associated with a Link field, as was indicated by berdir in #20.

I've created a new MR, https://git.drupalcode.org/project/linkit/-/merge_requests/113 , which scopes the absolute positioning to the Link field by excluding that CSS from the CKEditor context. Can others confirm that this resolves the display issues for them in CKEditor? (It does for me, but as has been observed in this discussion, part of the issue has to do with the order in which CSS files are loaded, which may differ depending on site configuration.)

🇺🇸United States mark_fullmer Tucson

I feel like the LinkIt popup is unnecessarily narrow. It would be nice if it could automatically expand a little wider on larger screens.

Can you clarify which version of LinkIt and CKEditor you are using? Based on the screenshot, this does not appear to be the latest version, unless you've added some custom CSS to render the "Insert" button differently?

🇺🇸United States mark_fullmer Tucson

Individual sites could also potentially rewrite such URLs as actual hyperlinks by implementing HOOK__preprocess_bibcite_citation(), which provides access to the already-rendered citation. Presumably you could use regular expressions to find/replace specific markup with hyperlinks. This would not be as elegant a solution as using Lambda functions to modify the citation processor itself, as described in https://github.com/seboettg/citeproc-php#use-lambda-functions-to-setup-c... , but might be more straightforward for some developers.

🇺🇸United States mark_fullmer Tucson

This change makes sense, is uncontroversial, and tests pass. Merging!

🇺🇸United States mark_fullmer Tucson

Okay, I'm going to merge this! Thanks everyone -- this was a long-running issue spanning 6 years!

🇺🇸United States mark_fullmer Tucson

Commit 2354202e adds test coverage to demonstrate that the following variations are matched successfully, to address the concerns in #115:

https://<domainname>/<internal-path>
https://<domainname>/<alias>
https://<domainname>/<subdomain>/<internal-path>
https://<domainname>/<subdomain>/<alias>
https://<domainname>/<language-prefix>/<internal-path>
https://<domainname>/<language-prefix>/<alias>
https://<domainname>/<subdomain>/<language-prefix>/<internal-path>
https://<domainname>/<subdomain>/<language-prefix>/<alias>
🇺🇸United States mark_fullmer Tucson

This meta issue was reviewed by me, a new maintainer in June 30. Each of the issues identified has been separately triaged and appropriate progress is being made. I'll therefore close this meta issue. Thanks!

🇺🇸United States mark_fullmer Tucson

This covers the testing (T) and I reviewed the changes (R) in Comment #6, so RTBC.

From one member of the C to another, thank you :)

🇺🇸United States mark_fullmer Tucson

Okay, thanks for the update. I'll go ahead and close this issue.

🇺🇸United States mark_fullmer Tucson

If you inspect the rendering of the title field, are the characters encoded or escaped? Also, if you can confirm that this is not happening on a basic node, such as "Basic Page," then this would seem to not be relate to Bibcite and you could search Drupal core issues for similar reports.

🇺🇸United States mark_fullmer Tucson

perhaps this can be mentioned in the README as a 'use at your own risk' as I think it is very usefull towards content editors?

Excellent suggestion. I've added a new documentation page at https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib...

🇺🇸United States mark_fullmer Tucson

To work with the nature of the Linkit configuration described above, I propose that a new setting be added to the text filter for setting "Media entity URL substitution", with three choices:

  1. Use metadata from when the link was created
  2. Direct URL to file
  3. Standalone media URL (e.g., /media/{id})

This will allow sites to update their text formats to normalize how media links are substituted. By choosing either option 2 or 3, they effectively instruct the processor to ignore the metadata value in the link and do one of the two available URL methods.

🇺🇸United States mark_fullmer Tucson

Okay, thanks. Closing this as out of scope for the Linkit module.

🇺🇸United States mark_fullmer Tucson

Following up on this, as an immediate workaround that would force rendering of any media matchers as direct URLs, people could modify the CKEditor text format filter as shown below:

diff --git a/src/Plugin/Filter/LinkitFilter.php b/src/Plugin/Filter/LinkitFilter.php
index 1b91e51..de9fc43 100644
--- a/src/Plugin/Filter/LinkitFilter.php
+++ b/src/Plugin/Filter/LinkitFilter.php
@@ -116,7 +116,9 @@ class LinkitFilter extends FilterBase implements ContainerFactoryPluginInterface
           if (!$substitution_type = $element->getAttribute('data-entity-substitution')) {
             $substitution_type = $entity_type === 'file' ? 'file' : SubstitutionManagerInterface::DEFAULT_SUBSTITUTION;
           }
-
+          if ($entity_type === 'media') {
+            $substitution_type = 'media';
+          }
           $entity = $this->entityRepository->loadEntityByUuid($entity_type, $uuid);
           if ($entity) {

A more robust solution that would determine what the site's current setting for a given matcher is and use that is complicated. As far as I know, Drupal text filters do not have access to know which text format executed the request for processing. They only have access to the text format's settings for the given text filter. Unfortunately, Linkit stores the matcher being used in the *editor* configuration of a text format, rather than in the text filter. This means that the executing code can't query "what Linkit matcher is being used with this configuration?"

Of course, the straightforward workaround would be to add a global setting that provides a default for the media substitution plugin. But since Linkit has zero global settings right now, I'm hesitant to create one for this purpose. Thoughts?

🇺🇸United States mark_fullmer Tucson

Marking this as a duplicate of 💬 Links surrounding Media Library item strips data-entity-type and data-entity-uuid Postponed . The conclusion stated in https://www.drupal.org/project/linkit/issues/3396049#comment-15450593 💬 Links surrounding Media Library item strips data-entity-type and data-entity-uuid Postponed still appears to be valid.

🇺🇸United States mark_fullmer Tucson

The "Result description" was renamed to "Metadata" in https://git.drupalcode.org/project/linkit/-/commit/3b8e2a33bf26f944ff2ae... , and that is included in the current schema. I'm going to close this issue as outdated. If this is still an issue for anyone, please don't hesitate to create a new issue!

🇺🇸United States mark_fullmer Tucson

This issue appears no longer to be an issue in the latest release of Linkit. As such, and given there has been no activity on this issue for more than 5 years, I'm going to go ahead and close it as outdated. If this is still an issue for anyone, please don't hesitate to create a new issue!

🇺🇸United States mark_fullmer Tucson

Thanks for the suggestion! This change makes sense. I've created a merge request for it.

🇺🇸United States mark_fullmer Tucson

mark_fullmer changed the visibility of the branch 3504398-add-convert-to to hidden.

🇺🇸United States mark_fullmer Tucson

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

🇺🇸United States mark_fullmer Tucson

I think the custom code suggested in this issue is the right way to go, short of a new UI setting that would allow sites to enable Linkit for the menu link UI and to configure which Linkit profile should be used for that. My hope is that the forthcoming core solution in Drastically improve Drupal's default linking experience in text fields Needs work will support menu links, and that there will be no need for this to be provided in contributed code. So, for now, I think this should not be added to Linkit itself.

We are instead thinking about creating a separate linkit profile without the media matcher so we can still move forward with this.

That sounds like a reasonable approach. I don't think it's likely that there would be a need for a media matcher to be used in Drupal menus, so I'm disinclined to complexify the code by somehow modifying the rendering of menu links (beyond just the autocomplete selection provided by Linkit).

🇺🇸United States mark_fullmer Tucson

Thanks for reporting this! Can you confirm that the staged merge request resolves the issue? Thanks!

🇺🇸United States mark_fullmer Tucson

Thanks for the cleanup work and the additional testing, richard.thomas. I've reviewed this today and the code changes look good to me. I'll set this to RTBC and, as a maintainer, plan to merge this soon unless anyone else sees problems with this approach.

🇺🇸United States mark_fullmer Tucson

I've converted the patch to a merge request so we can see what this does in automated test coverage. On the surface, the changes involved here look like they could have unintentional side effects.

🇺🇸United States mark_fullmer Tucson

Okay, the issue with the tests in fact demonstrates the scope change introduced by adding filter! In other words, without explicitly enabling the Linkit text format filter, it is NOT loading the JS, which is what this change is designed to accomplish. I updated the tests.

🇺🇸United States mark_fullmer Tucson

I can confirm the behavioral difference between the two links reported above. The proposed resolution makes sense to me and does not seem to carry a risk of unexpected consequences.

🇺🇸United States mark_fullmer Tucson

I've functionally confirmed that this is no longer an issue on Linkit for Link Field. Closing!

🇺🇸United States mark_fullmer Tucson

Thanks for the suggestion! I've added a merge request that stages the proposed change. Ready for community review.

🇺🇸United States mark_fullmer Tucson

Thanks for this report! Does the small modification to the CSS in the staged merge request resolve the issue for you?

🇺🇸United States mark_fullmer Tucson

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

🇺🇸United States mark_fullmer Tucson

Thanks for the report! The stage merge request adds a targetable unique id attribute to each autocomplete suggestion description and an aria-describedby attribute to the title. Representative output shown below:

<div class="linkit-result-line-wrapper published ui-menu-item-wrapper" id="ui-id-8" tabindex="-1">
  <span class="linkit-result-line--title" aria-describedby="title--deb05804-034b-4f7e-a94f-70f4a7157254">test</span>
  <span class="linkit-result-line--description" id="title--deb05804-034b-4f7e-a94f-70f4a7157254">Flex Page #1 | Thu, 26 Jun 2025 - 10:08 by admin</span>
</div>
🇺🇸United States mark_fullmer Tucson

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

Production build 0.71.5 2024