Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding a decorator for links breaks unlink #297

Closed
kbergha opened this issue Sep 30, 2024 · 2 comments
Closed

Adding a decorator for links breaks unlink #297

kbergha opened this issue Sep 30, 2024 · 2 comments
Labels

Comments

@kbergha
Copy link

kbergha commented Sep 30, 2024

Description

Adding a decorator for links, the unlink feature breaks / causes console errors.

Steps to reproduce

  1. Configure CKE with the following config options:
    unlink-cke-config

Config options / Decorator code:

return {
  link: {
    decorators: {
      isPhoneShortNumber: {
        mode: 'automatic',
        callback: url => url.match(/[+47]?\d{5}/g),
          attributes: {
              'class': 'ET-phone-home'
          }
      },
    },
  },  
  list: {
    properties: {
      styles: false,
    },
  },
}
  1. Add a link, in this case to 12345, which matches the regex.
    unlink-decorator

  2. See that ET-phone-home is added to the anchor element.
    unlink-decorator-html

  3. Try to unlink
    unlink-before-clicking

  4. Observe console error, "This link has no URL" and that "This is a link" still has link styling.
    unlink-console-error

Console error:

Uncaught CKEditorError: Cannot read properties of null (reading 'match')
Read more: https://ckeditor.com/docs/ckeditor5/latest/support/error-codes.html#error-Cannot read properties of null (reading 'match')
    at Object.callback (240-bork-bork-bork:1254:30)
    at Zo.<anonymous> (automaticdecorators.js:73:30)
    at Zo.fire (ckeditor5-dll.js?v=1727165961:2:628930)
    at Zo._testAndFire (ckeditor5-dll.js?v=1727165961:2:284745)
    at Zo._convertAttribute (ckeditor5-dll.js?v=1727165961:2:283201)
    at Zo.convertChanges (ckeditor5-dll.js?v=1727165961:2:281329)
    at ckeditor5-dll.js?v=1727165961:2:319634
    at fo.change (ckeditor5-dll.js?v=1727165961:2:250056)
    at Cr.<anonymous> (ckeditor5-dll.js?v=1727165961:2:319598)
    at Cr.fire (ckeditor5-dll.js?v=1727165961:2:628930)
  1. Clicking around creates more console errors. I've clicked the unordered list in this example, but the styling / HTML does not seem to be applied.
    unlink-multiple-console-errors

  2. Saving the entry, the unordered list HTML/styling from the previous step is applied.
    unlink-list

A note to step 2

It does not seem to matter if what you're linking to matches the decorator regex or not.
The error happens when linking and unlinking to https://example.com as well as long as the decorator is present.

Expected behaviour

This is without the decorator.

  1. Click unlink.
    unlink-no-decorator-before-unlink

  2. Only the previous link text is present. No "This link has no URL", and no link styling / console errors.
    unlink-no-decorator-after-unlink

Additional info

  • Craft version: 5.4.6
  • PHP version: 8.3.8
  • Database driver & version: MySQL 8.0.34
  • Plugins & versions:
    CKEditor 4.2.0
    Amazon S3 2.2.1
    Amazon SES 3.1.0
    Blitz 5.7.1
    Blitz CloudFront Purger 5.1.0
    Vite 5.0.1

HTML Purifier config:

{
  "AutoFormat.Linkify": true,
  "AutoFormat.RemoveEmpty": true,
  "AutoFormat.RemoveEmpty.RemoveNbsp": true,
  "HTML.Allowed": "a[href|class],p,ul,ol,li,h2,h3,h4,h5"
}
@kbergha kbergha added the bug label Sep 30, 2024
@i-just
Copy link
Contributor

i-just commented Oct 1, 2024

Hi, thanks for getting in touch!

This error comes from CKEditor itself. If you adjust the callback to account for the fact that the URL might be null, it should start working as expected.
For example: callback: url => url?.match(/[+47]?\d{5}/g), or you can use this syntax from the docs: callback: url => /[+47]?\d{5}/g.test(url),.

I hope this helps!

I’ll close this now, but feel free to reach out if you run into any further issues.

@i-just i-just closed this as completed Oct 1, 2024
@kbergha
Copy link
Author

kbergha commented Oct 1, 2024

Thank you for the fast troubleshooting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants