Problem/Motivation

We had and have quite some trouble with the WYSIWYG integration provided by media.
I think one of the reasons is that people that should / would solely focus on the media handling are bothered with WYSIWYG stuff - which's quite another problem space.
There's enough work in the Media / File Entity world already, and interruptions by WYSIWYG stuff doesn't help getting the work there done.

Proposed resolution

We should begin by spinning-off the WYSIWYG functionality into a submodule #2142571: Spin-off WYSIWYG support in dedicated module / project and then potentially move it into its own project.

Ideas to be discussed

Module structure:
One general module that provides the basic handling including a JS API for WYSIWYG integrations similar to / the same as the one provided by media atm.
Sub-modules for each WYSIWYG integration e.g. a sub-module for integrating with the WYSIWYG module an another one for integrating with the CKEditor module.

Features:

  • Provides by default an enabled wysiwyg file display mode (Ensures only a single html tag (<img />) is used as placedholder).
    The default behaviour could be overwritten by more complex WYSIWYG integrations..
  • The WYSIWYG JS API as currently provided by media - to be extended.
  • The integration into the WYSIWYG module as currently provided by media
  • The integration into the CKEditor module as provided by #1504696: Integration with CKEditor module (second priority).

Open points:

  • Where does the media-token handling belong to? It's somehow a WYSIWYG issue because it's text processing too.
  • Can we provide some kind of testing for the integrations? E.g. with selenium?

Remaining tasks

Bring people together to discuss this, agree on a roadmap and getting it done.

Original Report

Ok, so inline embedding of media via the plugin supplied by the media module. It almost works!

What I'd like to do Is help in a push to get the work finished. Here's my findings and my current set up.

Ok so using media v1 (not the unstable v2) both dev and beta 5, latest stabl and dev of wysiwyg and Drupal 7.8.

Ok, long story short, the problem with the current dev as I see it is that when editing via wysiwyg (CKEditor) the current media-wysiwyg plugin tries to render the complete player (passes rendering to the media module, in turn to the file display formatter). It should be using a placeholder and only applying the formatter on node display, when relivent JS/CSS libs can be added ect)

in more detail...

From what I can tell, as far as adding the inline media elements goes, all works well, if you stick to the file display mode (under media) as generic file there is no problme at all.

The problem is that if you set the display formatter (via media) to anything other than generic file the wysiwyg removes the markup on attach as it fails to render properly, also if anything other than generic file is selected the media embed popup breaks and won't submit on the output settings form.

It's soo close though, adding a file (formatting set to generic for all types) works, as does displaying the file (icon displayed), and clicking edit shows the file icon as expected too!

However once the output formatter gets changed the editor strips the media embed on attach, and trying to add media inline does not work as the submit button fails on the output mode screen.

As a test I tried adding the media via wysiwyg with the output formatters all set to generic file. No problem there, once I'd edited the node I went and changed the output formatter to a video renderer (mediaelement is being used for testing) and the inlien media is rendered correctly and formatted in the chosen formatter! Only problem is this formatter breaks the wysiwyg on re-editing.

My opinion is that when in the editor a placeholder image should be used regardless of output styling (currently it seems to want to try to style the output within the editor, which breaks it). and that the output styling should be applied on the input format level (replace the image with forced attributes with the relevant formatter), the last piece to the puzzle is parsing the media inline code (the [mediaxxx] coding) when the edit button is clicked back into the placeholder with attributes.

It seems that this is not too far from the direction dev is taking. Anyone out there that I can talk to to get involved finishing off this (currently broken) solution??

Please shout! I'm gonna get in touch with the maintainers of Media to see if I can get involved, push this forward, but I need help with defining starting points ect, as this work is pretty deep in Wysiwyg Javascript and drupal file handling core. Any help, either in getting involved or getting this fixed up would be so much appreciated!

Files: 
CommentFileSizeAuthor
#4 media_inline-test.patch1.09 KBidflood

Comments

Thanks so much for working on this issue.
Willing to help, can't provide any design/structure info but I could test any patch.

I can help,but this requires javascript skills:/
i am not an expert when it comes to js unfortunately.but i can hack around if needed

Version:7.x-1.x-dev» 7.x-2.x-dev

