Such behavior make impossible to use array of objects as options setting (i.e. the template TinyMCE plugin expect to get list of templates as array of objects). I think code should be altere to collapse only simple array. If scripts detects that array has another array or object as an element, it should pass it "as is".

Also Drupal.wysiwyg.clone should not convert arrays into objects during cloning as this can prevent some plugins (again TinyMCE template plugin is one of them) from working correctly. Update code follows:

Drupal.wysiwyg.clone = function(obj, clone_type) {
  var clone = (clone_type == 'array' ?[]:{});
  for (var i in obj) {
    if (typeof obj[i] == 'object') {
      clone[i] = Drupal.wysiwyg.clone(obj[i]);
    }
    if (typeof obj[i] == 'array') {
      clone[i] = Drupal.wysiwyg.clone(obj[i], 'array');
    }
    else {
      clone[i] = obj[i];
    }
  }
  return clone;
};
CommentFileSizeAuthor
#4 wysiwyg-HEAD.clone_.patch1.91 KBsun

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: TinyMCE options processing code collapses arrays into strings » Drupal.wysiwyg.clone turns arrays into objects
Component: Editor - TinyMCE » Code

Better title. I think.

jfhovinne’s picture

jfhovinne commented

I'm not sure about the real purpose of this function, but if you want to recursively clone any type of object in JS, jQuery.extend() does it very well:

var newObject = jQuery.extend(true, {}, oldObject);

More info at:
http://stackoverflow.com/questions/122102/what-is-the-most-efficent-way-...

HTH
--jf

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Version: 6.x-2.x-dev » 6.x-2.0-alpha1
Assigned: Unassigned » sun
Priority: Normal » Critical
Status: Active » Needs review

Wow. Sometimes, it's very helpful to have some JS masters around ;)

Attached patch completely replaces Drupal.wysiwyg.clone().

This needs to be tested with all editors, their default configurations, with native editor plugins only, but also with Drupal plugins.

If everything still works, this fix will make WYMeditor as well as YUI editor finally possible.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new wysiwyg-HEAD.clone_.patch1.91 KB
jfhovinne’s picture

jfhovinne commented

Happy to help :)

FYI, your patch works fine with latest WYMeditor files (r642), see also #28 in #362137: Add WYMeditor support.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Fixed

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

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

Status: Fixed » Closed (fixed)

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