Add WYMeditor support

Nice. Not bad, but needs a bit of work.

Please do not upload compressed files to drupal.org. See http://drupal.org/patch/create for how to create patches that add new files to CVS without commit access.

You probably want to look into the other editor integration files and use of them as a template for the version detection function.

Also, most of the arrays should be on multiple lines (due to their size), and comments need to start with a space and an uppercase letter (and end with a dot/full stop). See http://drupal.org/coding-standards (as well as the special page regarding comments linked there).

There are editors that cause nothing else than headaches. Oh my.

This is what I have so far. However, I'm not sure whether I want to work further on an editor that looks & feels *that alpha*.

Is it that bad? I like the concept of assigning conceptual structure instead of real markup. Is there an alternative?

Yes, its code is a nightmare. It does not seem to have a clean concept/structure, resp. API, to program against. If you look at the attach and detach functions of this patch, you can get a (starting) feeling of this pain.

Urghs, the detach function is indeed ugly. And seeing that their 0.5 release is 7 months late as to their roadmap, and trunk/ didn't have commits since late november, I think it's safe to postpone this for now.
It's a pity though, I could make use of an easy style chooser instead of a full fledged editor.

is this patch still working?

I applied it to wymeditor code and moved it under the wysiwig dir but nothing happened it does not seem to be recognized or registered.

I understand the bad state of wymeditor but really can't let my users use a wysiwig editor.

I don't understand why such editors even exist since all they do is wreck the content and sometime the website.

I work on a small publishing website and it is NOT an option to have bold and italics and such mangled in the html, or all the document value becomes nil, we will never be able to reuse them.

Some voice on the corner on my mind wonder why so many brilliant people as you gather to federate all this editor who will only bring crap inside our documents (and as advanced and stunning piece of code as they are, none of them works correctly on moderately complex documents, html was simply never meant to be an intermediate langage for interactive presentation).
A simple editor that would allow us to add wrapper and classes to these wrappers tied to css code to give a moderate feedback on the final document (maybe with a live or not live preview) would be much simpler to use, explain, maintain and produce much cleaner documents than any wysiwig editor.
I see no case for wysiwig editor besides false familiarity with "word" and apparent prettiness and only tons of drawbacks beyond that ...

Am I the only one with these feelings ?

All that to say that in my situation wymeditor is all I have to make do with.... :(
(http://wmd-editor.com/ wmd does not cut it because he is not integrated to drupal and does not allow to assign classes to containers)

thanks a lot for you time and sorry for the rambling, the editor situation is so frustrating :)

Unless WYMeditor improves, Wysiwyg API won't support it.

Hi,

Attached is a patch based on the latest jWYSIWYG stable integration files.
It was basically a search-and-replace task (and took me less than one hour to write), so it surely isn't very advanced, but anyway works fine for me (even with multiple instances); it edits and saves XHTML.
I guess it follows the Drupal guidelines - since it's almost the same code as sun wrote for jWYSIWYG - but I'm newbie here, so sorry if it doesn't.

Could you please review it?

Note: in order to test the editor:

• download WYMeditor at http://www.wymeditor.org/download/
• extract the package in a temp folder
• copy the resulting wymeditor/wymeditor folder (containing the jquery.wymeditor.js file) in the modules/wysiwyg folder

Note 2, for sun: I didn't touch the original CVS IDs, so you can easily find out which file versions it's based on.

BTW if you need more info about WYMeditor's architecture, please feel free to mail me at jf dot hovinne at wymeditor dot org, I'll be happy to help.

Thanks.

Here is an improved version of the patch, which adds the minified WYMeditor JS file support and the settings callback.

Note: the JS minification has been added lately in WYMeditor trunk, so you'll need the latest WYMeditor build:
http://files.wymeditor.org/wymeditor/trunk/build/build/wymeditor.tar.gz

FYI I'm also working on WYMeditor integration with the Image Assist (img_assist) module.

Until this patch is accepted, I'll upload further versions to WYMeditor's repository:
svn co svn://svn.wymeditor.org/wymeditor/trunk/src/apps/drupal/

Repo browser:
http://files.wymeditor.org/wymeditor/trunk/src/apps/drupal/

Just want to let you know that this is very high on my review list. I'll get back to you ASAP. Based on what I can see in that repo, the code looks very promising.

One (minor) remark: The language is (currently) configurable per wysiwyg profile, so we should use $config['language'] (if defined) instead of the global language variable. This will probably change very soon to auto-detect and/or use the global language instead, but for now, I'd prefer to have all editors use the same language configuration.

Oh, and Drupal.wysiwyg.editor.detach.wymeditor() can be invoked in 2 ways: Either with params[.field] or without params.field, which means to detach all editors on the page (and not just one). You might want to have a look into tinymce-3.js or most other editor implementations for examples.

I have downloaded the repository, followed the instructions in the readme and no matter what I do I still get the following error:

Error: jQuery(jQuery.grep(jQuery("script"), function (a) {return a.src && a.src.match(/jquery\.wymeditor(\.pack|\.min)?\.js(\?.*)?$/);})).attr("src") is undefined

I am using Drupal 6.10 and the following modules:
jQuery Update 6.x-2.x-dev (2009-Mar-21)
Wysiwyg API 6.x-2.x-dev (2009-Apr-01)

Am I missing something?

I really look forward to using this editor ;).

