I am using IMCE to add images in the TinyMCE editor. After adding the image, you should be able to click the image, then get the image edit dialog. However, when TinyMCE is re-initialized, the AdvImage button in the toolbar will not edit existing images.

After digging around, I noticed that TinyMCE was adding a class="mceItem" to each image when it was initialized. Adding this class prevents the AdvImage button from acting on said image. I found the javascript that adds this class to all images in the following file:
/wysiwyg/editors/js/tinymce-3.js

  prepareContent: function(content) {
    // Certain content elements need to have additional DOM properties applied
    // to prevent this editor from highlighting an internal button in addition
    // to the button of a Drupal plugin.
    var specialProperties = {
      img: { 'class': 'mceItem' }
    };
    var $content = $('<div>' + content + '</div>'); // No .outerHTML() in jQuery :(
    jQuery.each(specialProperties, function(element, properties) {
      $content.find(element).each(function() {
        for (var property in properties) {
          if (property == 'class') {
            $(this).addClass(properties[property]);
          }
          else {
            $(this).attr(property, properties[property]);
          }
        }
      });
    });
    return $content.html();
  },

By commenting out the line:
img: { 'class': 'mceItem' }

Things work as expected again. Obviously the above code is for a reason, but all it does on my end is break the Image button when editing existing images.

Can this be fixed? Or maybe there is an explanation or way to circumvent my issue without editing this code?

CommentFileSizeAuthor
#18 img_assist.patch1.44 KBsun
#17 wysiwyg-HEAD.prepareContent.patch2.37 KBsun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Title: Unable to Edit Images with AdvImage Button » editor.instance.prepareContent() breaks editor's native markup handling

Better title.

sun’s picture

Oh, and the reason for this is that Drupal plugins (f.e. Image Assist) must be able to insert markup (here: images) into the editor without triggering the editor's own image button when the inserted image is focused/selected.

This is most likely special to Drupal plugins, so a sane approach to solve this issue would be to invoke prepareContent() only for Drupal plugins.

Alan D.’s picture

I'm Just having the same problem, but I'm using ImageField and Easy Image Insert.

As a tmp fix, is it safe just to comment this line out until resolved. The only other non-standard plugin that we are using is Drupal break.

Thanks in advance

smacphail’s picture

Subscribing, had the same problem when I tried moving from v1.1 to 2.x-dev, to avoid this hack (http://drupal.org/node/361973#comment-1389254) to make a teaser break plugin.

Using:
Drupal 6.10
Wysiwyg API 2.x-dev (March 21, 2009)
IMCE 1.2
IMCE Wysiwyg API Bridge 1.0

Thanks!

Owen Barton’s picture

Subscribing

nik78’s picture

Subscribing

meecect’s picture

subscribing

agerson’s picture

subscribing

agerson’s picture

Commenting this line out did not fix the problem for me, in fact, class="mceItem" is still getting added to my images. Strange...

var specialProperties = {
     //AG // img: { 'class': 'mceItem' }
    };
sun’s picture

To solve this issue, we need to move the prepareContent() method into Drupal plugins. That is not easy, because there we need

1) a way to classify and identify the plugin's own markup while attaching/detaching (e.g. class="img_assist" or class="wysiwyg-break") - while the inserted content is usually generated on the server-side (!).

2) a mapping/registry info of the editor's native default class names we need to search for and replace and/or apply accordingly (here: 'mceItem') to prevent highlighting multiple buttons.

agerson’s picture

Any idea if the commenting out of
img: { 'class': 'mceItem' }

Has any negative consequences as a temporary fix?

Adam

agerson’s picture

Sorry, somehow this double posted...

Any idea if the commenting out of
img: { 'class': 'mceItem' }

Has any negative consequences as a temporary fix?

Adam

Alan D.’s picture

Duplicate of ...?
[edit] Thx. Saved a search of the issue queue, thinking that the issue was a duplicate, not the posting. [/edit]

melvix’s picture

Subscribing.

Owen Barton’s picture

@agerson - yes, there are some (minor) negative consequences - for example if you add a teaser break and click on the "break" image that is displayed in the editor it will highlight the image button and let you edit it, even though this is really just "chrome" and not a user editable image (like a photo, for example).

TwoD’s picture

