Integration with CKEditor module

I just installed all the stuff to give the latest spark version a try and totally agree with hctom, the media integration doesn't work as intended.
Attached patch integrates the missing detach / attach functions and fixes some of the code in library.js - parts of the code were copied from the Media Aloha module I worked on.

Updated issue summary.
@hctom I hope it's oky that I've hijacked your ticket ;)

Applied issue summary standards.

Looks RTBC to me, haven't tested IRL so I'll leave it to ckeditor guys to commit it :)

Hey @das peter - I'm getting a 'patch unexpectedly ends in middle of line' error when applying the patch in #1 - would you mind re-rolling the patch to see if it fixes the problem?

Many thanks!

Mike.

Patch re-rolled.

Did anyone really test it? ;)

Did anyone really test it? ;)

Well, I tested it while developing. But that doesn't count for RTBC-ing ;)

@kevinchampion Thanks for the re-roll.

A couple of notes - all attempts:

1. I wasn't able to get this patch to apply, either with git apply or with patch -p1 (against -dev from Feb 9). With the latter, I kept getting errors that the patch had already been applied, etc. So I manually applied the changes.

2. On a clean D7.20 install, using Media 7.x-1.3, the patch almost works. If I insert an image, for example, and choose a view mode, the element is being tokenized. If I later edit the image style that that particular view mode uses, then the node in question reflects the change. However, if I select at different image style for that view mode (Configure->Media->File types and then 'manage file display' for the 'image' file type, etc.), then those changes are not reflected in the node unless I first edit and re-save the node (no other edits required).

Here's an example tokenized element (note: I'm using DS to create a custom view mode):

[[{"type":"media","view_mode":"test_mode","fid":"2","attributes":{"alt":"","class":"media-image","height":"180","src":"http://d7civi/sites/default/files/styles/square_thumbnail/public/cross2.jpg?itok=MNIGamsv","typeof":"foaf:Image","width":"180"}}]]

Note that, while the view mode is there in the JSON, so is the src, which is explicitly pointing to the image style generated file (in this case, the 'square_thumbnail'). So that's why changes to the image style work (because that file is just being regenerated), but unless I edit the node (thus re-triggering the attach and detatch methods which handle the tokenizing), changing to a different image style entirely don't take effect.

3. As a further point, when I try this patch on an existing site using Media 7.x-2.x-unstable7, there's no tokenizing happening at all.

1. I wasn't able to get this patch to apply, .... (against -dev from Feb 9). ...

I can't reproduce this, whether with git on the command line nor with the tortoise git UI. Patch #5 applies just fine to the latest code in the 7.x-1.x branch.

2. ... then those changes are not reflected in the node unless I first edit and re-save the node ...

Did you clear the caches? The token filter is a cacheable on, which means the parsed raw-data are cached as long as there was no re-save. If manually flushing the cache is an issue (after editing a view mode) this is something to report in the media issue queue.

2. ... Note that, while the view mode is there in the JSON, so is the src, which is explicitly pointing to the image style generated file (in this case, the 'square_thumbnail') ...

src is ignored, see media_token_to_markup(). As far as I remember the path in the src points to the placeholder of the media e.g. for Youtube-videos you'll see a thumbnail instead the video element while editing.
Btw. we didn't re-invent the tokenizing code, this is the one from media itself - just slightly adjusted (comments, error handling).

3. As a further point, when I try this patch on an existing site using Media 7.x-2.x-unstable7, there's no tokenizing happening at all.

I'm not sure what you mean by this? Are you pointing out that the existing embedded images aren't switched to tokes? Or that newly inserted images don't store the token?
We could introduce an update hook to initialize the token replacements. But I'm not sure if that wouldn't lead to further issues. Intentionally added image tags could be replaced as well, resulting in an worse situation than having non-tokenized tags.

does this work with ckeditor 4.1?

over at http://drupal.org/node/1951964 there is an issue where 4.1 strips the tokens

does this work with ckeditor 4.1?

I don't know, I think I developed / tested with a 4.0.x version.
@treksler As it seems like you've installed 4.1, how about giving it a try and posting the result here?

Alright,

First, as far as I can tell, the patch in #5 does not work (I double checked that it was not a caching issue)
a) if there is already a valid token in the code, it renders it as text (I verified the token to be valid with tinymce or ckeditor and wysiwyg module)
b) when using the media module button to insert an image, it inserts it as straight HTML (no token)

Second, using 4.1 or 4.0 makes no difference. (i.e. the token does not disappear on render like it does in 1951964)
This leads me to believe that if the patch worked for me, it would also work with 4.1

PS. I am using Media 7.x-1.3

In my case #5 works using the latest 7.x-2.0-dev of Media. I believe it also works with Media 7.x-2.0-unstable7.

@treksler Thanks for testing! "PS. I am using Media 7.x-1.3" I guess that's essential. I never tested it with the Media 1.x branch.

@kevinchampion That seems to proof my experience that the patch works with the Media 2.x branch.

The question is now how to continue. Frankly speaking in my eyes the only clean solution would be to refactor the code in media so that the token handling (replacement) is handled there and ckeditor (or whatever uses media tokens) just has to invoke the handling.

I've tried to unify the token / filter functionality across the Media 1.x / 2.x branch.
That way the ckeditor should be able to work with both versions.
To test it apply the Media patch matching you version and the ckeditor patch.
I've tested only with Media 2.x yet, tests with 1.x would be appreciated.

@treksler Would be awesome if you could give this new approach a try :)

The patches for ckeditor and media 2.x apply flawlessly.