@sun: thanks for the feedback. I've updated the files.
I'm not sure about the new detach(), since I don't know where to test the 'without params.field' scenario. Anyway I guess it should work.
See http://trac.wymeditor.org/trac/changeset/598

@realazthat: WYMeditor computes some paths for its internal use, and it seems that the jQuery Update module sets jquery.packed.js as src, instead of jquery.pack.js. I've updated the compute functions, so it should work now (download the new WYMeditor build of course).
BTW, I'd recommend configuring the jQuery update module to use the minified version.

1) For FCKeditor and others, we are doing:

    if ($config['css_setting'] == 'theme') {
      $settings['EditorAreaCSS'] = implode(',', wysiwyg_get_css());
    }

to pass a list of CSS files to load to the editor, because style.css does not really cover all possible cases (Drupal supports sub-themes that mainly consist of an alternative CSS file, but may load additional styles from the "parent" theme). Does WYMeditor support multiple stylesheets?

2) Doesn't WYMeditor support different themes? (Fixed, see below)

3) Doesn't WYMeditor support plugins and buttons?

Now to the actual testing:

4) jquery.wymeditor.min.js is not contained in the downloaded 0.5-b2 package. I guess that jquery.wymeditor.pack.js should be the default then.

5) The 'compact' skin is similarly not contained in 0.5-b2. Implemented the editor themes hook to return actual themes.

6) When toggling ("enable/disable rich-text") multiple (WYM)editors on the same page multiple times, I get two (i.e. a duplicate) instances for the same textarea.

Aside from these points, the rest seems to be working properly.

7) Would it be possible to move the comments on constants in jQuery.extend(WYMeditor...) next to or above the actual constant definitions to make the version parsing easier in WYMeditor 0.6? Alternatively, the JSDoc syntax could be used:

  /**
   * @name VERSION
   * Defines WYMeditor version.
   */
  VERSION: '0.5-b2',

Or, completely alternatively, the version could be stated in the very first lines resp. JSDoc of the file itself like many other libraries do:

/**
 * WYMeditor 0.5-b2
 *
 * What you see is What You Mean web-based editor
 * Copyright (c) 2008 Jean-Francois Hovinne, http://www.wymeditor.org/
 ...
 * (or alternatively:)
 * @version 0.5-b2
 */

Attached is a patch with the updated files. I reworked/renamed some other things, so please base further patches on these files.

Proper version.

1) No, only 1 stylesheet is supported ATM, but it won't be very hard to support multiple stylesheets if needed.
2) Yes. The 'compact' skin has been specifically added for Drupal, so I'd suggest to set it as the default one.
3) Plugins: the plugin system is quite basic for the moment, so I don't think it's worth the effort. BTW, the plugin architecture will be rewritten in 0.6.
Buttons: yes, using the toolsItems option. I'll add the hook in wymeditor.inc ASAP.
4) 0.5-b3 will be released very soon and will contain the minified version and the 'compact' skin.
5) see 4)
6) I see. Sometimes the editor isn't the immediate next element of the textarea, so $field.next() doesn't always return it. Should be fixed by r603.
7) No problem. See r602.

