Problem/Motivation
The JavaScript part to deal with the media token filter depends quite a bit on the WYSIWYG module.
This makes it harder than necessary to integrate proper media token support into other modules (e.g. ckeditor module).
Further the JavaScript to use changed between the major media versions, what makes it even more complex to provide proper media token support for other modules.
Proposed resolution
I propose to split the JavaScript into two parts:
* A Media token library
* A WYSIWYG module integration
At the same time we could streamline the media token library to expose the same functions for Media 1.x and 2.x.
Remaining tasks
reviews needed - some testing was already done in the related issue: #1504696: Integration with CKEditor module
User interface changes
none
API changes
As there isn't an proper API as of now (as far as I know) we should document how to use the media token javascript library.
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#14 | media_filter_1970370_14.patch | 6.14 KB | das-peter |
#12 | ckeditor-3-visual.png | 604.03 KB | saltednut |
#13 | ckeditor-3-actual.png | 58.02 KB | saltednut |
#7 | Screen Shot 2013-08-15 at 3.25.14 PM.png | 84.93 KB | saltednut |
#5 | media_filter_1970370_3.patch | 11.09 KB | das-peter |
Comments
Comment #2
catchmedia.filter.js was hardcoded to 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.
Comment #3
douggreen CreditAttribution: douggreen commentedupdated patch
Comment #5
das-peter CreditAttribution: das-peter commentedFixed paths in patch. Patch was created in the docroot instead in the module folder.
Comment #6
Dave ReidI think this is working, I don't see any regressions in using the WYSIWYG. I'm still nervous about committing something which changes a large chunk of JS without any peer review. If someone else can confirm this is good, that would be appriciated.
Comment #7
saltednutLooks like it is not quite working with CKEditor module and CKEditor 4.1 - I had to apply the patch for CKeditor #1504696: Integration with CKEditor module and also the patch from #5.
I get as far as uploading or picking a media asset from the modal and setting up embed settings, but when submitting, it appears the full markup is not making it into CKEditor. This seems to be an issue with the CKEditor module or CKEDitor 4.1 not supporting the media tags.
I also attempted this with CKEditor library version 3.6 and the WYSIWYG module - in this case, everything works as expected. I don't see any regression and I think it is safe to move forward and follow up with the CKEditor module/CKEditor 4.1/Media 2.x integration in #1504696: Integration with CKEditor module
Comment #8
Dave ReidOk committed #5 to 7.x-2.x. Let's follow-up in the CKEditor issue queue then.
http://drupalcode.org/project/media.git/commit/3cb8d2c
Comment #9
das-peter CreditAttribution: das-peter commentedThank you, thank you so much! Committing that was a very important signal. I'll provide feedback & support in the other issue asap!
Comment #10
das-peter CreditAttribution: das-peter commentedWhile I tried to reproduce the issue mentioned in #7, which I couldn't, I came across another issue.
Follow-up and patch created: #2066859: Removed media are restored on save in WYSIWYG
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commentedThis actually breaks WYSIWYG + CKeditor 3. The Media filter token is not inserted into the tagmap because the attach callback is called while CKeditor is inserting the placeholder HTML.
The output in the saved text field is like this:
This makes it look like it's working, but it's not actually rendering a file entity.
Comment #12
saltednutdouble postComment #13
saltednutYep - I can confirm. THought this was working per visual inspection but did not realize the code snippet was incorrect.
Visually, looks correct:
Oops, code is wrong:
Comment #14
das-peter CreditAttribution: das-peter commentedHere's a patch to fix that (Attention: patch also contains #2066859: Removed media are restored on save in WYSIWYG).
Changes:
Drupal.media.filter.getWysiwygHTML()
returns the wrapped HTML to insert into the WYSIWYG and registers macro and markup. You can do this still by yourself but why should you.... I'll update the ckeditor patch to use this new function asap.wysiwyg-media.js
to the new ones provided byDrupal.media.filter
Tested with:
- WYSIWYG latest dev
- CKEditor 3.6.6.1 (>=4.1 doesn't work as of this #1956778-14: Ckeditor 4.1 ACF)
Comment #15
Anonymous (not verified) CreditAttribution: Anonymous commentedI'm confirming that the patch in #14 fixed 3.6.6.1.
The patch plays with the tag map before inserting the HTML. When the HTML is inserted, CKEditor was firing another event which was causing the "attach" function in the WYSIWYG plugin to run before the tag map was up to date.
Comment #16
Dave ReidCommmitted #14 to 7.x-2.x. Thanks again everyone! http://drupalcode.org/project/media.git/commit/b5e1141
Comment #17
Dave ReidComment #19
ParisLiakos CreditAttribution: ParisLiakos commentedthis commit seems to be the reason for a number of regressions/bugs #2067063: Wysiwyg integration is broken :(