the media button in ckeditor 4.1.0.80c139aa always produces a string "false" when saving the node or switching to html-sourcecode (button in ckeditor).

the same procedure with tinymce 3.5.8 works as expected. the sourcecode-view of the tinymce editor shows the media-tag correctly, with styles, floating, alt, title etc.

text-format and wysiwyg-format preferences of ckeditor and tinymce are identically.

why does it not work in my favorite, ckeditor? ^^

CommentFileSizeAuthor
#25 issue-1951964.patch597 byteslsolesen
#21 issue-1951964.patch666 byteslsolesen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dddave’s picture

Just to be sure: Are you aware of #1853550: Ckeditor 4.0 - The version of CKEditor could not be detected.? Have you downloaded the "Full" project of CKEditor?

Frank Pfabigan’s picture

yes, i did ^^

running the full ckeditor package and have the latest wysiwyg-dev with the patch from 1853550 included. wysiwyg itself with ckeditor 4.1... runs as aspected, no problems here.

it makes me crazy that media + wysiwyg + ckeditor 4 doesn't work. i searched for hours in the media issu queue and in google for a solution... now i need help ;-)

kiyoshin’s picture

Yes I' having same problem.
For temporary fix, I down grade CKEditor 4.0.1 from 4.1.

dddave’s picture

Too bad, I really hoped this was a simple misconfig. ;)

CKEditor is heavily changing which causes lots of troubles (than can be witnesses in the linked issue). I for sure cannot understand where the issues here lies. Sorry.

Frank Pfabigan’s picture

i thought it might a plugin (of ckeditor) that disturbes the media button behaviour, but there are a lot of changes in ckeditor itself, too.

it tested following:
i deleted one ckeditor-plugin after another, and tested everytime, if media button worked... no luck.

so perhaps the error lies within ckeditor itself or in a combination with a plugin.

after that, i downloaded ckeditor 4.0.1 and exchanged it with my installation of ckeditor 4.1.x in sites/all/libraries. the media button then works as desired, but this is just a workaround. and, doing so will confuse the icons for the editor-buttons. unfortunately there is no way to use ckbuilder with an older release of ckeditor (4.0.1). so you can't just download and throw it on ftp.

as some of the ckeditor-preferences are at plugins-level and some in the ckeditor.js itself, it's very complicated for me to get all desired ckeditor stuff to run together with the media-button.

perhaps someone else has another clue to solve this problem? - maybe mr Frederico Knabben itself? ^^

Frank Pfabigan’s picture

interesting... 4.0.1 in sites/all/libraries/ckeditor. took ckeditor.js from 4.1.x, copied it into sites/all/libraries/ckeditor and... media button works. what the heck is going on here? ^^

so the error comes from anything else in the 4.1.x package, not ckeditor.js itself. but it needs a better javascript/php developer than me to find out what's wrong with the 4.1.x package...

edit 10 minutes later:
nope, didn't work really. previous ckeditor.js (4.0.1) was in browser cache. now that the browser recognized the change of ckeditor.js file finally, it does not work any longer. pitty.

damontgomery’s picture

I think we are having a similar issue.

We load a node field (in edit) with WYSIWYG, CKEditor 4.1 and Media 2.x all most up to date dev versions.

The editor is completely empty while there was a media file (video or image) previously. Trying to add a new media file with the button in CKEditor does not work, still blank editor. We will try 4.0.1. Seems weird.

UPDATE:

We downloaded CKEditor 4.0.3 (newest 4.0 release we saw) and that seems to work with basic CKEditor functionality. The main issue is we couldn't get some buttons such as "HTML Block Format" which shows the HTML structure in the WYSIWYG to work. For now, that's ok for us.

Hope that helps anyone in a bind.

treksler’s picture

i can confirm the problem in media v1.3
the rest is the same setup as #2 (running the full ckeditor package and have the latest wysiwyg-dev with the patch from 1853550 included)

ckeditor 4.1.0.80c139aa does not insert the media into the text area
however ckeditor 4.0.3.0eafb700 does work

treksler’s picture

4.1RC also does not work

