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.

Comments

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.

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 ?

Status:Active» Needs review
StatusFileSize
new3.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.

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.

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

Yes, it does.

Comment removed because it was stupid =)

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.

+++ 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.

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 ?

Status:Needs work» Needs review
StatusFileSize
new2.57 KB

A cleaner patch.

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

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

Status:Needs review» Needs work

Status:Needs work» Needs review
StatusFileSize
new3.18 KB

Here is the patch created from CVS this time.

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.

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

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?

(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.

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.

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

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

Subscribing.

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

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.

StatusFileSize
new2.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.

subscribe

subscribe

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 ?

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.

Was this committed?

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.

Status:Needs work» Needs review
StatusFileSize
new8.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.

StatusFileSize
new8.06 KB

Bump, and a D6 version as well.

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.

StatusFileSize
new17.64 KB

+++ 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.

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.

subscribe

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

Status:Needs work» Needs review

nr

StatusFileSize
new18.77 KB

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.

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.

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.

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.. :)

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.

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.