Editor_file+Linkit: removing links broken

Created on 18 January 2024, 10 months ago

Problem/Motivation

When both modules are added removing a link is broken.

The following code remains after removing a link from linkit.

<a data-entity-uuid="8d3939ae-89a7-4d5b-acf4-caf039a5ed8e" data-entity-type="node">link to home</a>

Steps to reproduce

Using ddev.

mkdir drupal-editor-file
cd drupal-editor-file
ddev config --project-type=drupal10 --docroot=web --create-webroot
ddev start
ddev composer create drupal/recommended-project
ddev composer require drush/drush
ddev drush site:install --account-name=admin --account-pass=admin -y
ddev drush uli
ddev launch
ddev composer config minimum-stability dev
ddev composer require drupal/linkit
ddev composer require drupal/editor_file:^2.0
ddev drush en editor_file,linkit

Then in /admin/config/content/formats/manage/basic_html?destination=/admin/config/content/formats enable linkit default profile. ALso enable the "file upload" button. In "File" set the upload directory, allowed file extentions.
Create a new page (homepage)
Create a new page, add the following: a link to homepage, then save.
Edit the saved page, add a uploaded file (pdf or whatever)
Save
Edit the page en click the link you added to the home page, click the remove buttom. You should now still have a link there, but without a href. See the following code: <a data-entity-uuid="8d3939ae-89a7-4d5b-acf4-caf039a5ed8e" data-entity-type="node">link to home</a>

So for some reason they conflict, when editor file is not added the links work without a problem.

Proposed resolution

Not sure, but for some reason the removal of the attributes is stopped by editro_file, perhaps the hook how the attributes are managed are wrong, or it assumes the uuid/type attributes are the ones this module added instead of ones by linkit?

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Active

Version

2.0

Component

Code

Created by

🇳🇱Netherlands bbrala Netherlands

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @bbrala
  • 🇳🇱Netherlands bbrala Netherlands
  • 🇳🇱Netherlands bbrala Netherlands

    The problem is as follows:

    When an element has data-entity-type and/or data-entity-uuid, this module load that data into the ckeditor model. This means if another module adds those properties it will think its a file upload. This is wrong. The fix here is to check if the type is a file. Then load the model. There might be a world where it is better to add a new property like data-managed-by=editor_file, but im not sure of the side effects of this, and this might conflict on other levels.

  • Status changed to Needs review 10 months ago
  • 🇳🇱Netherlands bbrala Netherlands
  • 🇳🇱Netherlands bbrala Netherlands

    (adding patch for internal policy reasons)

  • 🇺🇸United States mkindred

    Although I've never used editor_file previously, I was able to confirm the bug in a fresh install of D10.2.2, editor_file 2.0.0-rc1, and linkit 6.1.2.

    Patch #6 fixed the issue for me.

  • 🇳🇱Netherlands bbrala Netherlands

    @mkindred if you tested, cant you RTBC then?

  • Status changed to RTBC 9 months ago
  • 🇺🇸United States matthew.page

    I'm facing a similar problem with ckeditor5. I've created a function intended to eliminate the data-entity-type and data-entity-uuid upon clicking. However, it currently only detaches the text, leaving the data attributes unchanged. I'm in the process of developing a converter to address this issue by removing the attributes while preserving the text, if that clarifies my approach.

    // Method to handle unlinking a file
      unlinkFileCommand() {
        const { editor } = this;
        const { model } = this.editor;
        const { selection } = model.document;
      
        let isUnlinkingInProgress = false;
      
        // Execute the 'unlink' command to remove the link from the selected text.
        editor.execute('unlink');
      
        // This single block wraps all changes that should be in a single undo step.
        model.change(() => {
          // Now, in this single "undo block" let the unlink command flow naturally.
          isUnlinkingInProgress = true;
      
          // Do the unlinking within a single undo step.
          editor.execute('unlink');
      
          // Let's make sure the next unlinking will also be handled.
          isUnlinkingInProgress = false;
      
          // The actual integration that removes the extra attribute.
          model.change((writer) => {
            // Get ranges to unlink.
            let ranges;
      
            const attributeNames = ['data-entity-type', 'data-entity-uuid'];
      
            if (selection.isCollapsed) {
              ranges = [
                findAttributeRange(
                  selection.getFirstPosition(),
                  attributeNames,
                  selection.getAttribute(attributeNames[0]), // Assuming both attributes have the same value.
                  model,
                ),
              ];
            } else {
              ranges = model.schema.getValidRanges(
                selection.getRanges(),
                attributeNames,
              );
            }
    
            // Remove the extra attribute from specified ranges.
            for (const range of ranges) {
               attributeNames.forEach((attr) => {
                 writer.removeAttribute(attr, range);
               });
             }
    
          });
        });
      }
    
  • 🇮🇹Italy kopeboy Milan

    I confirm that after applying patch #6 removing links from CKEditor is not broken.

    • e6347210 committed on 2.x
      Issue #3415505: fix removing links broken when linkit is enabled
      
  • 🇫🇷France duaelfr Montpellier, France

    Good job fixing that!

Production build 0.71.5 2024