What's the status here? Any updates? I see in http://files.wymeditor.org/wymeditor/trunk/src/wymeditor/jquery.wymedito... that the @version was added.

    var $field = $('#' + params.field);
    var index = $field.data(WYMeditor.WYM_INDEX);
    if (typeof index != 'undefined') {
      var editor = WYMeditor.INSTANCES[index];
      editor.update();
      $(editor._box).remove();
      delete editor;
    }

Can we replace var editor with var instance, please? We are not removing the editor, but one instance of the editor.

If you want to allow to configure editor buttons/plugins for wysiwyg profiles using WYMeditor, this patch would have to implement the _plugins() hook. However, we can defer that to a separate issue, if you want.

If the 'compact' theme was specifically designed for Drupal, then I'd be ok with using it by default (i.e. listing it as first theme in the array, followed by 'default' and the other themes). Currently, Wysiwyg API does not allow to select the editor's theme.

To get this patch in, http://www.wymeditor.org/download/ needs to allow users to download a version of WYMeditor that is compatible with this code.

It would be nice if you could attach the final files here, because I find it quite hard to get (download) the actual and proper/current files from trac.

Of course, I wanted to mention that with the new @version at the top of jquery.wymeditor.js, the version parsing in wymeditor.inc can be drastically simplified.

1) 'editor': replaced by 'instance', see r609.
2) Yes, it'd be better to create a separate issue for buttons/plugins support.
3) 'compact' is the default theme now, see r610.
4) About the download: we'll release 0.5-b3 this week, with the needed updates.

Instead of getting the files from the Trac, you can download a nightly build, available at http://files.wymeditor.org/wymeditor/trunk/build/build/wymeditor.tar.gz

Note: I've added support for the block_formats setting, see r607, and simplified the version parsing, see r608.

Meh. Sorry for the late response. I looked into trac for the integration files a few times, but just found out that I was always looking at Rev. 598 there... bah.

