Edit_Advanced_Link+Editor_file: conflicting field attributes

Created on 23 January 2024, 11 months ago

Problem/Motivation

If the modules "editor_advanced_link" and "editor_file" are both activated, the title and advanced dialog fields fail to persist after the form is submitted.

Steps to reproduce

To replicate this issue, enable both the "editor_advanced_link" and "editor_file" modules. Click on the editor file icon in CKEditor5, fill out the title, ARIA label, class, etc. Then, upon saving, you'll observe that none of those fields are applied.

Proposed resolution

Currently unsure, but it would be advisable to disable those fields while we perform a more in-depth investigation. The file URL and file status functions as intended when a file is added.

Remaining tasks

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

Active

Version

2.2

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States matthew.page

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

Comments & Activities

  • Issue created by @matthew.page
  • Status changed to Needs review 11 months ago
  • πŸ‡ΊπŸ‡ΈUnited States matthew.page

    Here's the patch. This unsets the advanced attribute fields. That way when you use editor_file module, these fields will be hidden since they do not work.

  • πŸ‡ΊπŸ‡ΈUnited States matthew.page
  • Status changed to Needs work 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States daniel korte Brooklyn, NY

    This appears to be an issue with the editor_advanced_link module not supporting the editor_file module so changing this to a Feature request. The editor_advanced_link CKeditor 5 plugin only has support for the ckeditor5-link plugin. JavaScript needs to be added to this module to respond/listen for the editor_file ckeditor5 plugin execution.

  • πŸ‡³πŸ‡±Netherlands joshahubbers

    I have been fighting with this one yesterday. I am not very good at these things but took a shot at it.

    Issues in my case:
    * editor_file is loaded after editor_advanced_link. Thus some of the JS in the init() function will not be applied to the editor_file functions.
    * editor_file displays a custom dialog, and returns a different set of attributes. not the standard "link" object, but an array with the attributes of the a-element.
    * setting the "target" was challanging. Finally found that the attribute "link0" with a value of "true" wil add "target=_blaink" to your link.

    I don't know how to create a patch for the ck-editor plugin in this module. So I ended up moving the module to my "custom" folder. Not really the ideal way to go.

    Open issues:
    * link editing doesn't load the advanced-properties.

    Modification to the file editoradvancedlinkediting.js:
    Added this function after the "init"-function. The handling is mostly copied from the editor_file module.

    afterInit() {
        const { editor } = this;
        // start
        const drupalFileCommand = editor.commands.get('insertFileToEditor');
        let drupalFileCommandExecuting = false;
        drupalFileCommand.on(
          'execute',
          (evt, args) => {
            if (drupalFileCommandExecuting) {
              drupalFileCommandExecuting = false;
              return;
            }
    
            evt.stop();
            // Prevent infinite recursion by keeping records of when link command is
            // being executed by this function.
            drupalFileCommandExecuting = true;
            const attributes = args[0];
            const { model } = this.editor;
            const { selection } = model.document;
    
            model.change((writer) => {
              const attrMaps = {
                linkHref: 'href',
                fileDataEntityType: 'data-entity-type',
                fileDataEntityUuid: 'data-entity-uuid',
                target: 'target',
              };
              this.enabledModelNames.forEach((modelName) => {
                attrMaps[modelName] = additionalFormElements[modelName].viewAttribute;
              });
    
              // When selection is inside text with `linkHref` attribute.
              if (selection.hasAttribute('linkHref')) {
                // Editing existing file link.
                const position = selection.getFirstPosition();
    
                Object.entries(attrMaps).forEach(([attr, key]) => {
                  const linkRange = findAttributeRange(position, attr, selection.getAttribute(attr), model);
                  writer.setAttribute(attr, attributes[key], linkRange);
                });
              } else {
                // Check if text is selected.
                let selectedText;
                const range = selection.getFirstRange();
    
                // eslint-disable-next-line no-restricted-syntax
                for (const item of range.getItems()) {
                  selectedText = item.data;
                }
    
                const linkAttrs = {};
    
                Object.entries(attrMaps).forEach(([attr, key]) => {
                  linkAttrs[attr] = attributes[key];
                });
                linkAttrs.link0 = linkAttrs.target === '_blank';
    
                const text = !selectedText ? attributes.filename : selectedText;
                const linkedText = writer.createText(text, linkAttrs);
    
                model.insertContent(linkedText);
    
                if (linkedText.parent) {
                  writer.setSelection(linkedText, 'on');
                }
              }
            });
          },
          { priority: 'high' },
        );
      }

    Than dont forget to run yarn to compile the js.

  • πŸ‡³πŸ‡±Netherlands joshahubbers

    New day, new try.

    Updated the function above, and added one to enable the editing dialog. Seems to be working ok now.

    Editoradvancedlinkui:

      afterInit() {
        const { editor } = this;
    
        const drupalFileEditButton = editor.plugins.get('DrupalEditorFileUploadActionUi').fileEditButton;
        const linkUI = editor.plugins.get('LinkUI');
        let drupalFileEditExecuting = false;
        drupalFileEditButton.on(
          'execute',
          (evt) => {
            if (drupalFileEditExecuting) {
              drupalFileEditExecuting = false;
              return;
            }
    
            evt.stop();
            // Prevent infinite recursion by keeping records of when link command is
            // being executed by this function.
            drupalFileEditExecuting = true;
            const selectedLinkElement = linkUI._getSelectedLinkElement();
    
            if (!selectedLinkElement) {
              return;
            }
    
            // Add existingValues of DrupalUpload.
            const existingValues = selectedLinkElement.hasAttribute('data-entity-uuid') ? {
              'data-entity-uuid': selectedLinkElement.getAttribute('data-entity-uuid'),
              'data-entity-type': selectedLinkElement.getAttribute('data-entity-type'),
            } : {};
    
            // Add existingValues of advanced link.
            const { enabledModelNames } = editor.plugins.get(
              'EditorAdvancedLinkEditing',
            );
            enabledModelNames.reverse().forEach((modelName) => {
              let attributeName = additionalFormElements[modelName].viewAttribute;
              existingValues[attributeName] = selectedLinkElement.getAttribute(attributeName);
            });
            existingValues.target = selectedLinkElement.getAttribute('target');
    
            const options = editor.config.get('drupalFileUpload');
            const DrupalInsertFile = editor.plugins.get('DrupalInsertFile');
    
            drupalFileEditExecuting = false;
            DrupalInsertFile.openDialog(
              Drupal.url('editor_file/dialog/file/' + options.format),
              existingValues,
              ({attributes}) => {
                editor.execute('insertFileToEditor', attributes);
              }
            );
          },
          { priority: 'high' },
        );
      }

    editoradvancedlinkediting.js

    afterInit() {
        const { editor } = this;
        const drupalFileCommand = editor.commands.get('insertFileToEditor');
        let drupalFileCommandExecuting = false;
        drupalFileCommand.on(
          'execute',
          (evt, args) => {
            if (drupalFileCommandExecuting) {
              drupalFileCommandExecuting = false;
              return;
            }
            evt.stop();
            // Prevent infinite recursion by keeping records of when link command is
            // being executed by this function.
            drupalFileCommandExecuting = true;
            const attributes = args[0];
            const { model } = this.editor;
            const { selection } = model.document;
    
            model.change((writer) => {
              const attrMaps = {
                linkHref: 'href',
                fileDataEntityType: 'data-entity-type',
                fileDataEntityUuid: 'data-entity-uuid',
                target: 'target',
              };
              this.enabledModelNames.forEach((modelName) => {
                attrMaps[modelName] = additionalFormElements[modelName].viewAttribute;
              });
    
              // When selection is inside text with `linkHref` attribute.
              if (selection.hasAttribute('linkHref')) {
                // Editing existing file link.
                const position = selection.getFirstPosition();
                Object.entries(attrMaps).forEach(([attr, key]) => {
                  const linkRange = findAttributeRange(position, attr, selection.getAttribute(attr), model);
                  attr = attr === 'target' ? 'link0' : attr
    
                  if (attributes[key]) {
                    writer.setAttribute(attr, attributes[key], linkRange);
                  }
                  else {
                    writer.removeAttribute(attr, linkRange);
                  }
                });
              } else {
                // Check if text is selected.
                let selectedText;
                const range = selection.getFirstRange();
    
                // eslint-disable-next-line no-restricted-syntax
                for (const item of range.getItems()) {
                  selectedText = item.data;
                }
    
                const linkAttrs = {};
    
                Object.entries(attrMaps).forEach(([attr, key]) => {
                  linkAttrs[attr] = attributes[key];
                });
                linkAttrs.link0 = linkAttrs.target === '_blank';
    
                const text = !selectedText ? attributes.filename : selectedText;
                const linkedText = writer.createText(text, linkAttrs);
    
                model.insertContent(linkedText);
    
                if (linkedText.parent) {
                  writer.setSelection(linkedText, 'on');
                }
              }
              drupalFileCommandExecuting = false;
            });
          },
          { priority: 'high' },
        );
      }

    Changedfiles (including build) in attached zipfile.

  • πŸ‡³πŸ‡±Netherlands adebruin

    This is a patch file based on the changes of #6. Tested the solution and it works.

  • Status changed to RTBC 9 months ago
  • πŸ‡³πŸ‡±Netherlands timohuisman Leiden, Netherlands

    Can confirm that #7 works.

  • πŸ‡³πŸ‡ΏNew Zealand Gold 20 minutes in the future

    What version of Editor File upload is being used here? And are any patches to that in place?

    I have Editor File upload at 2.0.0-rc1 with no extra patches. The patch here applied to EAL 2.2.4 fine.

    Were there any other steps that needed to be taken? e.g. Did the Filter processing order need to be reordered?

    I want to ensure that it is not me before I pulling back the RTBC.

  • Status changed to Needs work 9 months ago
  • πŸ‡³πŸ‡ΏNew Zealand Gold 20 minutes in the future

    Additional: Clicking "Open in new window/tab" works when the link is first created. If you just edit and save the modal without touching anything it looks like the is applied then as well.

    I've tested this in multiple environments now too. Pulling it back to Need Work.

    What I'm seeing on the second edit is that the target is applied to all of the surrounding paragraph.

    e.g.

    <p>
        <a target="_blank">Lorem ipsum dolor sit amet, consectetur </a><a href="https://media-staging-2.yvw.com.au/inline-files/dummy_19.pdf" target="_blank" title="Title field" id="an-id" rel="noopener" data-entity-uuid="e1ab4332-7d39-466e-988f-72d977484cdd" data-entity-type="file">sagittis</a><a target="_blank">, nulla eget metus odio.</a>
    </p>
    

    I've just looked over the patch. I'm wondering if this may be related to the selected text range? In the instance where the button is clicked from the popup in the existing link there is no selected range.

    I also note the following:

    • Double clicking the link in the editor and clicking the button in the toolbar opens the modal with the file populated but none of the other attributes.
    • If "Open in new window/tab" is unticked and rel is empty rel is added anyway with a value of undefined.
    • CSS doesn't appear to save reliably.
    • Multiple links is flaky. Having 2 links without "Open in new window/tab" and then editing the second one to "Open in new window/tab" crashes out the JS. Behaviour after that was unstable.
  • πŸ‡³πŸ‡±Netherlands adebruin

    This patch will probably not solve the issue that Gold has come forth with. This patch will solve an issue where if the 'editor_file' is not active in a certain CKEditor configuration the afterinit methods will do an early return and stop preventing CKEeditor from loading.

  • πŸ‡³πŸ‡±Netherlands adebruin
  • πŸ‡¬πŸ‡§United Kingdom retrodans

    Just to add a note, on a site we are using we have:

    - Editor Advanced link 2.2.4
    - Editor File upload 2.0.0-rc1

    The issue we had is that none of the values in advanced were getting saved.

    Patch #7 solved this issue, whilst patch #11 did not.

  • πŸ‡³πŸ‡ΏNew Zealand Gold 20 minutes in the future

    @retrodans, my early testing of the patch at #7 left me thinking it was okay as well. A deeper dive showed the issues I outlined in comment #10. Can you confirm that you do or don't see the issues from that comment?

  • Using either patch (7 or 11/11_0) I'm still seeing the issues from comment #10. If I get everything right on the first entry, it's great. If I try to edit that file link with any new advanced elements, it wraps the entire paragraph. If I edit the file link and simply leave all the advanced fields empty, it does correctly remove them without issue.

  • The patch is no longer functional in the latest versions of the module

Production build 0.71.5 2024