(Can't test functionality because after hitting submit while embedding media I get: "TypeError: c is undefined", I guess unrelated to this issue)

Both patches applied cleanly against today's media-7.x-2.x-dev and ckeditor-7.x-1.x-dev with the ckeditor 4.0.2-full library. Cleared all caches, Drupal and browsers. But now I'm not getting any JSON-type token creation at all (tested on two sites - one of them a clean install). Instead, I get something like the following:

<img alt="A cross" class="image-style-medium media-element file-default attr__typeof__foaf:Image img__fid__12 img__view_mode__default attr__format__default attr__title__Cross" data-file_info="%7B%22fid%22:%2212%22,%22view_mode%22:%22default%22,%22type%22:%22media%22%7D" height="220" src="http://d7civi/sites/default/files/styles/medium/public/cross2_0.jpg?itok=7EVR2Sah" title="Cross" typeof="foaf:Image" width="147" />

... both in the editor and in the generated output. (In the above example, I inserted an image from the library using the 'default' file view mode, which itself is configured to use the 'medium' imagestyle preset.

If I later change the 'medium' imagestyle preset to, say, scale the image smaller (and clear the caches, just in case), the same output comes though the browser - a smaller, regenerated image is served up (as expected), but since the height and width attributes are hard-coded into that tag, it appears the same size as before, but pixelated.

I'm not seeing any errors in the js console, and no PHP notices or warnings. I'm willing to believe I've got my text filter/ckeditor profile settings out of kilter or in the wrong order, but I've moved things around to no effect.

The patches work for me now as well.
The hardcoded height and width could be a problem indeed, but still, good work!

Hmm, I've to check where the fixed height, width properties come from.
But I think the media token filter should take care of omitting those attributes if present.
I think it's more important to be able to properly use the files display styles than manually setting those attributes for media elements.

Yes, file display styles should be first priority and standard behaviour.
Could manually setting the width/height attribute overwrite this behaviour when wanted? Some non-tech editors might find it strange if nothing happens.

Another problem of the height/width added automatically arises when designing a responsive theme and using a width: 100%; height: auto; without !important.

As far as I understand the filter keeps all properties set in the variable wysiwyg_allowed_attributes. The attributes height and width are part of the default value. Unfortunately the value is not exposed in a settings form an you've to manually adjust it if needed.
The handling of manual set attributes works as described below:
The file formatters provided by the file_entity module e.g. file_entity_file_formatter_file_image_view() check for the property override in the given file object to set the attributes.
And currently the media module uses always the override to pass the attributes provided by the media token - see media_get_file_without_label().

I think you could argue it's sensible of the media module to use the override functionality to set the customized attributes and it makes sense that the image formatter allows it.
However, the use case described above (changing the image style) is nonetheless a valid one too.
So my purposed solution would be to add a display setting to the image file formatter to define whether the override of the height / width attributes is allowed or not.

Ok, new problem.
Testing on latest CKEditor 7.x-1.x-dev (version 4.1 full library) and Media 7.x-2.x-dev with both patches.
Adding an image via the media module results in an image in the CKEditor body field, but switching to plain text or saving prints: "false" instead of any image html or tokens.
Only when picking a display mode with no display is enabled (using file entity) a generic file icon is shown, with the appropriate html (no media tokens).

I found out that removing the "Stylesheet Classes" manually (via the CKEditor image properties popup, tab "Advanced") results in the image staying in place.
So after switching view, or showing the source or saving, the image is preserved.

Trying the following code in library.js, also works. But breaks all good intentions.

    forceAttributesIntoClass: function (imgElement, fid, view_mode, additional) {
      imgElement.removeClass();
    },

EDIT: Ok this is a CKEditor 4.1 problem.
See related issues: Media Button in WYSIWYG CKEditor 4.1+ broken and CKeditor 4 converts Media tags to string "false"

This is rather important since Edit module requires version 4.1+ as of recently. So we can hack around it by setting the "allowedContent" CKEditor setting to "TRUE" somewhere.

Additionally: can we block the default CKEditor image properties popup for files added with the media plugin? Since the changes aren't reflected. Or even better bring up the media plugin embedding settings again?

I've just created a issue in the media issue queue #1970370: Split media token handling from WYSIWYG integration javascript
Would be good if you could post the test-results as well.
I think we should streamline the media code first so that we can continue on a proper base.

I'm postponing this one then.

We haven't had plan to put specified Media 2.x support code in CKEditor any time before it have a stable release. About Media 1.x, I guess people want to follow #1950498: JavaScript and other tags are being removed after updating to CKEditor 4.1 to fix the "new feature" in CKEditor 4.1.

I change the issue title as I don't think Media 1.x will accept such a change. Feel free to revert it if that issue get into the next Media 1.x stable release.

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

config.allowedContent = true;

We haven't had plan to put specified Media 2.x support code in CKEditor any time before it have a stable release.

This sounds worrying to me.

At the rate the media module has been creating new releases it could be years before there is a full stable 2.0 release. It has been almost 2 years and they are only almost at alpha. Then I assume there will also be betas and release candidates.

They had a few large changes in the unstable release series but I would expect large changes to be almost non-existent in alpha and probably non-existent in beta.

Already 2.x users are at around 12.5% (13074) of the total D7 media module users (106499).
That will climb a lot before media hits 2.0.

Waiting until a stable release to start work seems crazy as then it will be longer past that point until integration is working.

I would think starting during beta or even alpha would mean that it is ready to go when 2.0 is released (most likely before as I would expect its features & apis will be pretty stable long before 2.0).

#26 I completely agree with you, but no CKEditor maintainer has time to add (broken?) support for Media 2.x, a unstable branch that Media maintainers do not recommend to use.

Media 2.x wysiwyg integration is changing a lot. Will it be easier to put the CKEditor support into Media?

Media 2.x wysiwyg integration is changing a lot. Will it be easier to put the CKEditor support into Media?

Possibly. I haven't looked into it yet but WYSIWYG module support is in media.

I think it should be because generally it is up to the other module to integrate with the editor, for example footnotes or image captions.

Really, the text editor module should provide the API and then the other modules handle integration, IMO.

I would be in favour of moving this issue to the media module.

I would be in favour of moving this issue to the media module.

@rooby: That was the idea of #1970370: Split media token handling from WYSIWYG integration javascript - having a stable token filter / WYSIWYG integration over all media branches. More attention to this issue would definitely help to push it.

#25 worked for me!

Thx, Martijn. )

@#15 - For those of you submitting patches, can you please not use console.log, its reference is a JavaScript bug in old versions of IE which some companies still force their developers to support.

I had a tough time getting the media.filter-js-2.x.patch patch from #15 to apply for me. Perhaps it was my version of Media 2.x-dev, but it failed on 2/4 hunks. I've rerolled that patch after manually applying the changes and attached it here, in case it's useful to anyone else. The CKEditor patch in #15 worked fine for me.

Here's another re-roll, also fixes a fatal js error when inserting videos (filter js was still looking specifically for <img> tags).

Err less misleading patch name!

media.filter.js was hardcoded to <img> tags which completely breaks inserting video. In fact any regexp breaks because the formatter could be anything.

There's already the tagmap which has all of these in it, so we can just use that instead.

Sorry got confused which issue I was on, moved the media patch over there.

Updated ckeditor patch (also fixes hardcoding to image, changes insertHtml to insertElement) attached.

Attached is the latest version of the patch that @catch and I have been working on. This version does a better job of markup replacement. Instead of doing exact string matches, which fail when the editor adds newlines or other minor manipulation to the HTML, it adds a marker before and after the HTML, and just replaces everything between the markers.

ugh, I made the same mistake, here is the ckeditor patch. The above media patch is actually for #1970370: Split media token handling from WYSIWYG integration javascript

@douggreen: your patches work fine for me, except for the fact that the root directory can not be found (/docroot/sites/all/modules/contrib/ckeditor/) in my install.

One thing I notice is that once the files have been placed, the user can edit an image using the ckeditor image popup. Changes to, for example, the image dimensions, however, are not saved. Is there a way to turn the edit ability of an image off? It's a tad bit confusing for the user.

Also, when I display the title field in the manage display tab of an image, the field is displayed nicely when viewing the content after saving, but not in the ckeditor window (it seems to be stripped out).

Nearly there, I think!

Fixed paths in patch. Patch was created in the docroot instead in the module folder.

patch does not apply cleanly against latest dev version (6/22/13)

aegir@devmac:~/p/7/m/all/ckeditor$ patch -p1 < ckeditor_1504696_37_0.patch

patching file includes/ckeditor.lib.inc
patching file plugins/media/library.js
Hunk #2 FAILED at 20.
Hunk #3 succeeded at 349 (offset 63 lines).
1 out of 3 hunks FAILED -- saving rejects to file plugins/media/library.js.rej
patching file plugins/media/plugin.js

Scratch the last comment.
this patch applies cleanly.
the effor was caused because of a conflict with another ckeditor media integration patch which I applied found here: https://drupal.org/node/1898958#comment-7190352

I tried the patch from #40 out along with #1970370: Split media token handling from WYSIWYG integration javascript which has been committed to Media's latest dev - it looks like we are close but CKeditor is still commenting out the media code after the embed is inserted.

Screenshot of the HTML that gets output after I go through the modal, select/upload an image, and set up embed settings:

@brantwynn Unfortunately I can't reproduce this issue (however, I had a hard time to convince my browser cache to drop the ckeditor media plugin and reload it). Could you please provide more information about your setup?
Here some data from my test:
- Browser: FF 23, Win8 64bit
- Configured all text formats to convert media tags, enabled CKEditor Media plugin for all profiles.
- Media: latest dev
- FileEntity: latest dev
- CKEditor Module: latest dev + #40
- CKEditor library: 4.2 standard.

Source view: shows <p>[[{"fid":"1","view_mode":"preview","type":"media","attributes":{}}]]</p>
WYSIWYG view: shows the image.

However I came across an issue too, it looks like I can't delete an inserted image / media in the wysiwyg. I guess that's because if I delete the image/media the wrapper (<!--MEDIA-WRAPPER-START-N--> still exists and is replaced later with the media token from the tagmap. If I delete the media token in the source view the media is gone. I guess we could add a condition to the converter to check if a wrapper is empty and if so ignore it. However, the downside of this could be that broken media output (empty output) is considered as deleted media.

Oh, wrong status - I'm working on a patch for the issue I mentioned.

I guess we won't drop support for Media 1.x when we plan to support Media 2.x.

Btw. here's the follow-up for the issue I've found in #45: #2066859: Removed media are restored on save in WYSIWYG
I thought it's best to handle this scenario in the media part as this is likely an issue other WYSIWYG editors will have too.

Okay - I did a rebuild and using Chrome 28.0.1500.95 with latest file_entity (dev HEAD), media (dev HEAD), ckeditor (dev HEAD w/ patch from #40) and CKEditor library 4.2 standard.

Things are better, but still not as expected.

After inserting an image, I am not seeing the rendered media inside the WYSIWYG.

However, when I switch to the source view, I do see the proper media embed code.

FWIW, the actual rendered node also shows the inserted media correctly.

Ok I did a rebuild again. This time using standard profile. (I was using DF distro in #49)

Using

Drupal (7.x HEAD)
CKEditor module (1.x-dev) + patch from #40
CKEditor library (4.2 standard)
File Entity (1.x-dev)
Media (2.x-dev)

Everything is working as expected now. I see the rendered entity in the WYSIWYG. I see the Media embed code in the source view. I see the rendered entity on all view modes.

Lets get a few more eyes on this but I think it can be RTBC.

Here's an updated patch that depends on an updated Media patch: #1970370-14: Split media token handling from WYSIWYG integration javascript

I can confirm #51 is working with the patch from #1970370: Split media token handling from WYSIWYG integration javascript

I think this is ready now.

#1970370: Split media token handling from WYSIWYG integration javascript landed so that makes things a bit easier.

From my tests, this patch is working with multiple CKEditor libraries:

ckeditor_4.1.3_standard - Works
ckeditor_4.2_standard - Works
ckeditor_3.6.6.1 - Works initially, but I see the embed code instead of the media when going back in to edit the node after having created it.

Screenshot showing 3.6.6.1 bug - unsure if that makes this as Needs Work. Is the intent to fully support 3.6.6.1?

Also went in to add another piece of Media here and the source shows it is not putting in the code correctly once editing the existing content.

Here's an updated patch. This one should fix the 3.x issue.
The code for initializing the html (setting the placeholders) seems to work totally different.
While working on this I came across some strange issues.
It looks like deleting the media placeholders in the wysiwyg mode can lead to serious trouble.

Next step will be to check how we can ensure the whole placeholder including the comments (<!--MEDIA-WRAPPER-START-N-->) is always selected when working in wysiwyg mode.
I'm afraid that this (potential) bug doesn't just affect this module but the wysiwyg module integration as well.

Tested patch from #55 with the following versions of CKEditor library.

ckeditor_4.1.3_standard - works!
ckeditor_4.2_standard - works!
ckeditor_3.6.6.1 - ....works!

I think this is ready to go now for real.

FWIW, I tested this too and got it working with all 3 versions of CKE as well.

I continued work on the patch. Here the exact scenarios I used to test (with Win8x64, Chrome28):

1. Adding text data
   • Add an image with the media plugin
   • Now set the cursor right next to the inserted image and enter some text
   • Use firebug (or another inspection tool) to check where the text inserted
   • Text shouldn't be within the media wrapper tags!
2. Adding another image
   • Add an image with the media plugin
   • Now set the cursor right next to the inserted image and insert another image
   • Toggle source / wysiwyg mode and check if both images are properly handled
3. Removing image
   • Add an image with the media plugin
   • Now set the cursor right next to the inserted image and hit backspace
   • Toggle source / wysiwyg mode and check if all parts of the media code are removed

Unfortunately all those cases seemed to fail with the patch from #54.
The new patch uses a custom tag (<mediawrapper> instead comments (<!--MEDIA-WRAPPER-START-N-->) to wrap the placeholder. That way I've at least a chance to handle selections.
The new code does it's best to ensure the whole media wrapper element is selected if e.g. the placeholders image is selected.
Further it tries to move any selection within the media wrapper to its outside to ensure text data aren't inserted into the media wrapper but it's parent element.
Unfortunately this doesn't work as expected in CKEditor 4.
I've hacked a bit around that: if the cursor is set right next to the placeholder and there's already text after the placeholder you'll see that the cursor is moved always to the second char in the text (ugly).
That's because CKEditor 4 resisted all my attempts to avoid inserting text into the media wrapper while adding text right next to it. Unfortunately this hack works only if there's already text after the media wrapper, otherwise CKEditor 4 still happily inserts the text into the wrapper.

As I'm definitely no CKEditor expert every feedback would be appreciated and if someone is bored, here are the API documentations of CKEditor ;)

• Overview: http://docs.cksource.com/Main_Page
• v3: http://docs.cksource.com/ckeditor_api/
• v4: http://docs.ckeditor.com/

@das-peter FWIW, I've done some similar custom WYSIWYG and related plug-in implementations for clients relating to multimedia functionality similar to what you've been working on for CKEditor (although the work I did was on TinyMCE). The approach/conclusion I came to when doing so about 2 years ago was similar to what you've been doing here, especially with the most recent approach. In particular, I ended up implementing a custom wrapper for each plug-in that I could consistently rely on the identify and also increment the media elements, with the later being helpful for purposes of removing and cleaning up tags prior based on user input/upon certain triggers. There are likely better/more elegant options, but that approach did get the job done for immediate purposes.

There is another issue in media-filter.js.
#2067063: Wysiwyg integration is broken

Seems that the new token handling, introduced in #1970370: Split media token handling from WYSIWYG integration javascript doesn't work quite well

it might affect this issue.

@das-peter I tested the above patch with the latest dev versions of File Entity, Media and CKEditor and it worked well for me with both CKE 3.6 and 4.1.

@das-peter:

Updating old comments makes it almost impossible for people to know what you changed and it is confusing when you come to see a new one and have to trawl through the timestamps/new flags to find the latest comments.

Best to use new comments where possible and making note of what you edit in old posts if you have to do so.

I did some research on how this integration works with media_internet, media_youtube, media_vimeo and I am posting what I found out.

The integration (patch #58) works with pre-September releases of ckeditor, file_entity, media, media_youtube, media_vimeo, and with ckeditor library 3.6.6.1. The new version of media_youtube in particular breaks the inserting of youtube videos. I'm not sure about the rest of the modules - it is possible they can be updated.

I couldn't make the media_internet work for youtube/vimeo videos with ckeditor library 4.2.1, with any setup. If someone has more success with this, please post your findings. Marking this as "Needs work".

I would like to include media_crop to this for testing as well. I've been trying to get this work for with media 2.x and ckeditor with patch #58, but I'm not having any luck. Without the patch, None of the images are tokenized, but with the patch things don't work either.

One problem is that when the image is passed through ckeditor's JS, the media crop image isn't created yet, and thus the token generated doesn't know of the media crop ID yet. I can kind of fix that in media_crop.image_replace.js by replacing the values in Drupal.settings.tagmap, that contain the {MDIID} placeholder.

Another issue I notice is that I can't get to call media's media_token_to_markup() not anymore somehow. Edit: got this working again.

On top of that, there's 2 places in the media's JS where it cuts of attributes from the info object, so it can't create longer media tokens.

My findings:

• You need to turn off CKEditor's Advanced Content Filter to avoid mangling Media plugin's short code/token
• Adding attributes to an image placed via Media plugin (which uses [[{...}]] format) doesn't store them anywhere, so they get immediately lost as #64 noted
• Media button doesn't work for edit in place (with Spark) - guessing that the Media CKEditor plugin JS isn't getting loaded

As mentioned in #60, there is another issue going on in media module that particularly affects image resizing, discussed at #2067063: Wysiwyg integration is broken. Using patch #57 on that issue, which resolves the image resize issue, still breaks some JS introduced in patch #58 here. Here is a small patch that corrects that. Again, apply #58 above before this patch. I suppose until that patch gets committed to media module this is sort of a fringe case.

It took me way longer than I wanted, but here's a patch that introduces media as a widget in CKEditor >=4.3.
It provides the same functionality for all earlier versions of CKEditor as before - with the same issues - but from version 4.3 on it uses the new widget API to ensure the Media specific markup is handled as one block.
For now the integration is very basic but from here we could add inline editing of portions of the media markup.

@dafeder The new widget behaviour will essentially effect that because resizing has to be implemented specifically for it.

Thanks for the patch, but unfortunately it does not apply against the current 7.x-1.x HEAD version, so I was not able to test it.
I see the following output after "git apply ckeditor_1504696_67.patch":

error: cannot apply binary patch to 'plugins/media/icons/hidpi/media.png' without full index line
error: plugins/media/icons/hidpi/media.png: patch does not apply
error: cannot apply binary patch to 'plugins/media/icons/media.png' without full index line
error: plugins/media/icons/media.png: patch does not apply
error: cannot apply binary patch to 'plugins/media/images/icon.gif' without full index line
error: plugins/media/images/icon.gif: patch does not apply

I've removed the sections in the patch that were causing the issues (some image files) and the patch applied.
Posting it here and I'll start experimenting with it.
I'll probably be slow in testing, because this patch conflicts with some other patches that I am using currently for this module.

List of all the patches I am using now:
projects[ckeditor][patch][] = "http://drupal.org/files/ckeditor-media-plugin-files-1898958-4.patch"
projects[ckeditor][patch][] = "http://drupal.org/files/ckeditor-media-alt-title-1475774-2_0.patch"
projects[ckeditor][patch][] = "http://drupal.org/files/2024149-ckeditor-media-broken-image-ie8-2.patch"
projects[ckeditor][patch][] = "http://drupal.org/files/ckeditor-translation-1203846-17.patch"
projects[ckeditor][patch][] = "http://drupal.org/files/ckeditor-media_ie_fix-1914904-4.patch"

Patch in #67 is working fine for me - using it during a drush make build. Applies to current HEAD. #68 May be an issue with needing to apply the patch differently. Maybe try git apply [patchname] --index

Tested functionality manually and I am able to use the media button in ckeditor to insert an image onto the WYSIWYG.

Example markup:

<p>[[{"fid":"1","view_mode":"default","type":"media","attributes":{"height":"283","width":"481","class":"media-element file-default attr__typeof__foaf:Image img__fid__1 img__view_mode__default attr__format__default attr__field_file_image_alt_text[und][0][value]__ attr__field_file_image_title_text[und][0][value]__"}}]]</p>

No interdiff provided but I am thinking the patch posted in #69 is probably unnessecary.

I'm RTBC'ing #67 here.

@brantwynn Could you also test with CKEditor 4.3? That version handles the whole thing a bit different because it uses the widget API instead plain markup.

@brantwynn: Could you please clarify the versions of the ckeditor library you used. Thanks.

@DamienMcKenna: I think we should re-test all the cases listed #2067325: [META] Tracking WYSIWYG compatibility as I've made quite some changes with the last patch. I've tested 4.3 primarily.

@DamienMcKenna - using 4.2 - http://download.cksource.com/CKEditor/CKEditor/CKEditor%204.2/ckeditor_4... -- tested and works.

I tried switching out the library to 4.3 beta but that makes the editor look crazy and the media button is nowhere to be found.

After that test, I also switched out the library 4.3 for 4.2.2 - in that case the editor looked fine except the media button was missing. I was able to click the empty space where the media button would be and the functionality was there for adding an image.

Is the goal to support a beta version (4.3) or just the latest (4.2.2)?

If anyone wants to re-test this for #2067325: [META] Tracking WYSIWYG compatibility in a much more thorough manner, thats likely what we'll need done to get this back to rtbc.

Is the goal to support a beta version (4.3) or just the latest (4.2.2)?

I think the goal is to support everything >= 3.6
Unfortunately the approach with the wrapper tags/comments has massive flaws as I pointed out several times. Thus I'd prefer to go for 4.3 where the widget API/plugin was introduced which solves all those trouble.

@brantwynn Could you please re-test? I've tested again (Win8, FF24) and only found an issue with 4.0.x versions.
The attached patch should solve that.

Fyi to test the different versions I switch the version using the git tags (https://github.com/ckeditor/ckeditor-releases).
But to test 4.3 beta I've to use the code provided here: http://ckeditor.com/download - if I use the related git branch (https://github.com/ckeditor/ckeditor-dev) the editor doesn't show up at all but also doesn't throw any error.

Updated issue summary.

With the patch from #76 I ran into the issues outlined in #74.

I tried switching out the library to 4.3 beta but that makes the editor look crazy and the media button is nowhere to be found.

There are some changes to the buttons in the latest version of CKEditor which cause the editor to look weird if you've got some of the previous images or CSS cached. I found that the buttons appeared properly after clearing my browser cache.

After that test, I also switched out the library 4.3 for 4.2.2 - in that case the editor looked fine except the media button was missing. I was able to click the empty space where the media button would be and the functionality was there for adding an image.

The patch in #76 attempts to create two new .png image files, as required by the CKEditor 4.x plugin API, (media/icons/hidpi/media.png and media/icons/media.png) but it doesn't include the image files themselves. Attached is a patch that is identical to #76, except it includes a binary of the current Media icon converted from .gif to .png.

@das-peter if you have a better icon and/or a HiDPI version can to post an updated version of #76 that includes the image binaries? (git add the files and then git diff --cached --binary > example.patch).

Functionally the patch seems to work great! :)

I'm on the fence about marking this as "needs work" - I have run into one issue with #77. If one is attempting to use the Edit module with CKEditor, the modal does not appear and I am seeing the following in my console:

Uncaught TypeError: Cannot read property 'plugins' of undefined plugin.js?t=D8HF:45
editor.addCommand.exec plugin.js?t=D8HF:45
exec
CKEDITOR.tools.extend.execCommand
CKEDITOR.tools.extend.click
k.execute
(anonymous function)
(anonymous function)
CKEDITOR.tools.callFunction
onclick get-cultured-tour-london:1

The functionality is there on a node edit page. It is only in the Inline Edit scenario that we see this issue.

I had things working fairly well after applying the patch in #77 (using library 4.2.2). However, I just switched the library to 4.3 (looks like it is out of beta) and now I get no CKEditor showing up at all. Just a gray box. If I disable the Media plugin CKEditor works correctly.

I've tried clearing caches in Drupal and in my browser.

EDIT: Changing to CKEditor 4.2.3 fixed things right up.

As CKEditor co-maintainer, I suggest move this issue into Media module so that code between WYSIWYG and CKEditor module integration can be shared.

Was moved to wrong module.

Given that ckeditor module has moved this to media and media has stated frequently that it is not supporting ckeditor module, what should we do with this from here?

Define "support"? CKEditor 4.2 works with WYSIWYG and Media when the patch from #1956778: Ckeditor 4.1 ACF is applied.

I'm referring to support for the standalone ckeditor module, not using wysiwyg module. I'm basing the conclusion that media is not interested in supporting ckeditor standalone module from the conclusion of this issue:

https://drupal.org/node/1840632

I believe other issues have been closed as well based on the same premise.

Given that this issue was a ckeditor standalone module issue seeking to resolve integration with media module in the ckeditor standalone module media plugin, but has now been moved to media module queue by a ckeditor standalone module maintainer, and in context of the points above, it would seem this issue is a bit lost.

The competing namespaces of all this is truly confusing...

This patch adds on to #77 to provide support for applying the alt and title attributes to images.

Thanks for the reminder, Kevin, I've got a touch of flu brain today and I apologize if I missed the obvious.

I've applied patch from #85 to latest dev with 4.3 library, I still cannot see the media module embed icon available on my profile edit page.

I have ticked:
- Plugin for inserting images from Drupal media module
- Plugin for inserting Drupal embeded media

Hello folks,

thank you for your work. You did a good job!

Here is a updated version of the patch, it fixes the following things:

• The "Add Media" Button now appears in CKEditor 4.3
• Patch now works with Media 7-x.2-x and CKEditor 7-x.1-x. (The problem was, that Media 7-x.2-x implements different markup at several versions of the module)

My setup:

• Media module dev, CKEditor module dev
• CKEditor 4.3 custom version. The widgets plugin is not included in the default sets!!!
• I picked the optimized version of custom CKEditor 4.3. The source version did not load the wysiwyg!
• In CKEditor config, i checked "Plugin for inserting images from Drupal Media module" and dragged the button to the toolbar

Important: I did NOT check the patch with other module and CKEditor versions, for example the current Media "stable" release.

What worked:

• Add Media (images and videos) to the editor
• Media assets are kept in the widget, even after save and editing the node again.
• Drag & Drop within the editor works very well
• Videos can be played within the editor.

What does not work:

• Text align does not make any effect and seem to destroy the widget code
• copy&paste removes the media token and adds html-markup to the pasted area

What I am missing:

• I want to change the view mode after adding the media asset to the wysiwyg.
• I want to change the media asset id after i added it to the wysiwyg. I think this should be possible now

Please test and improve the patch :)

Greetings,
axe312

I have tested the patch in #88 with the same setup as axe312.

I added an image and the image stayed after saving and re-editing. Did not test drag and drop (I may not have that enabled I guess) or video.

To add to axe312's 'Text align' comment: If I only add an image and no text, the text align works correctly. It did not appear to allow me to enter any text if I added the image first, however. If I add text first, insert an image, and then try to edit the image properties it does not appear to pick up the image URL correctly so I can't apply a text alignment at all. If I click Source, and the back out, I can then select the image and choose 'image properties' and it then picks up the URL and the text align works correctly. This seems to be different than axe312's results. The text align seems to stay in place after a save and re-edit as well.

This seems to be very close now!

Thanks for working on this everyone. Are patches for CKEditor to continue now that this issue is moved to Media? I thought the plan was to move forward with removing the Media stuff from CKEditor and working on a patch for Media module?

See: #2140155: Remove media plugin

Awesome to see other's jump in here :)

Are patches for CKEditor to continue now that this issue is moved to Media?

Well, again some history. I originally started working on the Media WYSIWYG stuff because I wanted to support the spark initiative and thus the, then favoured, aloha editor.
To do so I wanted to clean up the token handling and the integration into the WYSIWYG module of the Media module.
The code was unnecessarily mixed up and so I started working in this: #1970370: Split media token handling from WYSIWYG integration javascript
Unfortunately somewhere along the way other changes were introduced into that patch as well, what lead to huge confusion.
In the mean-time spark switched from aloha to CKEditor and so I started to work in this issue too.
And because of the "other changes" in #1970370: Split media token handling from WYSIWYG integration javascript e.g. the unlucky wrapper tag the things became messed up quite a bit.

Now to the question itself - as far as my understanding goes the Media module maintainers have, understandably, no interest in supporting other WYSIWYG integrations. Thus it was an important step to clean up the JS(-API's).
And I personally think Media and it's maintainers should focus on Media and File Entity and not (other) WYSIWYG integrations.
Thus from my perspective this issue belongs to CKEditor.

My idea is that the integration of the Media into the WYSIWYG module should provide just the basic functionality and we can do our own in-depth integration for CKEditor here and leverage all the features of CKEditor where the WYSIWYG module is limited due it's requirement for interoperability between different WYSIWYG libraries.

Are there other ideas / opinions around? Does anybody see disadvantages of the outlined idea?
Feedback very welcome!

CKEditor is going to be the default WYSIWYG editor built into Spark and Drupal 8, correct? Wouldn't the Media team very much want to support CKEditor then? Even in preference over supporting the WYSIWYG module? There's probably a lot of history and maybe some politics I don't understand, but on the surface of it, CKEditor seems very important to support.

On another note, and I may be displaying my development ignorance here, but would it make sense to move this into a 'CKEditor to Media Bridge' module in the vein of the 'WYSIWYG to IMCE bridge' module? Seems that would make both the Media and CKEditor maintainers happy :) But again, from a development stand point that might be a terrible idea. I just don't know what is involved.

the implementation of ckeditor that went into core for d8 is, for the most part, the wysiwyg version iirc.

much as the ckeditor module was my preference for a long time, once the extensive visibility settings were removed, the main advantage it had was no longer applicable. I've been using the wysiwyg version for d7 exclusively, and it works great.

My guess is there won't even be a ckeditor module for d8.

also, there's basic image uploading/insertion built into core's ckeditor implementation as well.

I continued working on the patch primarily to be up to date again.
Changes:

• Added edit feature: Select an placeholder, hit the media button and the media configuration pop-up is displayed.
• Fixed CKEditor 4.3 widget integration - the widget feature is only used if the plugin is available.
• Some ugly handling to deal with the legacy media wrappers - just in case there's someone with that media version. Should be removed asap.
• Kind of forcefully allowed the tag attribute data-file_info to ensure no media infos are stripped.

I really hope this time I managed it to properly include the binary files in the patch ;)
Some more refactoring would really be required but for now I would be happy if we could manage it to get this properly working.

From #91

Thus from my perspective this issue belongs to CKEditor.

I completely agree with this.

From #92

would it make sense to move this into a 'CKEditor to Media Bridge' module in the vein of the 'WYSIWYG to IMCE bridge' module?

I've had the same thought. If ckeditor module no longer wants to support this and if it doesn't really belong in media module, perhaps there could be a way of splitting it out as a separate contrib. Not ideal, but it could help things to move forward.

@das-peter Is this a viable option? Do you have any interest in heading in that direction?

Its very nice to see that much activity in this issue!

My colleagues are trying to start a BoF about this the next Days at DrupalCamp Vienna. You can contact them via our wunderkraut munich booth! I won't be there, I am sick and can not attend :/

I'll test your patch tomorrow, what you've done sounds very good. I just need some sleep now :)

Thus from my perspective this issue belongs to CKEditor.

I'd also agree with this.

would it make sense to move this into a 'CKEditor to Media Bridge' module in the vein of the 'WYSIWYG to IMCE bridge' module?

Seems to be a good idea, if no maintainer of the both modules will support the "Media to WYSIWYG" implementation.

Couldn't get the patch in #95 to apply.

Edit: My bad, I applied the patch incorrectly. Ignore this.

#95 applies fine for me to 7.x-1.x.

With ckeditor 4.3 custom including widgets plugin, I didn't get any tokenization happening.

With ckeditor 4.2.3 full, file_links are broken and not tokenizing for me and image alt and title attributes aren't applied on embed from the media browser.

Builds off of #95 to provide support for applying the alt and title attributes to images.

would it make sense to move this into a 'CKEditor to Media Bridge' module in the vein of the 'WYSIWYG to IMCE bridge' module?

I've had the same thought. If ckeditor module...

@das-peter Is this a viable option? Do you have any interest in heading in that direction?

The interesting thing is that there is already something like a Media WYSIWYG module. Of course, currently it's dedicated to handle file display modes - but why? We could convert that to a module that takes care of the whole WYSIWYG integration. Including different WYSIWYG libraries.
It would make sense to have this module enabled always if you use WYSIWYG editors - I think the more powerful Media becomes (embedding videos n stuff) the worse the issue with the WYSIWYG placeholders (multiple tags, no wrappers and so on) will become.

So yes, as far as I'm concerned creating a dedicated module for the purpose of handling all the WYSIWYG related stuff is a good idea.
However, I'm not interested at all to become maintainer of a contrib module within the media module repository. I think the whole thing is volatile enough already and I won't help :P
The disadvantage of having an own repository would be the compatibility handling between the Media versions, especially the backwards compatibility to versions that brought their own WYSIWYG integration.

Next step: Let's check what all the maintainers of Media think about this idea.

@kevinchampion Thank you very much for restoring the attribute stuff. I thought that was a legacy piece :|

@deggertsen Did you use the --binary flag when applying? The binary stuff in the patch makes trouble sometimes.

@axe312 Unfortunately I'm not in Vienna - couldn't make it. But I'll try to stay online as consistently as possible so I should be reachable (luckily it's in the same timezone ;) ).

Hey all,

I was in need of something like this today and wrote a quick module to replace all file upload/browsing buttons within ckeditor with the media browser popup. While it is a little different than the discussion here, it solves a very similar issue. See: https://drupal.org/project/ckeditor_media

The reason I did it this way was that I'm creating various CKEditor widgets and many of them have images. I don't want to assume that we're using the media popup on them so I coded them for the generic file browser that comes with ckeditor. Then I created this module to override the functionality of the file buttons and use media instead. That way I can use the widgets with media, ckfinder, the default file functionality or any other file browser.

I'd be in favor of moving the WYSIWYG integration into a submodule. I'd also like to split out any other units of functionality, such as multiple file support (plupload/multiform integration, etc), into separate sub modules as was suggested in some of the talks/articles about media that came out of DrupalCon Prague.

I'm also for CKEditor module integration and employing the new CKEditor widgets API to help address some longstanding issues with clientside editors messing with markup on attach/detach, etc but we'll definitely need to maintain backwards compatibility with the current WYSIWYG implementation. I'd support maintaining the status quo for WYSIWYG integration and having new development occur in a CKEditor-specific plugin. I can see Media for Drupal 8 only shipping with support for the default CKEditor module with support for other editors left up to other projects.

As the media integration included with CKEditor is being removed, the current patch will need to be pieced out and moved into Media as appropriate and rerolled to have the CKEditor plugin included with and registered by Media. I'd feel even better if someone who worked on this issue having some form of a commitment/maintainership to the WYSIWYG sub module, (even if they try to avoid it *cough* @das-peter *cough* ;)).

@Devin Carlson:

Yesterday I was talking with das-peter about the sub-module. He'll create a proposal about that stuff this weekend and will post it on sunday evening.

Our current plan is to create the submodule based on the code in this patch. Me and my colleagues are going to help with the implementation and may get the first running version next week! Stay tuned :)

