I am working on getting the Autosave module to work with wysiwyg module rather than being tied to a specific editor (D5 versiononly works with Tiny 2.0). Jide has been helping me with the wysiwyg api to figure things out and he added a patch #613204: Add Drupal.wysiwyg.editor.instance[id].getContent() for adding a getContent method. This really helps my cause.

But, in a similar vain it would be useful if there were 1 or both of a removeContent or replaceContent methods.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TwoD’s picture

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

Good suggestions! I did see that other issue, just haven't gotten around to reply yet. We're planning on building a proper API for Wysiwyg 3.x (first for 7.x-3.x then backport) and methods like these would indeed be somehting to consider when building it.

jide’s picture

Hey TwoD,

I'm also in need for such API functions, e.g. for the Wysiwyg Cleaner module. I'm willing to help for this. Would it be useful if I continue to submit patches for this ?

jide’s picture

Status: Active » Needs review
FileSize
3.09 KB

Here is a patch for 6.x-2.x-dev which adds two methods : getContent and setContent. It supports TinyMCE 3, FCKEditor and CKEditor.

Sorry I don't have the time to set up a 7.x-3.x version right now but I think it is pretty straightforward to adapt.

I mark #613204 as a duplicate since this issue overlaps it.

liquidcms’s picture

thanks, trying it out now

btw, i think patch files are supposed to be relative to the module folder, not the modules folder. I have been editing them to fix this.

liquidcms’s picture

and i guess this includes patch-4 you provided the other day?

jide’s picture

Yes, it does.

jide’s picture

Comment removed because it was stupid =)

liquidcms’s picture

tested with new rel of Autosave module and works great. So far only tested with Tiny 3.0, will test with FCK later this week.

Autosave project page outlines need for dev version of wysiwyg and that it requires this patch; hopefully this will be committed soon and new rel made for WYSIWYG.

thanks again for you help with this jide.

TwoD’s picture

+++ wysiwyg/editors/js/fckeditor-2.6.js	2009-10-26 20:55:22.000000000 +0100
@@ -151,6 +151,16 @@ Drupal.wysiwyg.editor.instance.fckeditor
+  },
 
+  getContent: function() {
+    var instance = FCKeditorAPI.GetInstance(this.field);
+    return instance.GetData();
+  },
+  
+  setContent: function(content) {
+    var instance = FCKeditorAPI.GetInstance(this.field);
+    instance.SetHTML(content);
+  }
+  
+};

Empty lines should be completely empty.