To continue on the track of sun's #10 post:

  1. Each editor uses its own way of tracking certain items as "visual aids" or "placeholders", items which are not to be treated as part of the normal content. That is if they need them at all. As noted before, TinyMCE uses the class mceItem for its placeholder images etc.
  2. Placeholders generated by the editors themselves, or native plugins, will be handled correctly as that is left up to the internals of the editor.
  3. Drupal plugins may also use placeholders, like Teaser Break, or may wish to treat certain objects as "their own", like Image Assist, and prevent the editor from manipulating them in an undesired way. The active editor currently has no idea these "foreign" objects are placeholders so it completely disregards the wishes of the plugin writer; to leave those objects alone.
  4. Since we don't change any editor code, we need to use their internal "markers" from (1) to enforce (3).
  5. Only a Drupal plugin knows if a piece of text, an image or other pieces of content it can insert are supposed to be rendered as actual content or if they are placeholders. Only they will be able to "tag" the content correctly with the least effort. Currently they just spit out tag which to the editor looks like yet another piece of regular content, and it might be...
  6. Only an editor implementation knows how its editor tags placeholders, but it currently doesn't know which objects are placeholders.
  7. We need a consistent way of identifying them, regardless of the editor in use, or which Drupal plugin generated it.
  8. Drupal plugins should never need to know any details about the editors using it to be able to operate through the same interface. This way we can add support for new editors without needing to rewrite existing plugins and to keep them simple. This conflicts with moving prepareContent into the Drupal plugins as they would probably need to handle editor-specific details.
  9. It seems reasonable to assume most placeholders will be tags (img, span, div etc) which can be easily styled to reflect its effect on the surrounding content.
  10. As some editors use css classes to identify these special properties of objects, why can't we create our own editor independent set of these identifiers? They should never be seen by the user anyway since they are only needed on the placeholders. If they use an editor's "view source" feature or simply disable the editor they'll just see the "Image Assist filtertag", "Teaser Break comment" or whatever markup was used to produce the content we need a wysiwyg placeholder for.

If plugin writers could agree on using a [probably very small] set of these identifiers in all their plugins which need placeholders, we could easily implement "translations" for these in the editor implementations.

To give an example/use case:
TinyMCE is the active editor. The user has decided there should be a teaser splitter comment inserted somewhere in the content and clicks the button.

The Teaser Break Drupal plugin steps in, generates a placeholder image and applies the wysiwyg-item class to mark it as such and passes it to the editor implementation layer for insertion.

//break.js
  /**
   * Helper function to return a HTML placeholder.
   */
  _getPlaceholder: function (settings) {
    return '<img src="' + settings.path + '/images/spacer.gif" alt="&lt;--break-&gt;" title="&lt;--break--&gt;" class="wysiwyg-break wysiwyg-item" />';
  }

The editor implementation layer (we really need a shorter name for this hehe) looks for the wysiwyg-item class on the elements to which it applies and translates it into the corresponding editor class.

//tinymce-xxx.js
  insert: function(content) {
    content = this.prepareContent(content);
    tinyMCE.execInstanceCommand(this.field, 'mceInsertContent', false, content);
  }

  prepareContent: function(content) {
    var $content = $('<div>' + content + '</div>'); // No .outerHTML() in jQuery :(
    $('img.wysiwyg-item', $content).each(function () {
      $(this).addClass('mceItem');
    });
    return $content.html();
  },

(A similar procedure happens when content is first inserted upon editor and plugin attachment.)

TinyMCE no longer activates the built-in image button when the placeholder is selected because it now has the mceItem class. The Teaser Break button does activate as it should because the image still has the wysiwyg-break class.
When the editor is detached, the node saved, its content retrieved, or the source is viewed the plugin is also detached and so replaces the image with the <!--break--> comment.

Thoughts?

sun’s picture

Version: 6.x-2.x-dev » 6.x-2.0
Status: Active » Needs review
FileSize
2.37 KB

Very good analysis.

Nah, awesome, actually.

sun’s picture

FileSize
1.44 KB

Companion patch for Image Assist.

sun’s picture

Status: Needs review » Fixed

That said: Works flawlessly.

Thanks for reporting, reviewing, and testing! Committed to all branches.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

sun’s picture

That said: Works flawlessly.

Thanks for reporting, reviewing, and testing! Committed to all branches.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

TwoD’s picture

Version: 6.x-2.0 » 6.x-2.x-dev
Status: Fixed » Active

Wow, that was fast hehe. I'm glad you liked it sun!

TwoD’s picture

Version: 6.x-2.x-dev » 6.x-2.0
Status: Active » Fixed

What? I did not change that...

sun’s picture