I have been struggling with this issue too ( http://drupal.org/node/1261194#comment-5162928 )

Maybe I could help on the javascript side (and php too). I think the comments in #0 is right about the image placeholder, judging by this comment:

    // @TODO: the folks @ ckeditor have told us that there is no way
    // to reliably add wrapper divs via normal HTML.
    // There is some method of adding a "fake element"
    // But until then, we're just going to embed to img.
    // This is pretty hacked for now.

StatusFileSize
new1.09 KB

Here is a buggy patch, but it highlights what pieces are wrong. It's buggy in a sense that it will eventually break things, but I can embed audio with it.

Reading the @todo again, we might want to use the "fake element" method instead of the the placeholder image. This ckeditor feature isn't really well documented but it may be the proper way of doing it.

looking at the code, I saw why the "fake object" solution hasn't been used:

   * Due to problems handling wrapping divs in ckeditor, this is needed.
   *
   * Going forward, if we don't care about supporting other editors
   * we can use the fakeobjects plugin to ckeditor to provide cleaner
   * transparency between what Drupal will output <div class="field..."><img></div>
   * instead of just <img>, for now though, we're going to remove all the stuff surrounding the images.

Category:bug» task

I've tried to document myself about these fake objects, and if the following is correct I would say it's the way to go.

  1. possible to have an edit button. So we could then edit the media and not the placeholder image (select different format, ...)
  2. it's cleaner and will simply display the embed code in plain text for other editors (than ckeditor)
  3. also use most of the time a placeholder image, so displaying the real object in wysiwyg is still not possible

Chipping in, following a direct enquiry from idflood.
I have a manifesto/rant posted at http://drupal.org/sandbox/dman/1075470 my embedder sandbox project. This runs down my evaluation of a dozen previous "media" embedder filters. I'm planning to abstract and re-imagine them all into one pluggable, somewhat consistent approach. Nice big read.

TLDR: No more fake embed tags and markdown!

HTML,XHTML,HTML5 are capable of handling new-ish objects with additional valid attributes. It is actually EASIER to get a WYSIWYG, or inline filter to parse and render valid HTML-style tags and do interesting things to them than it is to rewrite yet another [square-bracket=tokenizer] made-up syntax.

I've worked already on WYSIWYG integration for my playground WYSIWYG project (works), and if I can figure out how to get right-click context menus working under wysiwyg.module+ckeditor I'd be done.
I'll revisit the status of my sandbox and see if a D7 port can go places.

I don't have all the background yet on the media project (though I tried again last week with uneven results) so not 100% sure about the current state of play, but between this, embedder and http://drupal.org/sandbox/dman/1110890 image_ownage and a few other render libraries I haven't released yet ... I'd like to help us all move forward to a nice generic solution.

thanks for the input dman, I might be able to help write the context menu for the CKEditor plugin you will have made, drop me an email ( I recently re-coded an ImageMap plugin with context menus to work in CKEditor 3.4+, inside WYSIWYG, insisde a CMS (inside a taco bell)...

Regarding HTML5 tags though, these are not what I'd class as 'generic' elements as If I jump onto an old (pre HTML5) browser (Opera 10, Firefox 3.0, IE7) or use a file format not supported by the browser (IE MP3 in anything other than IE9+, Chrome 10+) the markup will fail to display.

I mention all this as I currently program full accessibility and compatibility based websites for large companies that have able and disabled users using a wide variety of browsers. As they are government related organizations they have a requirement to conform to accessibility and compatibility. A great example is the NHS which currently have many, many workstations still running IE7 only.

Your thoughts?

I'm not talking about any special features of HTML5 or new tags, and my own work on government web standards and accessibility with Drupal should demonstrate that.

No, my whole big noisy point is that we should use SGML/XML/HTML style extensible markup <tag attr="val" /> and not another [[tag|attr=val]] markdown syntax.

When it comes to rendering, we would still scan, parse and re-render our elements using whatever filter options we like - normal.
When using the WYSIWYG, we *may* need to re-render and swap between representations when switching to view-source and back ... as we currently do anyway, but I think the code will be simpler if transforming html to html than it is transforming something that has to be string matched and tokenized.
And when the code is saved in the database, it is valid, useful, stand-alone code. Not something that will leave [[fake,custom=markdown]] pseudo-tags in the source when you try to export the database.

This is my mission, and I'd cry if I saw media project moving forward with its own made-up markdown, when there is already a well-known syntax that can do everything we want, with a thousand tools already written to work with it - it's called HTML.

We can't use standard HTML in our markup, because of the security risks involved.

"security risks"? Do you know what they are?

Are you saying that something like
<div class="media" fid="117" render="embedded" width="200" height="300" />
... that gets rewritten by the output filter *anyway* is a security risk on your setup but the equivalent in [square brackets] are not?

Sure, we have to be aware of nastiness that people can attempt to insert into style tags and attributes ... but that can be filtered normally anyway. There are a few things to be aware of, but stop and identify them first.

[fake markdown] syntax only became popular because

  • it was less scary for editors in very simple cases, and
  • was mostly compatible (that is - ignored) and safe with both non-and-wysiwyg editors.

Seeing as we are building full WYSIWYG integration anyway we don't have to be limited to a dummies version of HTML for the internal syntax, we can do it properly again. As I demonstrated on my embedder project page - after a while, the dummies version gets *more* complex and harder to write than real markup as it tries to sandwich more settings in.

If we use XML-style markup, the tools - like jquery and xpath and DOM manipulation - work for free. With [square brackets] people are just inventing their own language and need to write their own parsers.

If you have real security concerns, please be specific.

Sorry for dragging this issue a little off-topic and raising this idea here, but it's something I've been working on for years now, and feel strongly that it is very neccessary for any media embedding solution to progress. I'd love to get it working with the media project - if we can abandon the turn-of-the-century-style bbcode and get comfortable with SGML again.

I totally agree with dman. But: Shouldn't we work on a solution so that people like me do not have to dump the Media module once again? Am I currently seriously not able to embed a PDF in WYSIWYG?

I am trying to get this issue resolved for now, but I cannot wait until we figured out a clean approach to this. Except you have already.

The media/js/wysiwyg-media.js is the main file of concern, right?

Here is the issue:

  1. I upload a PDF inside a WYSIWYG (TinyMCE or CKEditor)
  2. I try to select a Format for the file e.g. "Full"
  3. I get an error "imgElement is undefined" in wysiwyg-media.js on Line 195.

Am I wrong or does this file assume that every element we embed hat a image placeholder? Could the maintainer of the WYSIWYG integration tell me how this works and I will try to fix this.

I'm browsing the wysiwyg-media.js and as was mentioned the code expects an image. However this breaks when uploading local audio/video media as (unless you install something like the video module the media won't have a thumbnail). In the previous module Embedded External Media the uploader could select and or upload a thumbnail for the local media (I won't elaborate on that subject as is isn't what this thread is about).

What if, based on the mime type (file extension) add a place holder icon for the file type?

What would be even better is documentation or sample source code for registering a rendering display for a given community module. Like stipulating how to register the JW player module (embedmedia.js or the 20 or so other client-side player/renders) to render both on the view state of an entity and during the edit state in a wysiwyg editor.

I also agree with dman that instead of using markdown that using a img tag with additional attributes could cover the "edit" mode display of a given display renderer.

Quick search (http://commons.wikimedia.org/w/index.php?title=Special%3ASearch&search=m...)

So would automatically adding a mime type icon make the js and input filter work half way?

Or maybe a shorter answer is to just have 5 icons to broadly represent the 5 different file types?

Documentation is really needed since I really had no idea how and where to fix this issue.

The placeholder approach sounds good. I think generalized icons would be enough for now. Lets take the default file types: Application (multipurpose), Audio, Image, Text, Video, Other

Can someone tell what needs to be done?

After toying around with it further it has become obvious that actually we need to specify three states for a style and or rendering display.

  1. Rendering in View mode.
  2. Rendering in Edit mode.
  3. Rendering thumbnail in media browser mode.

Expended requirements:

  1. Provide a default behavior and icon displays for any rendering display that doesn't specify a thumbnail rule set.
  2. Provide an API to define a rendering hook for each state that can be registered for a rendering display.
  3. Provide a default form with the ability to override/extend the form per rendering display for properties and settings to override the default rendering properties.

Thoughts, additions?

update: I have successfully setup JW Player to render video locally. However, the problem is defining the rendering display type. It doesn't work completely but here is the following scenarios where I got it to work.

Video as a Field of a Node (content type)
I created a "Video" content type and added a file field that uses the media browser as the display option (when editing). Browsing and selecting an uploaded view was successful. To make the video selectable in the media browser I had to specify a Content type icon display for the "Preview" style.

Next, I defined the JW Player render mode for the File Field in the Content Type and book it rendered the field as I expected.

Placing a video in the body of a node
Here is where it all falls apart.
The goal would be to place a local video via the media browser into the Wysiwyg editor and have the ability to see a placeholder that you could move around in the body DOM and change properties on.

As mentioned in the previous configuration I had a:

  1. Video type
  2. Multiple File Styles ("preview", "large" being the more relevant).
  3. File rendering styles (Icon Media Type and JW Player)

In order to place a local video I had to specify the Preview file style to use the Mime type icon renderer. Specifying the JW Player renderer for any of the File styles resulted in rendering a tiny JW player in the media browser and the "element" reference totally disappearing in Edit mode.

However, if I use the Mime Type icon rendering while in edit mode I could place video. But when viewing the parent node the icon would render. If (without editing the node again) I change the render display (for the video type and the render type) to JW Player. Then BOOM, the JW player appears and plays the local video.
But as soon as I click "edit" the "element" never renders and is gone from the wysiwyg DOM. :(

Does this make sense and have I described the problem accurately?

Would this module help? http://drupal.org/project/entity_view_mode or http://drupal.org/project/ds

Lukas von Blarer the good news is the module already comes with a few themed icon sets. We just have to figure out how to make the "mime type icon" rendered and make it render for a local video in the media browser and the edit state while allowing a different file renderer on view.

For now I've dodged a bullet on my current project. But I may have time over the next few months to work on a few patches for this.. I won't post anything until I feel it doesn't cause everything to become unstable.

I'll post my research and thoughts to the group and I hope everyone else can confirm/deny and or contribute some thought to figure this out and getting a patch committed.. it is a long time coming.

Till then. happy holidays.

Hey emptyvoid

This is quite much what I experienced. I have never had elements disappearing from the DOM. One problem I had is that it saved the node with the body containing the placeholder image saved in it.

I would love to see progress here. Tell me if I can help somehow.

I can confirm that with media 2.0-unstable3 and mediaelement 1.2 I am unable to add audio files of a certain format (ex: Large) only if i've applied a display to it (ex: MediaElement Audio). The submit button simply doesn't do anything. Should I open a separate issue for this?

I think emptyvoid is right in #17 when he says that the problem is in wysiwyg insertion and that media should have two render modes : one for wysiwyg edit (an image) and one for node view (the real media rendered).

Actually for the moment media can not insert anything in a wysiwyg edit field but an image. It is hardcoded. And it is not simple for a module to say "I want you to insert this image in wysiwyg and this rendered media in node view"

I don't really know how, but media_vimeo and media_youtube can insert an image in wysiwyg and render a vimeo/youtube embeded object in node view. So it is probably possible but how ? It's probably necessary to add some js.

PS. langworthy : there's already an issue about that : #1062948: Issue getting Media, Styles and WYSIWYG working together with MediaElement.js

There are at least two issues preventing this from working.

  • currently the js looks for an img element in the inserted files rendering. In the case of a video, for example, there is not an img element in the code, and this breaks the js. The suggested approach of using the media.preview seems like the right approach for now, as it will insert the rendering that appears in the media browser, which should correspond to the media files preview display mode.
  • The other aspect is the Drupal.setting.tagMap object. This keeps a mapping of filter string (eg format [[something]] > preview HTML. The filter string is always the bit stored in the node body, and then the wysiwyg swap out the filter string for its corresponding HTML snippet. Keeping this in sync is vital for the operation of the editor - if it is out of sync then data can be lost when editing the node.

I am happy to look into this when I find a little time.

Hey nikosnikos, the secret is a input filter.

I've used a module in D6 to emulate the media module: http://drupal.org/project/nodereference_explorer

it uses markdown for the object saved into the node body.

[[node:124;imagecache:block_square;]]

But it includes two renders: one for the WYSIWYG state (using a post-processor client-side/ajax script to replace the object in the DOM of the editor) and another rendering state of the node view (using an input filter)

Perhaps this technique could be used to address the problem we've been talking about?

Marked #1265674: Upload and embed flv file as a duplicate.

Sorry it's taken me a while to chime in on this issue. I've posted #1291518-3: Use Field Formatters for Embedding Options with my thoughts on how we should approach the WYSIWYG embedding logic once and for all and still provide backwards-compatibility support.

I've posted a patch to #1451316: Clean up wysiwyg-media.js that makes a start at fixing some of the issues discussed here. It also paves the way for Dave's proposal in #26. Any feedback would be greatly appreciated.

Possibly relevant workaround was posted on one of those bugs:
http://drupal.org/node/1062948#comment-6165748

Hi,

Just checking if there's any progress on this?

Thanks for all the hard work,
Victoria

I'm also having this problem. Thanks for marking the other issues as duplicate - that's how I found this page. I am happy to review patches here or on related issues.

Title:Inline Media, Display formatters and input filters - Can we get this finalised? (offer to help!)[meta] Improve WYSIWYG integration

Updating the issue details to be more understandable.

I would like to bring some attention to #1795968: Prepare for wysiwyg_load_includes being cached as well.

If anyone is looking for a temporary solution I got this working by creating an image formatter based on the media_youtube one. This is all pretty hacky but works.

This creates an image display called "Video Preview Image" that you need to enable at /admin/structure/file-types/manage/video/file-display and put above "MediaElement Video"

There is a hardcoded image called video_placeholder.png in root of files.

EDIT: module is called "atwork_video", change as desired

<?php
/**
* Implements hook_field_formatter_info().
*
* Copied from media_youtube
*/
function atwork_video_field_formatter_info() {
 
$formatters = array();
 
// add image formatter for wysiwyg placeholder
 
$formatters['video_placeholder_image'] = array(
   
'label' => t('Video Preview Image'),
   
'file types' => array('video'),
   
'field types' => array('file', 'link_field'),
   
'default settings' => array(
     
'image_style' => '',
    ),
   
'view callback' => 'atwork_video_file_formatter_image_view',
   
'settings callback' => 'atwork_video_file_formatter_image_settings',
  );
  return
$formatters;
}
/**
* Implements hook_file_formatter_FORMATTER_view().
*
* Copied from media_youtube
*
* hardcoded image
*/
function atwork_video_file_formatter_image_view($file, $display, $langcode) {
   
$wrapper = file_stream_wrapper_get_instance_by_uri($file->uri);
   
$image_style = $display['settings']['image_style'];
   
$valid_image_styles = image_style_options(FALSE);
   
// @TODO: If autosubmit is removed and we allow view modes that insert
    // images in the WYSIWYG, add file->overrides handling.
   
if (empty($image_style) || !isset($valid_image_styles[$image_style])) {
     
$element = array(
       
'#theme' => 'image',
       
'#path' => 'video_placeholder.png',
       
'#alt' => isset($file->override['attributes']['alt']) ? $file->override['attributes']['alt'] : $file->filename,
      );
    }
    else {
     
$element = array(
       
'#theme' => 'image_style',
       
'#style_name' => $image_style,
       
'#path' => 'video_placeholder.png',
       
'#alt' => isset($file->override['attributes']['alt']) ? $file->override['attributes']['alt'] : $file->filename,
      );
    }
    return
$element;
}
/**
* Implements hook_file_formatter_FORMATTER_settings().
*
* Copied from media_youtube
*
*/
function atwork_video_file_formatter_image_settings($form, &$form_state, $settings) {
 
$element = array();
 
$element['image_style'] = array(
   
'#title' => t('Image style'),
   
'#type' => 'select',
   
'#options' => image_style_options(FALSE),
   
'#default_value' => $settings['image_style'],
   
'#empty_option' => t('None (original image)'),
  );
  return
$element;
}
/**
* Implements hook_file_formatter_info_alter().
*
* Because I don't know what I'm doing and this works
*
*/
function atwork_video_file_formatter_info_alter(&$formatters) {
  if (isset(
$formatters['file_field_video_placeholder_image'])) {
   
$formatters['file_field_video_placeholder_image']['file types'] = array('video');
   
$formatters['file_field_video_placeholder_image']['view callback'] = 'atwork_video_file_formatter_image_view';
   
$formatters['file_field_video_placeholder_image']['settings callback'] = 'atwork_video_file_formatter_image_settings';
  }
}
/**
* Implements hook_file_displays_alter().
*
* This is to get the proper formatter for viewing or editing a node and
* I can't figure out how it works.
*
* This assumes that "Video Preview Image" is above the "MediaElement Video"
* display at /admin/structure/file-types/manage/video/file-display
*
*/
function atwork_video_file_displays_alter(&$displays, $file, $view_mode) {
  if (isset(
$displays['file_field_video_placeholder_image'])
       && isset(
$displays['file_field_mediaelement_video'])
       &&
arg(0) == 'node'
      
&& is_numeric(arg(1))
       && !
arg(2)) {
   
$video_weight = $displays['file_field_mediaelement_video']['weight'];
   
$displays['file_field_mediaelement_video']['weight'] = $displays['file_field_video_placeholder_image']['weight'];
   
$displays['file_field_video_placeholder_image']['weight'] = $video_weight;
  }
}
?>

I'm sorry, I don't understand. Where is this code supposed to go, Sarenc? I am very willing to try out your proposed solution, thank you.

@Nick Lewis in #32: I tried that solution today, personally it didn't work for me at all. Wish it did though, it seems simple enough.

Issue summary:View changes

Added summary of current WYSIWYG issues.

Updated issue summary with parts of the summary created by @das-peter in #2142571: Spin-off WYSIWYG support in dedicated module / project and added a few child issues.

Issue summary:View changes