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

Stripping out HTML attributes regardless of settings #217

Closed
jacksutherland opened this issue May 5, 2024 · 5 comments
Closed

Stripping out HTML attributes regardless of settings #217

jacksutherland opened this issue May 5, 2024 · 5 comments
Labels

Comments

@jacksutherland
Copy link

jacksutherland commented May 5, 2024

Description

HTML attributes added to the source HTML of a CKEditor field are being stripped out, regardless of config settings in the CP or in the HTML Purifier config file. Attributes such as target="_blank" for external links is critical functionality, yet still being removed. Class names and other attributes are removed from all elements as well. Unchecking "Purify HTML" in the field's advanced settings has no effect on this. I also tried adding every relevant setting I could find in the links the plugin provides at ckeditor.com and htmlpurifier.org, and they had no effect, even after clearing caches. For example, I tried re-enabling "Purify HTML" and adding settings (i.e. HTML.AllowedAttributes) to the field's selected HTMLPurifier's json file, and it made no difference. Neither did adding settings to the CKEditor Configs in the CP.

The docs at these links aren't specific to Craft, so its unclear what the preferred method is to handle this, or if all of these settings are even implemented. And I found this question asked multiple times in the Discord group with no responses, leading me to believe that there aren't many users clear on how to handle this.

I believe addressing this should be high priority since it blocks critical functionality like external linking, and because some site editors that understand code, need the ability to add HTML that's more complex than what the buttons and custom styles will easily allow.

Steps to reproduce

  1. Add rich text to an instance of CKEditor including a link.
  2. Click "Source" button to edit the HTML.
  3. Add a class attribute with a value to the link or any other element.
  4. Add target="_blank" to the link.
  5. Click the "Source" button again and the class and target attribute will instantly be removed.

Additional info

  • Craft version: Pro 4.9.1
  • PHP version: 8.0.25
  • Database driver & version: MariaDB 10.10.2
  • Plugins & versions: CKEditor 3.8.3
@i-just
Copy link
Contributor

i-just commented May 8, 2024

Hi, thanks for reaching out!

Custom attributes and classes are stripped out (by default) by CKEditor itself, but you can change that via configuration.

In a nutshell, you choose which “buttons” each CKEditor configuration should support, and you can further tweak their behaviour via “Config Options”. Within the “Config Options”, you can provide a configuration that’s described at https://ckeditor.com/docs/ckeditor5.

In terms of target="_blank" for links, you need to add the configuration that allows this attribute via link decorators, like described here: https://ckeditor.com/docs/ckeditor5/latest/features/link.html. Following related issues might help clear things up: #79, #98.

When it comes to classes, CKEditor will allow you to specify them via the Styles feature. Here are some related issues regarding styles and classes: #135, #105, #91.

Finally, if you’re using the Source editing feature, it has a htmlSupport configuration: https://ckeditor.com/docs/ckeditor5/latest/features/html/general-html-support.html, which allows you to configure what should be possible to use/do/adjust when editing via the Source mode.

I hope this clears things up!

I'll close this now, but feel free to reach out if anything's unclear.

@i-just i-just closed this as completed May 8, 2024
@a-am
Copy link

a-am commented May 8, 2024

@i-just, There seems to be some confusion about the "Purify HTML" feature. When we disable it, are we supposed to be able to include any HTML code in the source?

We've encountered a situation where we turned it off, but the HTML was still being sanitized. Can you clarify how this feature works?

@i-just
Copy link
Contributor

i-just commented May 8, 2024

When we disable it, are we supposed to be able to include any HTML code in the source?

@a-am, not quite. If you disable “Purify HTML”, you’ll be allowed to include any HTML code that CKEditor is configured to allow you to. I guess the easiest way to see what’s going on would be to first disable the purifier in your dev environment, configure CKEditor to allow all the tags/classes/attributes you need, test that they’re all retained between saved, and then enable the purifier and see if any adjustments to the purifier config are needed.

@jacksutherland
Copy link
Author

Thanks @i-just ! For one of the sites I'm adding this to, I'm the developer and the only CP editor, and don't need any handrails. What's the easiest way to disable this completely, so I can click Source and add whatever I want?

The docs say to install ckeditor/ckeditor5-html-support and add the config below. But I cannot figure out WHERE to add that configuration. I've tried adding it to the Config Options in the CP settings, but it makes no difference. What am I missing?

htmlSupport: {
    allow: [
        {
            name: /.*/,
            attributes: true,
            classes: true,
            styles: true
        }
    ]
}

@jacksutherland
Copy link
Author

The docs say to install ckeditor/ckeditor5-html-support and add the config below. But I cannot figure out WHERE to add that configuration. I've tried adding it to the Config Options in the CP settings, but it makes no difference. What am I missing?

I found my problem @i-just. The docs say to add:

htmlSupport: {
    allow: [
        {
            name: /.*/,
            attributes: true,
            classes: true,
            styles: true
        }
    ]
}

But name doesn't seem to be accepted. Including it makes everything in the entire allow array not work. Adding this instead works...

htmlSupport: {
    allow: [
        {
            attributes: true,
            classes: true,
            styles: true
        }
    ]
}

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

3 participants