Problem

CKEditor 4.1 introduced Advanced Content Filter (ACF), which essentially defines what the editor is allowed to output.
It has two active modes: automatic and custom.

Content is filtered when the editor loads it, potentially destroying existing content which wasn't created using the same editor configuration!

Automatic mode

  • CKEDITOR.config.allowedContent is null.
  • Plugins supply a list of tags and attributes they create/allow in the content.
  • CKEDITOR.config.extraAllowedContent can extend the automatic rules provided by plugins.
  • Content is stripped of any tags/attributes not defined by the rules defined provided by the plugins.
  • Disabling a plugin or a button will remove content allowed only by that plugin. (Ex: Remove the Image button and any <img /> tags will also be removed.

Custom mode

  • CKEDITOR.config.allowedContent is a string with tag/attribute names or an object based on CKEDITOR.filter.allowedContentRules.
  • Plugins supply a list of tags and attributes they require to work and which ones they may sometimes output.
  • Plugins/buttons requiring tags which aren't allowed are automatically disabled or their features are limited. (Ex if <img> tags aren't allowed, those tags will be removed and the Image plugin will be completely disabled.
  • The Styles and Format dropdown lists only contain items which have been defined in the content rules.

Disabled

  • CKEDITOR.config.allowedContent is true.
  • No tags/attributes (except for those considered dangerous, like <script>, are removed.
  • Enabled plugins/buttons stay enabled.

Workaround

It's possible for modules to quickly disable ACF while we work on a more permanent solution here:

/**
 * Implements hook_wysiwyg_editor_settings_alter().
 */
function MYMODULE_wysiwyg_editor_settings_alter(&$settings, $context) {
  if ($context['profile']->editor == 'ckeditor') {
    $settings['allowedContent'] = TRUE;
  }
}

Suggestion

In the CKEditor implementation's settings form callback function, create a couple of new form elements to control the behavior of ACF.
The first element would be a select box with the options "Automatic", "Custom" and "Disabled", where we make "Disabled"" the default. The reasoning being that this is - as it name implies - an advanced feature, and we don't currently have a way to let plugins (native or cross-editor) specify which tags/attributes/styles they require and which ones they are able to create.

The second element would be a textarea to hold a custom rule set defined either in the string format or the object format (JSON) representations of ACF rules, see the ACF documentation for details. There's a possibility we may need an extra element to toggle between the string format and the object format if it proves to be difficult to distinguish between the two.

When the profile form is saved we check if the contents of the textarea is a valid JSON object and set it to the allowedContent setting if that is the case. Otherwise we set the string value as is.
If the select box has been set to "Disabled", we override the allowedContent value with the boolean TRUE. If it's set to "Automatic", we move the value of allowedContent to extraAllowedContent and simply unset allowedContent.

Original post by mibfire:

ACF

The new great(really?) Ckeditor 4.x automatical remove all attributes what arent defined for editor so for example, id or class attributes will be removed and it is so annonying. To fix this we should create all atributes what we would like to use in editor.

This way it can be fixed:

function MYMODULE_wysiwyg_editor_settings_alter(&$settings, &$context) {
  if($context['profile']->editor == 'ckeditor') {
    $settings['allowedContent'] = array('p','address','pre','h2','h3','h4','h5','h6','div','script','iframe','*[*]');
  }
}

*[*] means it allows all attributes for the given html tags, but this way we lose the buttons that are set in Wysiwyg admin page. So if these arent defined and someone try to edit a content all attributes will be lost.

It should be fixed asap or we have to write our own code for evry text format. THX

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mibfire’s picture

To disable ACF: $settings['allowedContent'] = true;

TwoD’s picture

There's a new 'settings form callback' property in the editor definition which can be used to customize the profile form from within the module implementing the editor integration (Wysiwyg itself in this case). It works just like hook_form_alter() and is already in use to customize some of the CKEditor-specific things on the profile form.

It should be fairly easy to extend the profile form when CKEditor 4 is detected so this setting can be cuztomized (both allowing the "TRUE" value and a list of tags/attributes), if anyone feels up to it.

marktheshark’s picture

Is there no way for the user to specify CKEDITOR.config.allowedContent = true via the UI, like the custom flags the ckeditor allows the admin to add?

TwoD’s picture

@marktheshark Currently, no. That's what I hope will come out of this issue.

marktheshark’s picture

For the record, I followed approach #1. The module code in the original post did not prevent classes from getting stripped from my p tags, for some reason.

lsolesen’s picture

AvalancheOfLlamas’s picture

I have the same issue marktheshark does. CKEditor still doesn't like the classes in my <p> tags.

marktheshark’s picture

I just created a module with this code (in case it helps):

function ckeditor_acf_wysiwyg_editor_settings_alter(&$settings, &$context) {
  if($context['profile']->editor == 'ckeditor') {
    $settings['allowedContent'] = true;
  }
}
lotyrin’s picture

Priority: Critical » Major

Critical feature request?

rocketeerbkw’s picture

Priority: Major » Critical
Status: Active » Needs review
FileSize
4.29 KB

Marking critical since it was stated as release blocker in #1953106: Roll 7.x-2.3 release of Wysiwyg.

Here's a patch that follows the instructions in summary. I've tested and it works to my understanding of how ACF works. I'd like feedback on the help text that should go on the new form elements.

I added two fields to the Cleanup and output section. The allowed content field is only shown when Automatic or Custom are selected.

hass’s picture

Status: Needs review » Needs work
+++ b/editors/ckeditor.incundefined
@@ -138,6 +142,24 @@ function wysiwyg_ckeditor_settings_form(&$form, &$form_state) {
+    $form['output']['acf_mode']['#title'] = t('Advanced Content Filter');

ucfirst only.

+++ b/editors/ckeditor.incundefined
@@ -138,6 +142,24 @@ function wysiwyg_ckeditor_settings_form(&$form, &$form_state) {
+    $form['output']['acf_mode']['#options'] = array(CKEDITOR_ACF_AUTOMATIC => 'Automatic', CKEDITOR_ACF_CUSTOM => 'Custom', CKEDITOR_ACF_DISABLED => 'Disabled');

Strings are not translatable. Not Drupal code style.

+++ b/editors/ckeditor.incundefined
@@ -138,6 +142,24 @@ function wysiwyg_ckeditor_settings_form(&$form, &$form_state) {
+    $form['output']['acf_allowed_content']['#title'] = t('Allowed Content');

"Content" lowercase

+++ b/editors/ckeditor.incundefined
@@ -162,6 +184,15 @@ function wysiwyg_ckeditor_settings_form(&$form, &$form_state) {
+    form_error($element, t('Allowed Content is not valid JSON'));

Write "Content" lowercase. End sentences with periods.

rocketeerbkw’s picture

Status: Needs work » Needs review
FileSize
4.32 KB

Thanks! I've made those changes. Still needs form field descriptions.

rocketeerbkw’s picture

#1968318: Support for TinyMCE 4.x and #2018439: Move editor-specific options out of the default profile UI. make some big changes, will be easier to rewrite my patch after those go in.

TwoD’s picture

Awesome @rocketeerbkw! I haven't had time to actually test the patch yet, but it looks pretty solid just by what I can see with Dreditor.

I wonder if we could also provide a way for [cross-editor] plugins like Media to supply an abstracted set of rules for extending the sets here, since they can't directly use the editor API like native plugins. That would fix the major issue of inserted media not working. However, that's a fairly complex issue, and it'd have to be coordinated with the TinyMCE 4 integration.

I'll dig into that a bit more and get back here after rolling in the other patches you mentioned.

saltednut’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

I attempted to apply this patch but it did not apply to wysiwyg 7.x-2.x HEAD

error: patch failed: editors/ckeditor.inc:138
error: editors/ckeditor.inc: patch does not apply

*[7.x-2.x][~/Sites/drupal7/sites/all/modules/wysiwyg]$ git apply wysiwyg-ckeditor-acf-1956778-12.patch -v
Checking patch editors/ckeditor.inc...
error: while searching for:
  $form['output']['paste_auto_cleanup_on_paste']['#title'] = t('Force paste as plain text');
  $form['output']['paste_auto_cleanup_on_paste']['#description'] = t('If enabled, all pasting operations insert plain text into the editor, loosing any formatting information possibly available in the source text. Note: Paste from Word is not affected by this setting.');

  if (version_compare($form_state['wysiwyg']['editor']['installed version'], '3.6.0', '>=')) {
    $form['appearance']['default_toolbar_grouping'] = array(
      '#type' => 'checkbox',

error: patch failed: editors/ckeditor.inc:138
error: editors/ckeditor.inc: patch does not apply
rocketeerbkw’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
5.63 KB

I've rerolled against HEAD and added field descriptions. This should be ready to go.

dooleysarahe’s picture

The patch in #16 worked for me (had to go in and re-save the admin settings first).

guillaumev’s picture

Status: Needs review » Reviewed & tested by the community

Just tested this patch on a fresh Drupal install with:

  • Drupal 7.23
  • Media 7.x-2.0-alpha2
  • WYSIWYG 7.x-2.x-dev

and everything worked for me: I was able to easily add media files in the editor. Would be great to see this committed and a new release of wysiwyg created.

jrglasgow’s picture

I have tested this patch as well and all appears to work as designed.

rodricels’s picture

Quick & dirty D6 patch

RunePhilosof’s picture

Status: Reviewed & tested by the community » Needs work

#17 mentions having to resave the admin settings.
Can this be avoided or documented in the changelog?

TwoD’s picture

One way to avoid it would be to use an update to inject the setting into all active CKEditor profiles and have ACF disabled by default. This should of course be noted with a message on the update page.

eric.chenchao’s picture

FileSize
15.88 KB

The patch in #16 works as a charm.

In order to add class in div, I have set up AFC as "automatic" and add new rules in the allowed content

Ckeditor AFC settings

It is worth to note that the documentation can be found in the page - ckeditor/samples/datafiltering.html - in either Ckeditor standard or full version.

pjbarry21’s picture

#16 worked for me in OpenAtrium's latest dev (after I updated WYSIWIG to the latest dev).

sylus’s picture

Status: Needs work » Reviewed & tested by the community

Works great for me :) Hope it is okay to RTBC.

rocketeerbkw’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
5.59 KB

I made a small change to settings callback which should fix having to resave the profile settings. It does not require running update.php.

guillaumev’s picture

Status: Needs review » Reviewed & tested by the community

I just tested this, and it works like a charm. I configured the wysiwyg module with the media button before applying the patch, tried to add a media through ckeditor, it didn't work. Then, I applied the patch and tried to add a media through ckeditor, and it worked like a charm. RTBC ?

kunago’s picture

I also confirm this patch is working as my div classes are no longer removed.

k.skarlatos’s picture

works for me as well, +1 for rtbc

dmsmidt’s picture

Yes works like a charm, even the title / alt attributes work :-) for media.
Adding divs with a class also works with the settings in #23.

+1 RTBC

dddbbb’s picture

#26 works for me too.

Tested with both Insert & oEmbed modules - everything now works as expected. Also tested with with CKEditor 4.2.2 and everything appears to be happy.

Thanks!

dddbbb’s picture

#26 works for me too.

Tested with both Insert & oEmbed modules - everything now works as expected. Also tested with with CKEditor 4.2.2 and everything appears to be happy.

Thanks!

dddbbb’s picture

#26 works for me too.

Tested with both Insert & oEmbed modules - everything now works as expected. Also tested with with CKEditor 4.2.2 and everything appears to be happy.

Thanks!

dddbbb’s picture

Apologies for the comment spam. D.O had some troubles posting my original comment.

drupov’s picture

Yes, #26 works for me too! My setup: CKEditor 4.2.2, wysiwyg 7.x-2.2+23-dev, media 7.x-2.0-alpha2.

+1 RTBC!

Thanks!

TwoD’s picture

Status: Reviewed & tested by the community » Needs work

Great! Thanks for helping with this!

There's one thing we'll have to change though. The constants should not start with 'CKEDITOR_' since we don't really "own" that namespace. It's not ideal and unsupported, but it's possible to have ckeditor.module running alongside wsiwyg.module and they prefix their globals like that.
To play nice, we should prefix this with "WYSIWYG_CKEDITOR_", just like for our function names.

guillaumev’s picture

Status: Needs work » Needs review
FileSize
5.68 KB

New patch taking into account comments from TwoD. RTBC ?

sylus’s picture

Status: Needs review » Reviewed & tested by the community
elstudio’s picture

Patch from #37 works for me, thanks!

WYSIYWG 7.x-2.2+23-dev, CKEditor 4.2.2.

+1 for RBTC.

rocketeerbkw’s picture

+1 for #37 from me

kerasai’s picture

Quick visual on the patch, everything seems sane. I've got one of iterations (prob #12) running on a few sites with no known issues.

Let's get this committed.

jeffdiecks’s picture

patch from #37 worked for me with wysiwyg 7.x-2.x-dev + CKEditor 4.2.2 + Media 7.x-1.3

DamienMcKenna’s picture

I tried this out with Media v7.x-2.0-alpha3+3-dev with the patch from #2067063-124 and it worked GREAT! Full test notes: https://drupal.org/node/2067063#comment-8009717

Thanks everyone for putting this together!

deggertsen’s picture

patch from #37 worked for me as well.

deggertsen’s picture

Issue summary: View changes

Outlined the current situation and a proposed solution.

dddbbb’s picture

Did this get committed? I just updated to latest dev and found that I didn't need to apply any patches from this issue.

DamienMcKenna’s picture

@danbohea: No, none of the patches from this issue have been committed.

dddbbb’s picture

@DamienMcKenna: Thanks for clearing that up. I need to figure out what on earth's going on with my set up then :)

arosboro’s picture

I can confirm this patch works as intended with CKEditor 4.2.3.a8bf556, and the latest dev releases of Media (7.x-2.0-alpha3+17-dev) and WYSIWYG (7.x-2.2+23-dev)

green monkey’s picture

dev version appears to work with CKEditor 4.3.0.d2184ac

spgd01’s picture

Dev does not work with media module.

#37 works with CKeditor 4.3.0.d2184ac

suzanne.aldrich’s picture

I'll just add right here that the patch, and only the patch, in #37, makes this combination work:

CKEditor 4.3.0.d2184ac
Media 7.x-2.0-alpha3+31-dev
Wysiwyg 7.x-2.2+23-dev

Ship it.

alexmc’s picture

Any chance of incorporating these patches into nice released modules? I can't use CKEditor while it deletes my data

imclean’s picture

#37 is working well for me, thanks.

samwillc’s picture

I don't know where this 'advanced' option even is?!

https://dl.dropboxusercontent.com/u/63070476/Screen%20Shot%202013-12-17%...

I checked the box, saved, flushed cache, logged out/in, everything. I get no extra options.

dddbbb’s picture

+1 the sentiments in #51. I just wasted far too much time trying to get Media/WYSIWYG/CKEditor to work with latest dev versions, only to find out that this patch for CKEditor ACF was the missing piece of the puzzle. I was even already subscribed to this issue (for a different reason) yet it was still too much to troubleshoot and piece together.

+1 RBTC.

WYSIWYG 7.x-2.2+23-dev (latest at time of writing)
Media 7.x-2.0-alpha3+34-dev (latest at time of writing)
CKEditor 4.3.1.3ecd0b8 (latest download at time of writing)

deggertsen’s picture

Yet another month passes without this being committed. I think I'm using this on 5 production sites now, could we please commit it?

pdcrane’s picture

I'm using this on several sites as well. Seems to work great.

dooleysarahe’s picture

+1 RBTC. Would love to see this committed.

tjtj’s picture

How do I install the patch?

DamienMcKenna’s picture

tjtj’s picture

For those who are inexpert (like me), in the sites/all/libraries/wysiwyg directory, do:
patch -p1 < wysiwyg-ckeditor-acf-1956778-37.patch

I hope this is the right patch.

TwoD’s picture

Status: Reviewed & tested by the community » Fixed
FileSize
4.12 KB

I've committed the #37 patch with a few minor modifications, see the interdiff if you wish to apply the additional changes I made.
This fix is now in 7.x-2.x-dev and 6.x-2.x-dev.

Thanks all for reporting, patching, reviewing and testing this patch!

The -dev snapshots will be updated within 12 hours and this will be part of the next releases.

jerry’s picture

Many thanks for getting this committed, and for including D6 as well.

dddbbb’s picture

Yey! Top work.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

person101x’s picture

Will this feature allow me to blacklist certain tags/attributes with
config.disallowedContent ?

sadashiv’s picture

Hi all,

I tried the dev release of the module and can see these options in the configurations. I am still confused about how to configure this. I read all of the comments, but would be great if someone can help me.

I have a simple text as <p><span>MY TEXT HERE</span></p>

I am using dev branch of the wysiwyg module, ckeditor 4.5.3 and when I edit the page the ckeditor shows blank i.e. it destroys all text.

When I click on disable rich text editor it shows the html.

Not sure what should be the configurations for my html to retain everything which was entered previously.

Thanks,
Sadashiv.

kerasai’s picture

@sadashiv, sounds like input format configuration to me. Make sure it's allowing <p> and <span> tags

If so, your issue isn't directly related to the ACF configuration.

sadashiv’s picture

I double checked that I am not filtering html tags. I am sure that it's ACF because If I use a different rich text editor like IMCE then it doesn't trim anything.

Thanks,
Sadashiv.

rocketeerbkw’s picture

@sadashiv open a new issue and include what the current settings are for ACF

sadashiv’s picture

Hi,

I created a new issue at https://www.drupal.org/node/2636384

Thanks,
Sadashiv