+++ wysiwyg/editors/js/none.js	2009-10-26 21:13:10.000000000 +0100
@@ -66,5 +66,13 @@ Drupal.wysiwyg.editor.instance.none = {
+  },
+  getContent: function() {
+  	var editor = document.getElementById(this.field);
+    return editor.value;
+  },
+  setContent: function(content) {
+  	var editor = document.getElementById(this.field);
+    editor.value = content;

Indents on the 'var ...' lines have tabs.

+++ wysiwyg/editors/js/tinymce-3.js	2009-10-26 20:52:31.000000000 +0100
@@ -11,6 +11,8 @@
 Drupal.wysiwyg.editor.init.tinymce = function(settings) {
+//console.log(settings);
+//console.log(Drupal.settings.wysiwyg.plugins);
   // @see #454992: drupal_get_js() must not use 'q' as query string.

Please remove these comments.

+++ wysiwyg/editors/js/tinymce-3.js	2009-10-26 20:52:31.000000000 +0100
@@ -209,6 +211,16 @@ Drupal.wysiwyg.editor.instance.tinymce =
+  getContent: function() {
+    var editor = tinyMCE.get(this.field);
+    return editor.getContent();
+  },
+  
+  setContent: function(content) {
+    content = this.prepareContent(content);
+    tinyMCE.execInstanceCommand(this.field, 'mceSetContent', false, content);
+  }
+  

We need to check if TinyMCE is in fullscreen mode. See #606952: Inserting content in fullscreen TinyMCE. This patch could on the other hand be committed first, and then we adapt the other patch. Probably makes more sense than partially fixing this in two patches, so let's do it like that.

And yes, patches are best made relative to the module folder.
As long as the patch only touches the editor implementation files, it should apply fine to 7.x-3.x-dev for now.
Note that I have not yet tested the functionality provided by the patch, only reviewed for visible errors.

This review is powered by Dreditor.

jide’s picture

Status: Needs review » Needs work

Ouch, I missed (again) some debug code. And some tabs. I promise I'll be more careful next time.
I'll clean this up and make it relative to the module folder.

About 7.x-3.x-dev, I grabbed the last CVS version and could not make it work on HEAD version of Drupal 7 CVS. If you could provide some help on how to set up an instance, it would be helpful.

Any more methods we could add to this and that might be useful ?

jide’s picture

Status: Needs work » Needs review
FileSize
2.57 KB

A cleaner patch.

liquidcms’s picture

not a patch expert but they still look odd.

this:

--- ./editors/js/ckeditor-3.0.js 2009-10-23 04:11:02.000000000 +0200
+++ ../wysiwyg/editors/js/ckeditor-3.0.js 2009-10-27 21:00:04.000000000 +0100

should likely just be this:

--- editors/js/ckeditor-3.0.js 2009-10-23 04:11:02.000000000 +0200
+++ editors/js/ckeditor-3.0.js 2009-10-27 21:00:04.000000000 +0100

jurgenhaas’s picture

Patch from #11 needs the modification from #12 and then it works really nicely. Please include that in HEAD.

sun’s picture

Status: Needs review » Needs work
jide’s picture

Status: Needs work » Needs review
FileSize
3.18 KB

Here is the patch created from CVS this time.

sun’s picture

This is a nice patch, but we will defer it to the total revamp for 3.x. Read Wysiwyg 3.x - Wrapping it all up for more information.

liquidcms’s picture

not sure when W 3.x is going to be out.. but any chance of getting this patch committed for D6.2

sinasalek’s picture

Same question here, it's a very simple patch don't know why i should wait for 3.x.
We can re-roll and test it if that's the problem?

whatdoesitwant’s picture

(Although patches are set by default against the current head, this comment deals with wysiwyg 6.x-2.x-dev).

I lost three hours of tutorial write up today because I had forgotten to reapply this patch after the last security update. I had autosave running in stealth and hadn't stopped to notice it not working (I must confess that I don't run firebug within the production environment too often). So what I've decided to do to prevent this from happening in the future is to put the patch into a make file so sun can stay focused on version 3 and I can turn my frown upside down.

The relevant parts of the make file look like this:

    core = 6.x

   ; Wysiwyg
     projects[wysiwyg][subdir] = "content_management"
     projects[wysiwyg][version] = "2.x-dev"
     projects[wysiwyg][patch][] = "http://drupal.org/files/issues/614146_wysiwyg_api_3.patch"
     ; This patch enables the getContent() function needed for Autosave 2.5

I advise you to put the patch information line with the project information or put a referrence to the patch line in a comment behind the project line. After all the main goal of this approach is to prevent the situation I just described by loosing track of the patch again.

The drush_make readme has really good information on the commands that you can use.
For a gentle introduction to the practical application of drush make, check out Young Hahn's article.

Edit: The line with the subdirectory directive is optional, I just like to cluster my projects into subdirectories so I can switch stuff off and on depending on the site or whatever I'm theming.

liquidcms’s picture

started hearing about "make" files in SF this week at Drupalcon.. had no idea what people were talking about .. PHP/Drupal aren't compiled.

from you post i see it is a Drush thing.. sadly.. majority of site devs needing to add Autosave won't know anything about Drush.

David Stosik’s picture

Any news on this issue ? Will we really wait for WYSIWYG 3.x to be able to autosave WYSIWYG forms ?

Jim Kirkpatrick’s picture

I'm keen to know what's going on too... Is this getting into the D6 version soon?

achton’s picture

Subscribing.

TwoD’s picture

If we can get support for more (preferably all) editors and for all their version-specific implementations, I'm all for comitting this to 2.x as 3.x has been delayed longer than.we thought.

These are pretty small (code-wise) but important additions to the API, which we also won't need to change much for 3.x

Hiroaki’s picture

Need this function pretty bad.
I'm subscribing. Hope someone has time to add the change for other editors too. my favorite editor is nicedit.

aklump’s picture

FileSize
2.98 KB

I ran the patch from #15 on wysiwyg dev version downloaded today and ran into an error for the tinymce-3.js portion. I believe the changes I made to the patch (and subsequently attached to this comment) may help someone, as it seems to have fixed the error I was receiving for tinymce-3.js. Hope it helps.

slashrsm’s picture

subscribe

rismondo’s picture

subscribe

jide’s picture

Status: Needs review » Needs work

@TwoD : I'll try to work on this to add support for remaining editors as well. In this case, would you commit it ?

TwoD’s picture

Yes, as long as it doesn't change existing behaviors, only adds new ones, I can committ it to 2.x branches.
Also, for the editors which support them, make sure cross-editor plugins' 'attach()' methods are called when setContent() runs and their 'detach()' methods are called on getContent(). The editors that have events/callbacks for those operations should take care of it on their own.

Renee S’s picture

Was this committed?

TwoD’s picture

No, the issue would be marked "fixed" or "patch to be ported" if it was committed to one or more of the relevant branches.
The patch needs implementations of the additional methods for all the supported editors before we can commit it, or we'll just introduce a half-done feature. I'm up to my ears in work and this is is pretty far down on my todos at the moment. Anyone's welcome to complete the patch if they have time, it shouldn't be that hard even if you're not familiar with the internals of the module or the editors.

fearlsgroove’s picture

Status: Needs work » Needs review
FileSize
8.83 KB

Fairly complete patch attached. Some libraries have no or limited support for insert/get/set equivalents, so I've defined functions with JS alerts there, which seemed a little less "WTF" to me than failing silently or allowing js exceptions to happen.

Also didn't do old versions like tiny 2.x, and YUI insert seems a little flaky -- not sure there's anything I can do bout that.

fearlsgroove’s picture

Bump, and a D6 version as well.

TwoD’s picture

I'm going to review/comment this ASAP but it's been a long night at work and I need some sleep now so I'll just post an updated patch.
I have been working on filling in the gaps in patch #33 and compensating for things like major API differences between Whizzywig versions so this patch has grown a bit. ;) I believe insert()/setContent()/getContent() is working for all currently supported editors now, but it needs some more cross browser testing to be sure.

sun’s picture

+++ editors/js/nicedit.js
@@ -55,7 +55,7 @@ Drupal.wysiwyg.editor.detach.nicedit = function(context, params) {
-  insert: function (content) {
+  insert: function(content) {

Anonymous function definitions should have a space between function and the parenthesis. See http://drupal.org/node/172169

Fixed that in attached patch.

It looks like we could save tons of code and performance by setting up and permanently having instance properties for

  1. the native editor instance of the editor instance (e.g., instance.native = CKEDITOR.instances[this.field];)
  2. this.field and other Wysiwyg params, which I'm not sure of whether they are consistently available on all instance objects currently (e.g., instance = $.extend(instance, params))
  3. perhaps more?

Happy to discuss this in a follow-up issue though, if you think it's safe to do this in a follow-up.

That said, my goal and intention is to make faster progress on various larger issues. I'll add details about this in #735346: Wysiwyg battle plan in a moment. Short version is that it's likely we will commit this patch very soon.

TwoD’s picture

Status: Needs review » Needs work

Thanks for the cleanup.
I agree with all your points there. Most of these variables could be set up in attach(), some in init() and some perhaps even before that.
I think it would make more sense to make those changes when turning our JavaScript integrations into a single object [think class] per editor library. Then we can create on-demand instances of these container objects for each editor instance to hold configuration data that would stay the same when toggling the editor. That would also help us decouple things from Drupal.settings.wysiwyg and help with lazy-loading/AJAX workflows but it'd make this patch's focus too wide.

Oh that reminds me: The None editor doesn't get .field etc set when an editor is being disabled (works fine if no editor is enabled by default). We need to make sure those properties are always set, even when None gets "lazy-inited", or the new methods added here will cause errors when looking for them.

+++ b/editors/js/none.jsundefined
@@ -65,6 +65,14 @@ Drupal.wysiwyg.editor.instance.none = {
+  setContent: function (content) {
+    $('#' + this.field).val(content);
+  },
+
+  getContent: function () {
+    return $('#' + this.field).val();

this.field is not guaranteed to be set if None was not attached from the beginning.

+++ b/editors/js/openwysiwyg.jsundefined
@@ -65,4 +65,56 @@ Drupal.wysiwyg.editor.detach.openwysiwyg = function(context, params) {
+  insert: function (content) {
+    alert(Drupal.t('Sorry, inserting content is not supported on openWYSIWYG.'));
+  },

Must not forget this one. Critical for other modules using our API.

+++ b/editors/js/whizzywig.jsundefined
@@ -123,4 +104,44 @@ Drupal.wysiwyg.editor.detach.whizzywig = function(context, params) {
+  insert: function (content) {
+    insHTML(content);

This function is a bit scary. It will insert at the current selection, which it accesses via a global sel object, or it replaces an element held in the global papa variable - both of which are only guaranteed to be defined if an insHTML call has thrown an error!

Additionally, it runs eval() on any content prefixed with "js:" instead of inserting it.

Avoid it?

Will see if I can do these tomorrow.

Powered by Dreditor.

bryancasler’s picture

subscribe

fearlsgroove’s picture

A follow up .. fixes initializing the field attribute on the none editor.

I'd respectfully disagree with your points on insHTML and openWYSIWYG. There's only so much that's reasonable to do on editors that don't have a reasonably useful api (or that have a unreasonably dangerous API :D). There's a few things I've got waiting for this patch to at least make it into dev (specifically CiviCRM integration). It seems this patch is 98% there, and it'd be unfortunate if a few feature-poor editor libraries held it up.

Then again, looks like you've already gone much farther to support the marginal editors anway so .. good for you guys i guess :D

fearlsgroove’s picture

Status: Needs work » Needs review

nr

TwoD’s picture

This looks pretty good and does appear to work according to my tests, but I had to make some additional changes.

+++ b/editors/js/wymeditor.jsundefined
@@ -44,12 +44,24 @@ Drupal.wysiwyg.editor.detach.wymeditor = function (context, params) {
+  getContent: function (content) {
+    return this.getInstance().xhtml();

Removed the argument.

+++ b/editors/js/openwysiwyg.jsundefined
@@ -65,4 +65,56 @@ Drupal.wysiwyg.editor.detach.openwysiwyg = function(context, params) {
+  insert: function (content) {
+    alert(Drupal.t('Sorry, inserting content is not supported on openWYSIWYG.'));
+  },

Found a way to do this.

+++ b/editors/js/whizzywig-56.jsundefined
@@ -130,4 +113,36 @@ Drupal.wysiwyg.editor.detach.whizzywig = function(context, params) {
+  insert: function (content) {
+    insHTML(content);

Made sure Whizzywig won't execute scripts starting with 'js:'. Should have minimal impact on what gets inserted. Changed in all whizzywig#.js files.

quicksketch’s picture

Status: Needs review » Reviewed & tested by the community

This patch looks and works great to me. I've tested it with "none", CKEditor, and tinyMCE on both D6 and D7 (patch applies to both branches without trouble). I've been working with the Autosave project which requires this patch. That project has just over 1000 users, most of which probably also have WYSIWYG module installed. It would be great to include this enhancement to WYSIWYG API so that at least a couple hundred sites could move back to the official, unpatched releases of WYSIWYG.

sun’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, @quicksketch!

Didn't apply anymore, but I've manually re-rolled it.

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.

liquidcms’s picture

lol.. hilarious.. been trying for over 2 years to have this committed.. quicksketch asks; and it's done.. sweet.. gotta hire you out to post to more of the patches i submit.. :)

TwoD’s picture

When quicksketch reviews, patches get committed. *Old Drupal saying* ;)

Thanks for picking this up, sun. I should have gotten back to this long ago but I've built up a backlog so big it's not even funny anymore... Sorry for the long wait, liquidcms.

liquidcms’s picture

sad part is this was originally done up to help out the D6 version of Autosave; but then Crell took over Autosave from me for D7 and much to my dismay he dropped WYSIWYG support. hopefully this will prompt getting Autosave back on track.. :)

thanks guys.

Status: Fixed » Closed (fixed)

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

elhendricks’s picture

Issue summary: View changes

I'd like to request that this patch be applied to the D6 version, if another release is planned. This corrects an error in the 4.4.x and later versions in CiviCRM. I have applied the patch but it would be good to have it included in any future releases. Thanks!

TwoD’s picture

The patch was applied to 6.x-2.x about two years ago so it's in the -dev snapshot. And yes, I am working on getting a release out there...