Note that I also wanted to mention that this new approach probably requires the definition of special properties for each editor only. The actual processing is (most probably) the same for all editors, so in 3.x, that could be moved into wysiwyg.js - respectively - the new editor prototype we badly need - which would even allow an editor to override it when required.

Status: Fixed » Closed (fixed)

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

Vidarls’s picture

Firstly, the patch works for me aswell. I found out, at last.
But;
It does not work for images in nodes that have been created and then edited with an earlier version of wysiwyg.
So, if you believe that this patch does not work for you, check out if your images have the mceItem class stored as part of the HTML (use firebug when viewing the node normally). Solution is to manually remove the class from the markup without the editor, or to delete and re-insert the image

Feels good to get that of my chest...

matthew petty’s picture

Version: 6.x-2.0 » 6.x-2.x-dev

I think I'm missing something.

I'm using the latest development version, and it's still not working. I tried applying the patch, too, but it didn't help.

mnlund’s picture

Just delete the image and send it to textarea again. The old code is already there. You have to get in the new elements for js to work on.

Update: Got damn, you said it Vidar. Didn't see it before I posted.

boztek’s picture

This is still broken for me for 2.x-dev. It does not always occur, but is reliably reproducible in the following way in this specific order:

1. Create new node; set input format to WYSIWYG format that uses TinyMCE to reveal editor
2. Enter text into TinyMCE, multiple paragraphs.
3. Upload images into Filefield Insert configured CCK Imagefield
4. Select somewhere in middle of multiple paragraphs in TinyMCE then click Send To Text Area in Imagefield

The image is not editable and has the class "mceItem" set which when removed manually allows editing.

In addition to latest filefield_insert and wysiwyg I am using TinyMCE 3.7 and have tested on 3.6 as well with same results.

UPDATE: After two reinstalls of wysiwyg and tinymce the "mceItem" problem is gone so it must have been a cache problem as I only cleared cache on the last attempt. I was creating new nodes and using new images each time though. I still have a problem selecting images in Safari but this is unrelated to the mceItem issue which does indeed seem fixed now.

bigknot33’s picture

I've updated to the lastest 6.x-2.x-dev version, but after inserting an image into a node via the Image button, the popup doesn't appear when I click on the button to re-edit the image.

The class 'mceItem' isn't being applied to the image.

I've tried with new nodes, and I've flushed the cache.

Is there another module I should be updating? Any ideas appreciated.

TwoD’s picture

@bigknot33, if you're using the editor's own "Image" button, that problem is unrelated to this issue. Images inserted using the regular "Image" button should not have the "mceItem" class applied to them. Placeholders with that class are ignored by the regular Image button's code, which is what other image handling modules like Image Assist need. Please open a new issue if your problem does not also appear in the official editor examples.

nedjo’s picture

I opened a new issue, #758576: Teaser break plugin breaks image editing, that may be related. Also included there are sample queries for fixing up legacy content with the mceItem class.

avior’s picture

Hi
I have the same problem ,
this thread look old, but i can not see the patches code inside tinymce-3.js

can someone tell me in what version this is solved ?

dmarr’s picture

I've put together a fiddle illustrating the buggy behavior that I believe prepareContent is responsible for. In versions of jQuery older than 1.8.2 (maybe newer, edge is the only version I've seen that doesn't have this behavior) ,script tags are removed when nesting them inside an html call. For example:

<div>
  <p>yup</p>
  <script>foo();</script>
  <script src="foo.js"></script>
</div>

<script>
var scripts = $('div').html();
console.log($('<div>' + scripts + '</div>').html()); // only logs <p>yup</p>
</script>

My fiddle (look at console w/jQuery edge vs 1.8.2)

mariacha1’s picture

There's a good explanation of what's going on here:

http://stackoverflow.com/a/2699905

I stepped through what is happening explicitly in this JSfiddle:

var thisDiv = $('div');
// thisDiv = Object[div]
var script = thisDiv.html();
// script = "\n  <p>yup</p>\n <script>foo();</script>\n <script src="foo.js"></script>\n"
var newDiv = $('<div>' + script + '</div>');
// newDiv = Object[div, script, script foo.js] <-- see how the scripts are now their own keys in the array!
var script2 = newDiv.html();
// script2 = "\n  <p>yup</p>\n  \n  \n"
console.log(script2);

Because html() only reads the first entry in the object it's called on, it's only looking at the contents of the div in newDiv. I'm having trouble finding a workaround, but that's the issue at least.