I've just created following issue #2142571: Spin-off WYSIWYG support in dedicated module / project
Please help me to polish the proposal and once it's worth to review / discuss to gain some attention from Media maintainers :)

@Devin Carlson *cough* I don't know what you mean *cough* :P
@randallknutson Sound's interesting, I'll happily take a look into the module if I find some time :)

I've applied #102 against latest ckeditor dev and latest media dev. The button works great for images, however for files (I'm attempting with PDF's), when you switch to plain text or save the node it strips the media tag.

I have disabled 'Advanced Content Filter' and added config.allowedContent = true; to custom javascript configuration field.

I can confirm #109 and have traced it to this chunk in library.js:

      // Insert element. Use CKEDITOR.dom.element.createFromHtml to ensure our
      // custom wrapper element is preserved.
      if (wysiwygHTML.indexOf("<!--MEDIA-WRAPPER-START-") !== -1) {
        ckeditorInstance.plugins.media.mediaLegacyWrappers = true;
        wysiwygHTML = wysiwygHTML.replace(/<!--MEDIA-WRAPPER-START-(\d+)-->(.*?)<!--MEDIA-WRAPPER-END-\d+-->/gi, '');
      } else {
        wysiwygHTML = '<mediawrapper data="">' + wysiwygHTML + '</mediawrapper>';
      }