function wysiwyg_wymeditor_version($editor) {
  $script = wysiwyg_get_path('wymeditor/wymeditor') . '/jquery.wymeditor.js';

Instead of wysiwyg_get_path() we want to use $editor['library path'] here.

    $containers = array(
      'p' => 'Paragraph',
      'h1' => 'Heading_1',
      'h2' => 'Heading_2',
      'h3' => 'Heading_3',
      'h4' => 'Heading_4',
      'h5' => 'Heading_5',
      'h6' => 'Heading_6',
      'pre' => 'Preformatted',
      'blockquote' => 'Blockquote',
      'th' => 'Table_Header',
    );

Shouldn't these values be wrapped in t() to allow translation via Drupal's localization system?

    foreach ($block_formats as $block_format) {
      if (isset($containers[$block_format])) {
        $container = array(
          'name' => strtoupper($block_format),
          'title' => $containers[$block_format],
          'css' => 'wym_containers_' . $block_format,
        );
        $containersItems[] = $container;
      }
    }

We only use camelCase variables for classes in Drupal, should be $container_items. That variable needs to be initialized here first or users will get PHP notices.

That said, I wonder whether the following wouldn't be simpler?

    $containers = array(
      'p' => array('name' => 'P', 'title' => t('Paragraph'), 'css' => 'wym_containers_p'),
      'h1' => array('name' => 'H1', 'title' => t('Heading 1'), 'css' => 'wym_containers_h1'),
      'h2' => array('name' => 'H2', 'title' => t('Heading 2'), 'css' => 'wym_containers_h2'),
      'h3' => array('name' => 'H3', 'title' => t('Heading 3'), 'css' => 'wym_containers_h3'),
      'h4' => array('name' => 'H4', 'title' => t('Heading 4'), 'css' => 'wym_containers_h4'),
      'h5' => array('name' => 'H5', 'title' => t('Heading 5'), 'css' => 'wym_containers_h5'),
      'h6' => array('name' => 'H6', 'title' => t('Heading 6'), 'css' => 'wym_containers_h6'),
      'pre' => array('name' => 'PRE', 'title' => t('Preformatted'), 'css' => 'wym_containers_pre'),
      'blockquote' => array('name' => 'BLOCKQUOTE', 'title' => t('Blockquote'), 'css' => 'wym_containers_blockquote'),
      'th' => array('name' => 'TH', 'title' => t('Table header'), 'css' => 'wym_containers_th'),
    );
    $block_formats = array_flip(explode(',', $config['block_formats']));
    
    foreach ($containers as $container => $format) {
      if (!isset($block_formats[$container])) {
        unset($containers[$container]);
      }
    }
    $settings['containersItems'] = $containers;

Offtopic: Why does WYMeditor not calculate/use that 'css' value by default?

    $settings['containersItems'] = json_encode($containersItems);

That, I don't understand. The entire returned $settings are passed as JSON encoded string already, so why an extra json_encode() via PHP here?

    if ($config['css_setting'] == 'theme') {
      $css = path_to_theme() .'/style.css';

This should use wysiwyg_get_css() instead. As long as WYMeditor does not support multiple files, just use the first returned array element (being most likely the same as the filepath assigned here, but wysiwyg_get_css() will soon also handle proper initialization of the theme when required). A comment should clarify that WYMeditor does not support multiple files currently (so people don't waste time in trying/patching).

Drupal.wysiwyg.editor.attach.wymeditor = function(context, params, settings) {
  $('#' + params.field).wymeditor(eval(settings));

Uh oh - eval is evil. Why did you add it? (The revision log contains no explanation.) We avoid eval() at all costs.

  if (typeof params != 'undefined') {
    var $field = $('#' + params.field);
    var index = $field.data(WYMeditor.WYM_INDEX);
    if (typeof index != 'undefined') {
      var instance = WYMeditor.INSTANCES[index];
      instance.update();
      $(instance._box).remove();
      delete instance;
    }
    $field.show();
  }
  else {
    jQuery.each(WYMeditor.INSTANCES, function() {
      this.update();
      $(this._box).remove();
      $(this._element).show();
      delete this;
    });
  }

Looks much better now. Is there are reason why you do not use the internal element reference via $(instance._element).show() in the first condition? If there is one, a comment should clarify why.

All in all, we're very close to completion though. Note, however, that I did not test the latest code.

Link to latest files: http://trac.wymeditor.org/trac/browser/trunk/src/apps/drupal/wysiwyg/edi...

@jfh: Any updates? I think we are almost done! :)

@sun: Yes! I've been focusing on making a release, and many things had to be fixed before.
So rc1 (instead of beta3, we're making a feature freeze) is available at the download page - http://www.wymeditor.org/download/

I hope to find some time this week to work on the integration files, based on your comment #21.
I'll keep you updated.

@Domsou: glad to see you here.
About the WYMeditor module: I guess fall_0ut doesn't maintain it anymore.
IMHO it's preferrable to focus on the Wysiwyg API integration.
And BTW any help would be greatly appreciated ;)

• About #21:
  1. wysiwyg_get_path() replaced, see r638.
  2. WYMeditor uses its own localization system (23 languages are currently supported).
     The 'title' member here is perhaps a bit confusing, since it's rather a placeholder that will be replaced by the correct string, e.g. 'Heading_1' becomes 'Titre 1' in French.
     We can add an option in WYMeditor to override this mechanism, but it won't be available very soon, so I'd suggest to stick with this for the moment, if possible.
     See r639 for camelCase.
     About the 'css' member: we prefer to let the user define its own class names, if needed.
  3. In Drupal.wysiwyg.editor.attach.wymeditor(), settings.containersItems isn't an instance of a JS array, and WYMeditor needs one. That's why the json_encode() and eval() have been added, but any suggestion to avoid this is welcome.
  4. wysiwyg_get_css() used instead of path_to_theme(), see r640.
  5. I simply forgot it, sorry. See r641.
• About Wysiwyg 6.x-2.0, I don't see the WYMeditor files in the dev snapshot. What are your plans about this?
• About the WYMeditor module itself, it seems it's been abandoned by the maintainer, who doesn't reply to my emails. Is there any way to (at least) update the module page to point users to the Wysiwyg module?

Thanks.

Awesome, jfh!

I will look into that eval() issue you mentioned. I don't really understand what you mean with "instance of a JS array" though - perhaps the issue raised in #470928: Drupal.wysiwyg.clone turns arrays into objects?

I guess it's the issue here.

Replacing Drupal.wysiwyg.clone() code by:

Drupal.wysiwyg.clone = function(obj) {
  return $.extend(true, {}, obj);
};

seems to fix the issue, and we don't need eval anymore.

Great. I've applied your patch in #470928, and WYMeditor works like a charm.
See r642 for the updated wymeditor.inc and wymeditor.js.

Yay!

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.

Ugh. I should have mentioned that all JavaScript breaks when Drupal's JavaScript aggregation is enabled.

WYMeditor tries to determine its basepath, and additionally a "wympath" by walking through the scripts loaded on the page.

Adding one additional setting for the path (whichever is required) is not an issue. However, why does it need two paths? It also won't find jquery.js...

Additionally, Drupal supports also CSS aggregation, so WYMeditor's auto-discovery of its stylesheet won't work with that as well...

I am very glad I found this thread yesterday, while trying to integrate WYMeditor (0.5-rc1) to Wysiwyg API (latest from cvs). It seems all works 4 U.
Very good job you've done with all these pathces, thank you, sun & jfh =]. While this seems to be current topic, I wanted to report one mystic bug.

When I create content and select WYMeditor as Input format it doesn't show up at all and throws javascript error:

Error: $("#" + params.field).wymeditor is not a function
Source File: http://localhost/sites/all/modules/wysiwyg/editors/js/wymeditor.js?c
Line: 7

and afterwards you can not switch to no other Input formats, since it throws an error again (while trying to detach WYMeditor):

Error: WYMeditor is not defined
Source File: http://localhost/sites/all/modules/wysiwyg/editors/js/wymeditor.js?c
Line: 16

By the way, since I am just a few weeks into Drupal, just curious, what's the use of that ?c

So what I eventually did was downloaded all those other funny editor alternatives and set them up with Wysiwyg API to investigate.
And here comes the point - if anyone could explain me why it is so, I'd be happy to hear - packed js versions in Firefox don't accept arguments.

http://localhost/sites/all/libraries/wymeditor/wymeditor/jquery.wymedito... - opens up well (Mozilla Firefox / Internet Explorer 6 / Google Chrome)
http://localhost/sites/all/libraries/wymeditor/wymeditor/jquery.wymedito... - page could not be found (but just in Firefox - using 3.0.10 version)

Could this be because of Firefox version or maybe of Firebug addon alone I am using? (none other editors I tested are using packed js versions)

Anyway, while I didn't notice at once that it's kind of browser compatability issue, I've found a work around by using min.js instead of pack.js

So to force Wysiwyg API to use min, navigate to sites\all\modules\wysiwyg\editors\wymeditor.inc and in wysiwyg_wymeditor_editor() function change the order of $editor['wymeditor']['libraries'] placing 'min' element above 'pack' (to be exact cut 4 lines starting from 21 and paste them before 'src' => line).

@auksas: That sounds like you are *not* using the latest development snapshot - which uses the minified library variant by default already.

Hmm, not sure. The packed one was the first item.
See fix r643.

To avoid compute() issues while using JS aggregation, I've added the basePath, wymPath and jQueryPath settings, see r644 and r645.
@sun: About r645, I couldn't find the correct API calls to get the actual editor's JS file path and jQuery's path. Could you please correct this?

Anyway I think it fixes the issue described in your comment #30. I've tested the JS and CSS aggregation, and WYMeditor works fine.

FYI the editor needs these paths to dynamically load some files (language, skin, ...).
The skin path is calculated using the base path, so it won't be an issue here.

OK, I just saw that the files are now available on the CVS (and in the dev snapshot). Great, it'll be easier to submit patches :)

@jfh: Just wanted to mention that ;) Can you create a patch against CVS or shall I diff your latest changes?

Thanks! Committed attached patch 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.

@jfh: It would be good though, if (at least) that second @todo could be fixed in WYMeditor (perhaps using window.opener.jQuery or similar). Definitely a separate issue.