I am guessing, but the most likely culprit is the new data filtering that discards the Media tokens?
Seems like the media module needs to tell CKEditor what kind of code or tokens or tags it provides, so the editor doesn't filter it out .. again, just a guess at this point.

From the CKEditor Changelog:

+## CKEditor 4.1 RC
+
+* [#9829](http://dev.ckeditor.com/ticket/9829): Data and features activation based on editor configuration.
+
+  Brand new data filtering system that works in 2 modes:
+
+  * based on loaded features (toolbar items, plugins) - the data will be filtered according to what the editor in its
+  current configuration can handle,
+  * based on `config.allowedContent` rules - the data will be filtered and the editor features (toolbar items, commands,
+  keystrokes) will be enabled if they are allowed.
+
+  See the `datafiltering.html` sample, [guides](http://docs.ckeditor.com/#!/guide/dev_advanced_content_filter) and [`CKEDITOR.filter` API documentation](http://docs.ckeditor.com/#!/api/CKEDITOR.filter).
dddave’s picture

Title: Media Button in WYSIWYG CKEditor 4.1.0.80c139aa does not work » Media Button in WYSIWYG CKEditor 4.1+ broken
yettyn’s picture

This got me too... The simple solution to this is obviously to feed cke-4.1+ with this:

CKEDITOR.replace( textarea_id, {
	allowedContent: true
} );

or should that possibly be

var editor = CKEDITOR.replace( textarea_id, {
	allowedContent: 'p'
} );

I have no clue though where to do this. Also a question, do we really need this new fancy annoyance? Doesn't Drupal sanitize the output in the end preventing intentional or accidental evil? Personally I strongly dislike this idea that the editor should tell me what I am allowed to type or not!

I vote for simply switching off this new fancy annoyance. In such case, isn't this really a bug for wysiwyg module? I'm not sure of and cannot make that call though.

yettyn’s picture

Actually, as a workaround this change in config.js should do the job

CKEDITOR.editorConfig = function( config ) {
	// Define changes to default configuration here. For example:
	// config.language = 'fr';
	// config.uiColor = '#AADC6E';
+	config.allowedContent = true;
};

Note that the + sign just marks that the line was added and should not be put in the code.

camdarley’s picture

#12 doesn't work for me... image are still replaced by "false"... downgrading to ckeditor 4.0.3.0 works.

MXT’s picture

Same problem here:

  • using ckeditor 4.1.0.x doesn't work (images are replaced with "false" string)
  • using ckeditor 4.0.3.x works well
TwoD’s picture

Wysiwyg does not use CKEditor's default config file.
You can change the allowedContent setting from within hook_wysiwyg_editor_settings_alter(), see #1963270-2: CKeditor 4 converts Media tags to string "false".

The problem is that all plugins in CKEditor can now declare which tags they are able to generate, as well as all tags they must be allowed to generate to in ordered to be considered an active feature. We do not yet have the means to include such information in the cross-editor plugin API.

Frank Pfabigan’s picture

solution is here: http://drupal.org/node/1963270#comment-7355144 (wysiwyg issue cue).

jwilson3’s picture

We do not yet have the means to include information about which tags it must be allowed to generate in the cross-editor plugin API.

Is there an issue for this in the WYSIWYG issue queue?

TwoD’s picture

@jwilson3, I don't think so.

Martijn Houtman’s picture

For now, you can just add a setting to 'Advanced options' / 'Custom JavaScript configuration' at admin/config/content/ckeditor/edit/Advanced:

config.allowedContent = true;

Edit: sorry, you may ignore this, this comment belongs at the CKEditor issue (http://drupal.org/node/1504696#comment-7410626)

jghyde’s picture

Making a module or adding this hook to a module is the only way as of now to solve this issue. Since the Media module places an array of data into the textarea, trying to parse out the tag is complicated. So, just set this, make the CKEditor behave the way it did before CKEditor added the ACF feature, and your life will be swell.

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

See http://drupal.org/node/1963270#comment-7355144

Also, smart folks are working on ACF integration in D8 #1936392: Configure CKEditor's "Advanced Content Filter" (ACF) to match Drupal's text filters settings

lsolesen’s picture

Status: Active » Needs review
FileSize
666 bytes

Why not create it as a patch for the media module?

Status: Needs review » Needs work

The last submitted patch, issue-1951964.patch, failed testing.

lsolesen’s picture

Status: Needs work » Needs review

#21: issue-1951964.patch queued for re-testing.

yettyn’s picture

Is this really a Media issue? Not imho and I vote -1 to commit this patch. I stay away from being as bold as changing the status though, but I think it should be changed. A workaround is already available in the Wysiwyg issue que. It's also the correct module where to address this, together with the standalone CKEditor module.

In other words, this is a CKEditor/Wysiwyg bug that affects Media, not a Media bug and should properly be closed imho, but of course I leave that up to maintainers to decide ;-)

lsolesen’s picture

FileSize
597 bytes

Rerolled patch from #21 as it does not apply anymore.

TwoD’s picture

I would agree committing this patch is a bad idea. It will override any value Wysiwyg itself will ever be able to set, and maybe even the value set by site-specific modules (depending on the module weight in the system table). The plan is to have settings for configuring CKEditor's Advanced Content Filter manually, as well as allowing plugins to tell which elements/attributes they need to function properly. This is part of why I haven't promoted Wysiwyg 7.x-2.x-dev to 7.x-2.3 yet.

aaron’s picture

Status: Needs review » Closed (won't fix)

I agree. This is fundamentally not a media issue.

lsolesen’s picture

But we need to push for a fix, because it affects all media users with wysiwyg implementation with the newest ckeditor (which is chosen for d8) :/

jghyde’s picture

We're going to send all the newbies to Wordpress unless we fix these silly little problems that are hidden in esoteric Drupal.org issue queues. This module should work out of the box with Media! Someone be a leader and make this happen, please?

treksler’s picture

form #24

A workaround is already available in the Wysiwyg issue que. It's also the correct module where to address this, together with the standalone CKEditor module.

can you provide a link?

damontgomery’s picture

#16 and #20 here provide links to https://drupal.org/node/1963270#comment-7355144 which describes how to create a module (with a hook) to override the WYSIWYG settings to pair with CKEditor.

We're just using an old version of CKEditor for now (4.0.3).

Wim Leers’s picture

This won't be a problem in Drupal 8, #1936392: Configure CKEditor's "Advanced Content Filter" (ACF) to match Drupal's text filters settings ensures that CKEditor's ACF is tightly configured, automatically :)

Note that ACF is there for a reason: to ensure that what the user sees in the WYSIWYG editor will also show up on the front-end. i.e. setting allowedContent = true means that e.g. tables will be displayed happily by CKEditor, but will be stripped away by a default Filtered HTML text format for end users.
It's also there for security: to prevent people from blindly copy/pasting from other sites which might contain an iframe tag with malicious JS, which CKEditor would then execute.

So, yes, the "solution" here will make it work like before, but it will also prevent CKEditor from being better than the WYSIWYG editors of old.

Dave Reid’s picture

@Wim: How can something like inline entities (and hence rendered files) work in CKEditor? When you never know what markup would be generated by the output?

Wim Leers’s picture

#3: I think we should discuss that in a separate issue? :) If you want to discuss it here, tell me, and I'll reply here.

elvis2’s picture

#25 fixed my issue.

Here is a description of the issue I ran into. Hopefully this helps others...

I am setup with ckeditor library 4.3 along with wysiwyg api, media_youtube, media_vimeo, brightcove_field along with the media module (all modules were latest dev versions, as of Nov 15).

Every test for each media type (brightcove, youtube, vimeo) worked fine within the modal but after clicking save, to pass the media asset back to the wysiwyg editor text area, nothing would change within the editor.

Patch #25 fixed this issue. Thanks lsolesen!

elvis2’s picture

After reading many of the comments, I too agree that this should remain closed. This issue has already been handled by the wysiwyg module.

kenorb’s picture

Component: WYSIWYG integration » Code
Issue summary: View changes
Related issues: +#1606596: ckeditor keeps removing my code when switching in and out of source mode