Returning this chunk to what it was before #88 allows the media token to be preserved:

      // Insert element. Use CKEDITOR.dom.element.createFromHtml to ensure our
      // custom wrapper element is preserved.
      wysiwygHTML = wysiwygHTML.replace(/<!--MEDIA-WRAPPER-START-(\d+)-->(.*?)<!--MEDIA-WRAPPER-END-\d+-->/gi, '<mediawrapper data="$1">$2</mediawrapper>');

I'm not totally clear on the purpose behind this code, so haven't been able to determine a fix that preserves the addition but doesn't break document tokens.

Some notes:

Media dev from Nov 13th.
Ckeditor dev.
Ckedtior library 4.3 with widget support.
Patch from #102.

Tokens are incorrent until I changed in plugin.js:

data = data.replace(/<mediawrapper data="(.*)">(.*?)<\/mediawrapper>/gi, replacement);

to

data = data.replace(/<mediawrapper"(.*)">(.*?)<\/mediawrapper>/gi, replacement);

Also editing (#95) don't work for me.

This is what my html code looks like in editor (breaks added):

<p>
	<span tabindex="-1" contenteditable="false" data-cke-widget-wrapper="1" data-cke-filter="off" class="cke_widget_wrapper cke_widget_inline cke_widget_selected" data-cke-display-name="mediawrapper" data-cke-widget-id="1">
		<mediawrapper data="" data-cke-widget-data="{}" data-cke-widget-keep-attr="0" data-widget="mediabox" class="cke_widget_element">
			<a href="/file/2" data-file_info="%7B%22fid%22:%222%22,%22view_mode%22:%22teaser%22,%22type%22:%22media%22%7D" class="media-element file-teaser" alt="" title="">
				<img typeof="foaf:Image" src="http://bazaar.dev.local/sites/default/files/styles/medium/public/moonrise_over_san_francisco_cr.jpg?itok=TtLGJEN4" alt="">
			</a>
		</mediawrapper>
		<span class="cke_reset cke_widget_drag_handler_container" style="background-color: rgba(220, 220, 220, 0.498039); background-image: url(http://bazaar.dev.local/sites/all/libraries/ckeditor/plugins/widget/images/handle.png); top: -15px; left: 0px; background-position: initial initial; background-repeat: initial initial;">
			<img class="cke_reset cke_widget_drag_handler" data-cke-widget-drag-handler="1" src="data:image/gif;base64,R0lGODlhAQABAPABAP///wAAACH5BAEKAAAALAAAAAABAAEAAAICRAEAOw%3D%3D" width="15" title="Click and drag to move" height="15" draggable="true">
		</span>
	</span>
</p>

This is how token looks like in source mode (breaks added):

<p>
	<mediawrapper data="">
		[[{"fid":"2","view_mode":"teaser","type":"media","attributes":{"class":"media-element file-teaser"}}]]
	</mediawrapper>
</p>

Update:

Ckeditor not-patched

- Latest Ckeditor Dev (with no patch applied)
- Latest media dev with media_wysiwyg enabled.

• Adding images via media browser works great.
• Adding files via media browser does not work – Only outputs a file icon..

Ckeditor patched from #102

- Latest Ckeditor Dev (patch from #102 applied)
- Latest media dev with media_wysiwyg enabled.

• When clicking the 'Edit' tab, getting a fatal error:

Fatal error: Call to undefined function media_include_browser_js() in /usr/local/apache2/htdocs/d7multi/sites/all/modules/ckeditor/includes/ckeditor.lib.inc on line 824

Two patches: one for CKEditor and one for Media. The patches integrate the CKEditor plugin from #95 into Media, removes the dependency on the WYSIWYG module from Media: WYSIWYG, allows Media to alter the bundled CKEditor Media plugin and changes CKEditor to include the proper JS when using Media WYSIWYG instead of the bundled CKEditor Media plugin.

Everything is functioning fine, except I'm getting a "Uncaught TypeError: Cannot read property 'length' of undefined" error when pressing submit on the media format form.

@Devin: the patch for CKEditor looks very correct and is a nice clean up. Do you mind filing it against CKEditor module?

@Devin
The patch for ckeditor changes the way attributes are saved. The wysiwyg module stores the attributes in the data-file-info attribute of the img tag.
Do you see any reason to put them into the class attribute?

if we use the create_element and create_macro methods in media_wysiwyg.filter.js they would take care of the attribute handling.

if #2126697: Wysiwyg -- Alt and Title fields require some special handling. gets committed, we can change

this

      // Customization of Drupal.media.filter.registerNewElement().
insertMediaFile: function (mediaFile, formattedMedia, ckeditorInstance) {
      var element = Drupal.media.filter.create_element(formattedMedia.html, {
        fid: mediaFile.fid,
        view_mode: formattedMedia.type,
        attributes: formattedMedia.options
      });

to

insertMediaFile: function (mediaFile, formattedMedia, ckeditorInstance) {
  var attributes = Drupal.media.filter.parseAttributeFields(formatted_media.options);
  var element = Drupal.media.filter.create_element(formatted_media.html, {
          fid: this.mediaFile.fid,
          view_mode: formatted_media.type,
          attributes: $.extend(this.mediaFile.attributes, attributes)
});

to take care of the special handling for the alt and title fields.

Here's a little bit more about that error @Devin mentioned in #113. It's making it so that it wont submit. I haven't figure out how to get it fixed yet.

Uncaught TypeError: Cannot read property 'length' of undefined jquery.min.js?v=1.7.1:2
e.extend.each jquery.min.js?v=1.7.1:2
Drupal.media.filter.create_element media_wysiwyg.filter.js?mxeuhr:92
Drupal.settings.ckeditor.plugins.media.insertMediaFile library.js?mxeuhr:46
(anonymous function) library.js?mxeuhr:38
dialogOptions.buttons.(anonymous function) media.popups.js?v=7.x-2.x:188
s.click jquery.ui.dialog.min.js?v=1.10.2:4
f.event.dispatch jquery.min.js?v=1.7.1:3
h.handle.i jquery.min.js?v=1.7.1:3
Drupal.media.formatForm.submit media_wysiwyg.format_form.js?mxeuhr:53
f.event.dispatch jquery.min.js?v=1.7.1:3
h.handle.i jquery.min.js?v=1.7.1:3

Maybe i discovered something. Drupal.settings.media.wysiwyg_allowed_attributes in media_wysiwyg.filter.js is undefined so the create_element() method throws an error.

it somehow depends on the wysiwyg_module. With the wysiwyg module enabled, it is properly set. It is added in media_wysiwyg/wysiwyg_plugins/media.inc.

media_wysiwyg.filter.js in line 73

/**
     * Serializes file information as a url-encoded JSON object and stores it as a
     * data attribute on the html element.
     *
     * @param html (string)
     *    A html element to be used to represent the inserted media element.
     * @param info (object)
     *    A object containing the media file information (fid, view_mode, etc).
     */
    create_element: function (html, info) {
      if ($('<div></div>').append(html).text().length === html.length) {
        // Element is not an html tag. Surround it in a span element
        // so we can pass the file attributes.
        html = '<span>' + html + '</span>';
      }
      var element = $(html);

      // Move attributes from the file info array to the placeholder element.
      if (info.attributes) {
        $.each(Drupal.settings.media.wysiwyg_allowed_attributes, function(i, a) {
          if (info.attributes[a]) {
            element.attr(a, info.attributes[a]);
          }
        });
        delete(info.attributes);
      }

Using patches from #113 I do not see a JS error "Uncaught TypeError: Cannot read property 'length' of undefined" but instead the modal goes to a white screen that says "The requested page "/media/1/format-form?render=media-popup&fields=undefined" could not be found."

Tested this with CKEditor library 4.3_beta_standard and with 4.3.1_standard and got the same result.

Edit: Needed to enable the media_wysiwyg module.

I am now seeing 'Uncaught TypeError: Cannot read property 'length' of undefined' when attempting to submit.

#117 hit the nail on the head. Looking at media/modules/media_wysiwyg/wysiwyg_plugins/media.inc I noticed that media_wysiwyg_include_browser_js() needs to be called in order to get Drupal.media.wysiwyg_allowed_attributes to be set. media_wysiwyg_media_plugin() was the only function that was calling it. This explains why things 'just worked' when WYSIWYG was installed - because media_wysiwyg_media_plugin() is a hook provided by WYSIWYG.

The media_wysiwyg module needs to call media_wysiwyg_include_browser_js() if ckeditor module is present. I also noticed that the file media_wysiwyg.ckeditor.inc was not being included anywhere. So the hook it was provided would not be working, probably. I'm not sure if this is the best possible way to be calling media_wysiwyg_media_plugin() but its something, and working in my tests.

Nice job tracking this down everyone and thanks for the patch @brantwynn!

Tested with version 4.2 and 4.3 of the CKEditor library and everything seems to be functioning. I think that the file inclusion, given the current architecture, should be taken care of by CKEditor. So I've attached rerolls of #113 with modified versions of the fixes from #119.

Will this still work if you apply ckeditor styles(from the custom classes dropdown) to the embedded media?

When I used the CkEditor library + WYSIWYG module + Media 2.x in the past, if I applied a style using the CKEditor dropdown it would cause the media tags to get messed up.

See #2028255: ckeditor styles break media module markup

I didn't notice the media tags were broken until after I reedited the node and examined the media tag markup.

thank you all for the great work!
i managed to apply both patches cleanly to the latest dev versions of ckeditor and media module.

I successfully inserted images into ckeditor 4.3.1.

If i switch to source-code or save the node, the media tag gets replaced by an image tag.

i get an error on the coeditor config page:
Notice: Array to string conversion in ckeditor_load_plugins() (Zeile 342 von ../sites/all/modules/ckeditor/includes/ckeditor.lib.inc).

My setup: ckeditor dev, media dev, ckeditor 4.3.1., tested with safari and firefox.

#120. It works! Wonderful! Thank you everyone!

Hid all old files so that this issue only displays the most updated patches.

i found the cause of the error mentioned in #122.

It is caused by the link it module. The link it module and the ckeditor module both implement the hook_load_plugins(). They both implement the same plugin and the plugin config strings get converted into an array with duplicated entries.

Committed to http://drupalcode.org/project/media.git/commit/c3cda2b. Thanks for the great work!

Has a followup been posted in the CKEditor issue queue for the other patch?

I don't believe so. Who wants to take care of that?

Done. #2159403: Make CKEditor plugin system modular and clean

You guy's are awesome, thank you so much!!
I really hope I can spend some time in contrib & D8 during the upcoming holidays :)

I'm bit confused about versioning and where to apply which patch to get things working ...
could anyone tell me what versions to use
media 7.x-2.x-dev ...
still need to apply patch ?
https://drupal.org/files/issues/media-add-ckeditor-support-1504696-120.p...

ckeditor module 7.x-1.x-dev
still need to apply the patch there
https://drupal.org/files/issues/ckeditor-accomodate-latest-media-changes...

and which ckeditor library ? 4.3.1 full ?
http://download.cksource.com/CKEditor/CKEditor/CKEditor%204.3.1/ckeditor...

@webatelier - here is a gist containing makefile snippets of what you'll currently need: https://gist.github.com/brantwynn/8015543

(The extra modules, Linkit and Media YouTube are optional.)

For those too lazy to click on a link.

1. Latest dev version of File Entity
2. Latest dev version of Media
3. Latest dev of CKEditor module with the patch
4. CKEditor library 4.3.1 (standard or full recommended)

Slight update to brantwynn's comment - it looks like the patch for #1504696: Integration with CKEditor module was committed on 18 Dec.

The other patch RE #2158741: External plugin declarations are redundant. (Linkit, ckeditor_link, ckeditor_swf not working) is currently marked as Needs Review, so you still need to apply that patch... until you see a strike-through line in this sentence.

Re #133 - #2159403: Make CKEditor plugin system modular and clean has not been committed. The issue you linked to is this issue - its the Media side of the changes and it was committed on Dec 18th. As a result, this issue is closed (fixed).

I would recommend the following patches to CKEditor to get it working with the latest Media dev
#2159403: Make CKEditor plugin system modular and clean
#2158741: External plugin declarations are redundant. (Linkit, ckeditor_link, ckeditor_swf not working)

As of this posting, nothing new has been committed to CKEditor since Nov 4, 2013.

I've installed the modules / library combination suggested in #132 with patches suggested in #135 but I'm not seeing a media browser button available when I am editing my full html ckeditor profile.

I've ticked:

[ ] Plugin for inserting images from Drupal media module
[ ] Plugin for inserting Drupal embeded media

I've enabled 'Convert media tags to markup' on my full html input format.

I'm testing on a clean install of 7.25

Did everything in #132. When I click on the "add media" button in ckeditor I just get a blank modal. Empty. Nothing in it. No error messages in the log. Just... blank.

Re #136: "Plugin for inserting images from Drupal media module" should be checked. "Plugin for inserting Drupal embeded media" is deprecated and should not be checked.

#137 and #136 - you should be following up here, #2159403: Make CKEditor plugin system modular and clean; this thread is closed.

#137 (Blank popup): It is probably caused by Media: YouTube. #2178023: Media Browser Pop up Blank upon latest upgrade

Blank popup can also be caused by not having the Media WYSIWYG module enabled, if I recall correctly.