I'd like to be able to export the wysywig configurations with ctools exportables, so they can be kept in svn and moved between sites easily. Seems like this might be on the roadmap for 3.x, but couldn't find an issue, so opening one.

Files: 
CommentFileSizeAuthor
#242 wysiwyg-entity-exportables-624018-241.patch4.3 KBMiSc
#235 0001-feature.inc-from-624018-211-drush_make-7.x-2.1.patch3.09 KBpatcon
#223 0001-feature.inc-from-624018-211.patch3.51 KBthat0n3guy
#212 wysiwyg-7.x-2.x-entity-exportables-624018-208_drush_make.patch5.12 KBtnightingale
#211 wysiwyg.features.zip1016 bytessmk-ka
#208 wysiwyg-entity-exportables-624018-208.patch5.15 KBquartsize
#207 wysiwyg-entity-exportables-624018-206.patch5.15 KBderhasi
#199 wysiwyg-entity-exportables-624018-186.patch4.36 KBrv0
#186 wysiwyg-entity-exportables-624018-176_1.patch4.35 KBgirishmuraly
#183 wysiwyg-entity-exportables-624018-176.patch4.34 KBjide
#176 wysiwyg-entity-exportables-624018-176.patch4.36 KBquartsize
#173 wysiwyg-entity-exportables-624018-173.patch3.59 KBquartsize
#172 fe_wysiwyg.zip1.43 KBmoritzz
#150 wysiwyg-n624018-150-exportables.patch2.58 KBDamienMcKenna
#139 wysiwyg-624018-with-ui-5.patch20.61 KBIsaMic
#138 624018-138-wysiwyg-entity-exportables.patch2.97 KBnedjo
#133 624018-133-wysiwyg-entity-exportables.patch5.06 KBnedjo
#117 wysiwyg-624018.ctools.patch5.82 KBquartsize
#100 wysiwyg-624018-with-ui-4.patch19.9 KBdagmar
#85 wysiwyg-624018-with-ui-3.patch21.1 KBdagmar
#80 wysiwyg-624018-with-ui-2.patch20.38 KBdagmar
#77 wysiwyg-624018-with-ui.patch17.84 KBdagmar
#72 wysiwyg-624018.patch6.23 KBdagmar
#50 wysiwyg-624018-ctools-export-input-formats-2.patch20.09 KBdagmar
#46 wysiwyg-624018-ctools-export-input-formats.patch20.33 KBdagmar
#39 wysiwyg-624018-ctools-features-exports.patch12.93 KBdagmar
#39 screenshot_wysiwyg_features.png7.53 KBdagmar
#38 624018_wysiwyg_export_override.patch11.58 KBlangworthy
#29 wysiwyg_export_override.patch10.57 KBpounard
#26 wysiwyg_export.patch9.45 KBpounard
#25 wysiwyg_export.patch9.86 KBdrifter
#21 wysiwyg_exportables_1.png75.3 KBrickvug
#21 wysiwyg_exportables_2.png96.06 KBrickvug
#12 wysiwyg-624018-DRUPAL-6--2.patch8.82 KBdagmar
#12 wysiwyg-624018-DRUPAL-6--3.patch9.53 KBdagmar
#11 wysiwyg_features_1.patch9.2 KBrickvug
#10 wysiwyg_features.patch9 KBrickvug
#9 wysiwyg-with-features.tar_.gz87.87 KBrgristroph
#6 wysiwyg_features.module.txt2.4 KBcatch

Comments

You took the words right out of my mouth. Creating a solid WYSIWYG configuration can often rely on a lot of small settings. Having exportables for those settings would be a god send.

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

Good point. Bumping this to 7.x-3.x-dev as that's where the new major features will be added first. Note that this is not a promise that it will be added, but we'll look at it.

+1 this would be great, in order to build features, like catch said.

Yes this is an excellent feature. But, if settings of wysiwyg are related to an input format, export settings will mean export the input format too?

@dagmar You bring up a good point. Especially since input filters are set numerically, making them problematic to import and export reliably. I'm hoping that this is not the case in D7 seeing as how much of the filter module API has changed. The Exportables project may be a good place to tack on an exportable API for this. If not, WYSIWYG could always implement its own work around. In the end, I would hope that this complication doesn't side track any incremental improvements. WYSIWYG configuration exportables would still be helpful for install profiles and other use cases without input format exportables.

StatusFileSize
new2.4 KB

I'm using features on a large site upgrade to avoid having to write and rewrite custom hook_update_N() for as many things as possible, so I took an hour or so to write very, very basic features integration for wysiwyg-6.x-2.0. This doesn't attempt to resolve any of the bigger issues around exporting input formats + wysiwyg, just allows you to put the contents of the wysywig table into code and version control / revert / override it.

So I'm deliberately not even marking this needs work, but in case someone else is hoping to do the same thing, maybe it'll be useful. It's also implemented as a self-contained module rather than a patch, since that's how I'm handling these temporary upgrade feature implementations.

Issue tags:+CTools exportables

Tagging.

+1

StatusFileSize
new87.87 KB

I somehow missed this discussion when I set out to do my own WYSIWYG / features integration.

After making the settings exportable, I also made them importable, and then I made the WYSIWYG check for settings using a default_hook so that the module created by Features could provide the settings. When you export WYSIWYG settings to a feature, and then install that feature, the settings are in code and not in the database. If you then edit the WYSIWYG profile settings and save them, then those go to the database and override the ones in the feature.

I have attached a tgz of my modified 6.x-2.0 WYSIWYG. Should I also provide a patch against one of the CVS branches ? If so, which one ? DRUPAL-6--3 ?

StatusFileSize
new9 KB

@rgristroph Thanks for sharing! I created a patch out of your modifications to make them easier to review.

Status:Active» Needs review
StatusFileSize
new9.2 KB

First patch did not include the new file wysiwyg.features.inc (although it is blank anyhow).

Well I have reviewed and modified a bit this patch.

Big changes:

Exports provides as unserialized arrays (this is better to diff changes)
Same for feature support.

Small changes:

A lot of coding styles fixes.
Changed title in export page.

Mantainers: Related to #2: I'm providing this patch against DRUPAL-6--2 and DRUPAL-6--3, I have only tested this patch using DRUPAL-6--2 and I'm sure that is the branch were we are going to get more testers.

Sadly I haven't much experience developing for Drupal-7 (in fact we have to wait one week more for the first alpha for D7) so If you agree, I suggest change the version for this issue to 6.x-2.x-dev (or 6.x-3-x-dev if you enable it) to get test from more people.

Awesome! Subscribing.

I'm not having luck with the DRUPAL-6--2 patch. The editor settings aren't showing up in either the default "Filtered HTML" format or a custom format I defined on the source site. Perhaps the import is breaking due to the lack of the custom format, which wasn't created prior to enabling the feature?

It's also worth noting that the status of the feature is "Default," even though the settings are clearly wrong.

I just tried a simple export/import of the "Filtered HTML" format's WYSIWYG setting and received an error. I suspect this is why the feature isn't working.

Here's the export:

$wysiwyg = new stdClass;
$wysiwyg->disabled = FALSE; /* Edit this to true to make a default wysiwyg disabled initially */
$wysiwyg->api_version = 2;
$wysiwyg->format = 1;
$wysiwyg->editor = 'tinymce';
$wysiwyg->settings = FALSE;

The error:

    * warning: implode() [function.implode]: Invalid arguments passed in /my/site/path/includes/form.inc on line 841.
    * Settings for that format input type already existed; your import was not used and nothing was done. Please delete the existing wysiwyg format settings for that input type and import again.

I tried the same export/import on a cleaner install. It worked this time, but I still got an error:

warning: implode() [function.implode]: Invalid arguments passed in /Users/todd/Sites/yale/includes/form.inc on line 841.

Version:7.x-2.x-dev» 6.x-2.x-dev
Status:Needs review» Needs work

@Todd Nienkerk: Ok I will review the patch again, thanks for your test.

In a conversation via IRC whit sun, he told me that patches for 6.x version are still accepted. So I'm changing the version for this issue.

(18:44:58) dagmar1: tha_sun: hi, I'm working in a patch to provide exportable support for wysiwyg. I would like to know what is the current working branch
(18:45:31) dagmar1: sorry, development branch
(18:45:53) tha_sun: dagmar1: HEAD is 7.x, everything else is 6.x... the 2/3 branches actually should not differ yet, so choose whatever you have locally ;)
(18:46:38) dagmar1: tha_sun: so, patches against DRUPAL-6 are still accepted?
(18:47:51) tha_sun: dagmar1: yes, especially because I think that features/exportables isn't ported to D7 yet...?
(18:48:11) dagmar1: tha_sun: good point :)

Hi Folks, I believe you're going to have some trouble with this approach. The issue stems from how WYSIWYG relates it's configuration back to the filter formats using numeric IDs and then uses these IDs itself as the primary identifier of the configuration. In order to get exportables that are useful between sites you're going to need to get away from using a numeric ID as the identifier.

I can see two possible approaches; 1) somehow use the human readable name of the format (as stored in the filter_formats table) as the id in the export. 2) store string IDs in the wysiwyg table and then some how map those back to formats.

I'm not sure which is going to work out better, but I think these have the potential to get better exports for this config.

jmiccolis is right, numerical ids cause problems in these cases, though very convenient in most cases. I see this is also discussed elsewhere, like #643566: Text formats and #640302: Provide exportable for input formats.

I think one way to solve this problem of relating say Wysiwyg profiles to Input formats would be to add an extra step after importing the profiles. In this step they are all listed and can be mapped to another list of the current input formats (by the user). Possibly with the ability to keep the profiles in this half-imported-state until new Input formats have been created, if there weren't enough to assign all the profiles in one go. There is of course no guarantee that a profile being assigned to a format will actually have matching settings, but that is an issue in the current state as well, even if the format ids all exist. So when exporting an editor profile, the format id is left blank. Then on import, a profile does not go straight into the database (after validation) but is instead stored in a temporary place/cache somewhere where it can have those format ids set before being put into the db (passing validation again). It's just a thought, but I think it might be possible to apply to the relationship between input formats and filters as well. However, the amount of time and user interaction it takes to import it all will of course increase a lot.

Yeah I deliberately didn't set #6 to CNR or CNW because it wasn't supposed to be a real patch. Using numeric IDs works as a quick hack to save me writing a custom hook_update_N() in my specific use-case for features, but it shouldn't be used for anything more than that.

To do this properly, since core itself is using numeric ids for formats and there's not much can be done about that until D8, it might be possible to work around it with http://drupal.org/project/exportables - something like that needs to be worked out for exporting input formats first, then applied to wysiwyg/better formats etc. once it's in place so they can share the same basic metadata about what refers to what.

StatusFileSize
new96.06 KB
new75.3 KB

For what it is worth I tried the patch in #12 and received similar errors. On first install the feature enabled properly. After changing a setting, it showed as overridden and would not revert. See the attached screenshots for the errors and diff output. I then tried a clean install and received the same error (once). Beyond the error message and overridden status the patch seems to work as expected.

sub...

subscribanatoring...

subscribe

Status:Needs work» Needs review
StatusFileSize
new9.86 KB

Rerolled the patch in #12 for Wysiwyg 2.1. Also fixed the error in #16 (_validate() didn't have the correct arguments). Added an option to (optionally) select a target input format on import, so you can export from one format and import into another.

Apply the patch without -p0.

StatusFileSize
new9.45 KB

@drifter #25
Works for me, I manually fixed your patch in order to have the right file path (rest of the patch is exactly the same).

A clean update function is missing for already existing sites.

There is no actual database schema change, so no update hook is necessary. The changes in .install/hook_schema are for CTools exportables support.

Thanks for the updated patch, couldn't figure out the correct git diff switches :)

Ok, finally, I got a bug :)

The first export of my features went well and got back editor options (as I remember), but then I used the feature recreate form to get back an updated version and I loosed all options. Since, I'm not able to export those options anymore.

If I found why, I'll try to patch it.

StatusFileSize
new10.57 KB

Ok found something. When you edit a "default format", the submit handler does an sql update on an empty row, so it does not save. I modified the the overview form, and edit form in order to handle defaults.

Now on the overview form, if a default profile is not present into database, the 'edit' link becomes 'override'. At edit submit time, the submit handlers checks if there is any data in database, if not, create a new record instead of modifying an empty one.

Now it works fine on my environment. Redone your patch using the diff command.

subscribe

I had to manually apply hunk#1 (the extra else added to wysiwyg.admin.inc) - tried the patch (and various other patches in this thread) a few times.

-Bram

Why is it when I reply to #29 (by clicking the reply link in that message) my reply does not appear indented underneath #29? It ought to be....but that's not an issue for this case!

Did you applied the patch on 2.1 stable release?

Subscribing.

Subscribe

StatusFileSize
new11.58 KB

I had trouble with #29 when using an exported feature definition in a *.features.wysiwyg.inc file.

I changed the following line in wysiwyg.module

$obj->settings = unserialize($obj->settings);

to
  if (is_string($obj->settings)) {
    $obj->settings = unserialize($obj->settings);
  }
  else {
    $obj->settings = $obj->settings;
  }

patch attached

//edit: noted which file change was made to relative to #29

I have reviewed the whole patch, and I did make some changes:

  • Usability. Now in features exports users see something like the attached screenshot.
  • Features revert and override are working fine.
  • The wysiwyg.features.inc is not empty anymore
  • CTools is not a dependency for wysiwyg, so, this patch now checks and guides to the user to install Ctools if he want export/import capabilities

I did test the import/export process using plain exports and features, they seems to be working fine. There is only a problem when users try to delete a profile from an input formats that is loading the profile from a feature. It is never deleted. (until of course, you disable the feaure) It is obvious, because the profile is loaded from the code every time if not found in the database. I don't know if we have to document this behavior, or this can be fixed in some way.

Anyway, it is working! please make your tests and provide feedback.

Looks great... but it doesn't work for me. In features, I'll get the extra 'wysiwyg profiles' option. I can see the extra input format which I've defined. But it's naming is missing. When I click on the tickbox, nothing happens. There is no indication whether or not the format was included. The code of the feature is lacking every reference to the wysiwyg profile/input format too.

Drupal 6.16 with better formats and wysiwyg module.

My bet is a conflict with Better Formats.

I skimmed through the code of the patch. Noticed a change in hook_schema in wysiwyg.install. Of course, I had already installed and configured wysiwyg before applying this patch. That's why it didn't work in the first place.

Now, it's working great. Even with Better Formats.

The numeric id's are definitely something which needs taking care of though.

Looking good. This is much needed. I haven't tested. From a code review, a few comments:

+++ wysiwyg.admin.inc
@@ -316,7 +316,16 @@ function wysiwyg_profile_form_submit($form, &$form_state) {
+  if (!db_result(db_query("SELECT 1 FROM {wysiwyg} WHERE format = %d", $format))) {
+    $wysiwyg = new stdClass();
+    $wysiwyg->format = $format;
+    $wysiwyg->editor = $editor;
+    $wysiwyg->settings = serialize($values);
+    drupal_write_record('wysiwyg', $wysiwyg);
+  }
+  else {
+    db_query("UPDATE {wysiwyg} SET settings = '%s' WHERE format = %d", serialize($values), $format);
+  }

Would be slightly cleaner as:

<?php
 
// Determine if this is an update.
 
if (!db_result(db_query("SELECT 1 FROM {wysiwyg} WHERE format = %d", $format))) {
   
$update = array();
  }
  else {
   
$update = array('format');
  }
 
$wysiwyg = new stdClass();
 
$wysiwyg->format = $format;
 
$wysiwyg->editor = $editor;
 
$wysiwyg->settings = serialize($values);
 
drupal_write_record('wysiwyg', $wysiwyg, $update);
?>

+++ wysiwyg.admin.inc
@@ -529,3 +556,102 @@ function wysiwyg_profile_delete($format) {
+    return form_error($form['wysiwyg'], t('Unable to interpret view code.'));

Should be "Unable to interpret wysiwyg code."

+++ wysiwyg.admin.inc
@@ -529,3 +556,102 @@ function wysiwyg_profile_delete($format) {
+  // Check to see if the setting exits (should have been caught by validation hook above); if so give an error;

"exits" should be "exists"

+++ wysiwyg.admin.inc
@@ -529,3 +556,102 @@ function wysiwyg_profile_delete($format) {
+  if ($dwr_ret == FALSE) {

May as well use ===

+++ wysiwyg.admin.inc
@@ -529,3 +556,102 @@ function wysiwyg_profile_delete($format) {
+    return ;

Extra space after return.

+++ wysiwyg.module
@@ -585,12 +601,22 @@ function wysiwyg_get_css() {
+    foreach ($default_profiles as $def_prof) {
+      if (!isset($profiles[$def_prof->format])) {
+        $def_prof->default = TRUE;
+        $profiles[$def_prof->format] = $def_prof;
+      }
+    }

$default_formats instead of $default_profiles

$default_profile or $format instead of $def_prof

+++ wysiwyg.module
@@ -609,6 +635,16 @@ function wysiwyg_profile_load_all() {
+    foreach ($default_profiles as $def_prof) {
+      if (!array_key_exists($def_prof->format, $profiles)) {
+        $def_prof->default = TRUE;
+        $profiles[$def_prof->format] = $def_prof;
+      }
+    }

$default_formats instead of $default_profiles

$default_profile or $format instead of $def_prof

+++ wysiwyg.module
@@ -968,6 +1004,77 @@ function _wysiwyg_process_include($module, $identifier, $path, $hook) {
+/**
+ * Load a single format setting.
+ */
+function wysiwyg_format_load($format) {
+  ctools_include('export');
+  $result = ctools_export_load_object('wysiwyg', 'names', array($format));
+
+  if (isset($result[$format])) {
+    return $result[$format];
+  }
+}
+
+/**
+ * Export wysiwyg settings.
+ */
+function wysiwyg_format_export($obj, $indent = '') {
+  ctools_include('export');
+  if (is_string($obj->settings)) {
+    $obj->settings = unserialize($obj->settings);
+  }
+  else {
+    $obj->settings = $obj->settings;
+  }
+  $output = ctools_export_object('wysiwyg', $obj, $indent);
+  return $output;
+}
+

While it's true these are only called from code blocks that test for ctools, they should still include the same test, in case they're called from elsewhere. Or they should be marked as private functions (function _wysiwyg_format_load).

Powered by Dreditor.

Great work, reviewing.

I have recently uploaded http://drupal.org/project/input_formats to deal with the numeric ids for input formats. Please, take a look to this module and provide some feedback :)

Maybe we can make an integration with this patch and import wysiwyg settings using machine_names instead of numeric ids.

Ok, here is a possible approach to integrate machine names with Wysiwyg without make input formats a dependency for Wysiwyg: http://drupal.org/node/821036

ideas?

I vote +1 for input_formats module support.

Well, I have worked a bit in this patch.

First to all, thanks @nedjo for the review. I have included all the tips you said.

Ok, this patch includes a lot of news features:

  • Features integration using a modified version of ctools functions. This allows other modules to alter settings before create the feature. So, Input Formats can modify the numeric ids and add some dependencies before the feature is created.
  • As this patch allows to load settings from code, a new cache system was implemented using cache tables (like Imagecache does).
  • Patch #39 was modified to display the UI with 'export' and 'revert' where appropriate.
  • Modified wysiwyg_profile_load to take advantages of the cache system.
  • Several drupal_alter() calls to allow other modules to modify wysiwyg settings when are loaded.

As you can see in the patch, wysiwyg.features.inc is bigger than in #39. Well this was necessary to allow other modules to alter this exports. Remember, I'm doing all this things to get exportables using 'machine names' instead of numeric ids.

I have committed all the wysiwyg_*_alter implementatations to modify numeric ids into Input Formats checkout from cvs or wait to alpha-2 to tests it.

subscribing

subscribe

Subscribing.

This is a small modification of #46. Delete profiles were not possible with previous patch.

Also I forgot mention that Heine Deelstra suggest me via IRC to include a new permission to import wysiwyg profiles.

Title:ctools exportables supportctools exportables and features support

Yikes!
I missed this when I was implementing features/ctools support too. Changing the title so other people who search for 'features' instead of ctools might find it.

In #50, dagmar said...

"... include a new permission to import wysiwyg profiles."

I'm all for granular permissions, but this one confuses me. Help me understand under what circumstances someone would be importing a wysiwyg profile who does not have permission to administer filter formats?

Mmm, yes, it is true, but if you see in other perspective, an administrator may want allow users to edit wysiwyg profiles without allow they to "import profiles using php code".

So, the permission is provided to allow users to edit wysiwyg's settings without allow they to execute php code and not backwards.

This looks very useful, I'm working on a WYSIWYG feature right now, I'll give the code a spin.

Status:Needs review» Reviewed & tested by the community

OK, works beautifully! As far as I can see this is good enough to commit.

Just a couple of notes for anyone trying it out:

1) If you apply the patch to an existing site, you have to clear caches before the export will work, I think because schema doesn't get updated to find the export information.

2) When you set up a new site and enable this as a feature, you have to clear the caches before you will see the wysiwyg editor (filter caching or something).

I applied the patch, exported an existing configuration into a feature, set up a clean install and enabled the feature and it worked beautifully.

I should note I was using this with http://drupal.org/project/input_formats, as noted above, to be able to export the input formats so wysiwyg could apply the editor to the format.

Subscribe

Edit: that didn't work the way I wanted it to. :)

subscribe

Status:Reviewed & tested by the community» Needs work

This is looking generally okay and the introduction of the Input formats module is a step forward, but the patch needs some more thought and work.

The main issue is the handling of numerical IDs. If it relies on numerical IDs, it seems to me, it's not a feature. If we're implementing this, we need to get it right, and that means that means machine names.

Currently, the patch reworks some code from ctools to introduce a number of hooks. The Input formats module has been patched to implement these hooks. So exports will have machine names if a site admin has thought to install input formats, if not then numerical IDs.

This sorta works, but awkwardly. First, if Input formats is an API module facilitating import and export of data related to input formats, it shouldn't be necessary to introduce a bunch of new hooks and patch the Input formats module every time another module wants to use it. To follow this model for e.g. Better formats, (see #616496: Features integration) would have to have its own versions of ctools exportables functions, introduce new hooks, and then patch Input formats.

Instead, we should start off with: Is Input formats the right approach, and if so is it stable enough to rely on? Is there further work to be done in that module before relying on it?

If input formats is the right approach, it should be introduced as a requirement for wysiwyg import/export functionality alongside ctools bulk_export. Since we'd be testing in several places for two separate modules, best to pull this test into a helper function.

Then we call methods in Input formats to load the machine names and to switch out machine names for numerical IDs.

In general, I'd like to see how another module has introduced optional ctools exportables support for comparison. Can someone suggest a module to compare?

Other smaller issues:

For consistency with existing usage, wysiwyg should be capitalized in several places, Wysiwyg.

+++ wysiwyg.admin.inc 16 Jun 2010 17:10:45 -0000
@@ -415,13 +426,15 @@ function wysiwyg_profile_overview() {
+  $ctools_installed = module_exists('ctools');

It's not really ctools but bulk_export that we need to test for and give a message about. Installing and enabling ctools isn't enough.

+++ wysiwyg.admin.inc 16 Jun 2010 17:10:45 -0000
@@ -456,10 +488,115 @@ function wysiwyg_profile_overview() {
+  ¶

Spacing issue.

+++ wysiwyg.features.inc 16 Jun 2010 17:10:46 -0000
@@ -0,0 +1,100 @@
+ * ¶
+ * All this hooks are not required to export settings using CTools.
+ * But, due wysiwyg use numeric ids related to input formats, we are
+ * allowing other modules to take care about this numeric ids and ¶
+ * provide exportables with machine names.
+ */
+
+/**
+ * Implementation of hook_features_export_options().
+ * ¶

I'm thinking we should revert all of this, but if it stays, there are several spacing and grammatical issues. Should read:

These hooks are not ... But since wysiwyg uses ... we are enabling other modules to provide exportables with machine names.

+++ wysiwyg.features.inc 16 Jun 2010 17:10:46 -0000
@@ -0,0 +1,100 @@
+  // Only the first paramaeter is passed by reference in drupal_alter

paramaeter should be parameter

This and several other comments need end of line punctuation (period).

+++ wysiwyg.features.inc 16 Jun 2010 17:10:46 -0000
@@ -0,0 +1,100 @@
+      // Use $xport[$wysiwyg->format] instead of $export[$object] ¶

$export instead of $xport.

+++ wysiwyg.module 16 Jun 2010 17:10:49 -0000
@@ -87,6 +114,20 @@ function wysiwyg_help($path, $arg) {
+  return array('import wysiwyg profiles');  ¶

Extra spaces at end of line.

Powered by Dreditor.

subscribing

does the latest dev build patched with ctools exportables and feature support ?

Subscribing.

Subscribing.

I'm going to try to explain the problems I got while I was coding this patch and input formats...

The big problem we have are not numerical ids... the problem is, that filter module is part of the core, and we cannot modify it.

Other modules, like Quicktabs (#572880) can be modified to use machine_names instead of numerical ids. But, with filters we cannot do that.

There is another problem with this. CTools Exportables are expecting a machine name as key of the base table. And, due wysiwyg uses numerical ids from filters, CTools will use this numerical ids in their exports/import functions.

Due to the filters architecture cannot be modified, not until Drupal 8, I just created input_formats module to don't have to wait two years more to be able to load wysiwyg and input formats from code.

Input formats is just an API to synchronize numerical ids with machine names, plus a set of tools to export/import settings for input formats.

Is it stable? Well, it's wroking... of course probably with the time there will be things to fix, but right now is usable and I have fixed all issues I founded during my tests.

Also, why so many hooks? Again, since numerical ids are something normal in input formats, many times objects are loaded using this ids. So I had to check every load and import case to make sure that in all the ways where a wysiwyg profile was exported/imported finally a numeric id were used.

The hooks implemented in input formats were introduced to avoid a dependency for input formats. Wysiwig is a mature module, and input formats isn't yet.

I know that it is better to use a few call to input_formats api instead to use a dozen of hooks calls, I was wondering what would think others developers about this... wysiwyg depending of a new module? A module with 2 weeks of life...? Probably the patch would be rejected.

Input formats is the first module to offer a solution for numerical ids for filters. I don't know if there is another module to manage filters with machine names.

I just didn't do things mentioned in #59 because I did know that input formats wasn't ready to be a dependency of wysiwyg. But if the community thinks that input formats can be the solution for numeric ids for filters in Drupal 6 and 7... so we can work in a stable version of input formats before continue working on this patch.

This are my thoughts right now. I agree with @nedjo, wysiwyg cannot use numeric ids in features, if anybody else have another approach for this patch, it would be nice to put they here too.

BTW, @nedjo, thanks again for your review.

This was longer than I expected, sorry for my english.

@dagmar, have you thought about abstracting your code for use with the Exportables module? Essentially making it so that arbitrary numeric keyed entities can be exportable?

I am working on a patch that uses hook_filter_default_formats instead. I hope to have it ready to post in a day or so.

sub

subscribing

@q0rban: Yes it is a good idea. However I would like to see a more stable version of Exportables, like a beta or a release candidate to rely on its API.

I asked to dmitrig01 to be the co-maintainer of exportables, it seems to be the last commit for this project was 28 weeks ago...

But yes, in general, with some patches to exportables I could modify input formats to work with it.

subscribing

I'm having a little issue with this patch, when you have more than one wysiwyg profile in a feature, it seems only to import the first one. Anyone have experienced this issue?

Status:Needs work» Needs review
StatusFileSize
new6.23 KB

A week later, here are some news:

Thanks to @dmigrig01 and @jazzslider I'm the new co-maintainer of exportables, and recently have created a new branch of exportables to deal with the machine names.

Also, @merlinofchaos committed a patch that allows ctools exportables to use joins in tables.

The result?

A really small patch that provide full exportable support using features, the admin UI to export patches is missing, but I will code it soon.

A good new is that input formats is not a dependency anymore, and as @q0rban suggested in #65, I will port the api of input formats to use Exportables api.

To try the patch you need the last development version of ctools, and the 2.x branch of exportables.

I'm setting this as "needs review" but indeed it need a bit of work to include the UI to import/export.

+1

Just tested dagmar patch in #72 inside a feature for wysiwyg profiles and it seems ok

Status:Needs review» Needs work

Some things to do with this patch.

If an input format doesn't exists, wysiwyg should inform to the user that there is a wysiwyg profile that cannot be installed because the input format doesn't exists.

If exportables module is not installed, {exportables} table will not exists, and therefore, a sql query warning will be displayed, maybe this needs a patch for ctools, or maybe a implementation of the hook_exportables_api with a if (module_exists('exportables')) like the patch in #50.

The UI have to be coded. I tried the new UI provided by ctools/exportables-ui, but it is too much for this module, I would like to include the UI as a separated module, something like Wysiwyg Copy (as cck does with content copy) and there, include ctools and exportables as a dependency. Opinions?

Features module is using a hard-coded implementation of ctools_export_crud_delete in ctools_component_features_revert(), this have to be modified in both, CTools and features, to support DELETE FROM table1, table2 ... using machine names. If this is not ready, revert features will not be possible.

Changing to "needs work' until finish this things.

I've been using the patch from #46 (haven't tested the patch from 72 yet) and using it with an install profile. After an install, I have to visit (I only have to visit, don't need to save anything) admin/build/features, admin/build/imagecache, and admin/settings/wysiwyg for my feature to work right.

After visiting admin/build/features, my input formats show up. After visiting admin/settings/wysiwyg, my wysiwyg editors load after changing them in a nodes input formats. After visiting admin/build/imagecache, my imagecaches show up w/ wysiwyg_imageupload in my wysiwgy editor.

My feature works fine if I enable it from features, but not from install profile. Just thought I'd let ya know.

Status:Needs work» Needs review
StatusFileSize
new17.84 KB

Here is the patch with the UI working. I have created a sub module called Wysiwyg Copy to provide the Import/Export UI. (something like cck does)

Now features integration are only available if exportables and ctools are installed. This prevent warning from missing tables in the db.

The UI is very similar to views UI, terms like Overridden, Revert and Export are used in the same way.

I have created the two issues mentioned in #75, I will try to provide patches for those issues as soon as I can.

Features: #853120: Use ctools functions instead of querys for revert features
CTools: #853114: Delete objects from tables considering joins

I recommend use the last development version of features (I did my test with that version) and remember to use the 2.x branch of exportables module.

One thing you might look at for UI implementations is the new export UI include in the dev version of ctools. :)

I did it, but it is too complex for wysiwyg options. Also it doesn't allow to select the wysiwyg editor with a select box, too much things to modify.

StatusFileSize
new20.38 KB

Well, it is working pretty good :)

This last patch includes a hook_requirements() to inform about missing Ctools or Exportables modules in the Status Report.

Also include a way to revert wysiwyg profiles directly from features UI, but it requires a patch for features #853120: Use ctools functions instead of querys for revert features

The patch for Ctools (from comment #77) is no longer required.

I have tested a few iterations of this patch (together with both stable and HEAD versions of both ctools & features, clean installs) and have the same problem as above - it only ever shows the first WYSIWYG configuration in features.

As far as I can tell it is not joining to the exportables table at all, and hence is not picking up the machine name for the input format, causing the array items to have no key and them clobbering each other.

@dagmar - it seems like you have this pretty well figured out, any ideas why ctools isn't joining the tables correctly?

@Owen Barton: Yes, I know there are some issues. However, I would like to insist on this: This patch is for wysiwyg, not for input formats. If you don't have the input format installed in your site, the wysiwyg won't be installed.

My tests for #80 were done with all the input formats created (and they works fine, with two wysiwyg, both were loaded). You can export both, the input format and the wysiwyg profile in a feature, but there are some issues. I recently discovered in input formats module that are making this things not work properly.

I'm going to modify the way how input formats works in beta-2. I discovered a way to rely on strongarm and ctools to load input formats "really" from code (at this moment, input formats are copied into the db). But I need a week to create this new code. Until that, test to this patch should be done with the correct input formats properly created.

So, if anybody can confirm #71 under the conditions of testing I described above, feel free to change this issue to 'needs work' again. And of course, @pcambra, @Owen Barton, thanks for the feedback.

@dagmar, thanks for responding. I am referring to exporting the configuration to a feature (not installing the configuration using an already installed feature), if that helps. The input formats are obviously already present in the case of exporting - do you mean that the input formats need to be added to the feature (I am using your input formats module also) when exporting before the wysiwyg configuration can be added to the feature?

Loading input formats directly from code would be awesome, if that is possible - if that works it seems like it would be quite straightforward to do the same for wysiwyg also.

@Owen Barton: now I can replicate this problem. I'm not sure why it was working fine before...

Take a look to this patch. I think this is the raise of the problem: #862320: Skip check if 'export' exists on a schema when are used in joins

StatusFileSize
new21.1 KB

Here is a new patch that works with the recently created release of Exportables 6.x-2.0-beta1.

Also includes the revert capabilities using CTools 1.7 api that were not included in Features 6.x-1.0-beta10.

@Owen Barton: Input formats 6.x-1.0-beta 2 rely on Ctools 1.7, Strongarm and Exportables 2.x. Now input formats are loaded from code. http://drupal.org/node/869978

Could the module just have an "export/import" function independant from CTools? This is just like using a nuclear war head to kill a tiny fly. Strongarm has really specific use cases, and is a great module, but here for defaults handling and simple import, this is just an enourmous overhead that does not proove itself to be usefull. Why does people has to use such machinery, I know that Drupal lacks the "exportable" concept (really lacks it a lot), but using 3 different modules to achieve this goal, this is just insane. Exporting a wysiwyg module profile is like 10 lines of code, no need for those modules don't you think? And shouldn't the wysiwyg module remains independant from such modules? The features module support, even if -for some people at least- is really needed, at best, would need CTools exportable, or, if you have a bit of courage, maybe 50 lines of custom code to be done.

Using CTools AND Exportables only for default profiles loading/support is crazy, and useless.

@pounard Have to disagree here. CTools is nearing ubiquity now. More modules using its exportables API means less custom code needs to be written and the simpler this all becomes. I agree that having exportable input formats and WYSIWYG settings shouldn't take a group of modules with dependencies. However this is the option we are left with when working around core's limitation while keeping things modular.

@pounard: Why is it an overhead to use ctools? It is not like you are exporting these things every day. What about, to mitigate against overhead - having any supporting code that has to live in this module, to live in a submodule wysiwyg_export, and let that submodule have the dependency on ctools. That way, you can just enable wysiwyg_export and ctools when you need to export. Disable them again when you are done. No overhead. Less overhead infact, since you have then removed all the export specific code out of the main wysiwyg module, which is the only one that needs to be enabled all the time.

ctools really is the way to go IMO. Individual modules should not be reinventing this stuff.

If you just look at the "module provided defaults" feature, fetching the defaults is something like calling a simple hook. Using CTools goes within a call stack trace with an incredible depth, forces to write all sorts of hooks, doing (really a lot) of includes, which seems a bit too much just to call a hook and enventually do (static) caching.

Remember that this particular feature (defaults) can potentially be called each time a wysiwyg editor appears.

Plus, if I look at the latest patch, there is as many lines of codes just to use CTools that it would need to implement a custom and simple hook.

EDIT: Typo (twice)
Notice: This is only IMHO, and if you choose to use CTools and Exportable I will totally respect that.

@pounard, thanks for your comments. I just would like to say that I have been working a lot of days around this issue, and I have examined a lot of possibilities, and this last patches are the best approach I have found. A balance between performance, don't reinvent the wheel and functionality.

As you can see in wysiwyg_profile_load_all(), all the 'hard work' is performed only if cache is not available. As I said in #46, the cache system is implemented to improve the performance of all the system. This means that wysiwyg profiles cache will be re-loaded only when you visit admin/settings/wysiwyg, and in admin/build/modules, and not every time a wysiwyg is requested.

For other side, it is true that CTools is slower than use our own code. But, as @mfelton said in #91, you will not export and import wysiwyg as a regular task, so few seconds are not so important.

Related to Exportables, I would like to said, there are not other valid options. Filter module doesn't provide machine names, and wysiwyg profiles are deeply relatd to input formats. Even more, exportables don't overhead a site, it just work in special cases, like administrative screens to keep sync the input formats with their machine names. And strongarm is no needed for this patch, strongarm is needed for input formats, other module.

I know that this stuff can be improved in terms of performance, and, in my opinion, is here were we should put our efforts, make this things works as faster as possible.

Anyway it is good to see this discussion, because we can see where are the critical points of action.

@dagmar Where I really agree with you, is the problem of numeric input formats. There is no easy way here. But may be using the human name, or a hash of the human name, as key for import/export could be a solution, Drupal core default provided formats always have the same name? (I known, this is not really a solution, but at least, it would avoid adding non core related meta-information for those).

EDIT: In fact, the whole compexity of export comes from that fake need of machine name, which is kind of tricky there. I don't think putting machine name everywhere is the solution.

subscribing.

A bit off topic note, there is a similar issue there, with somehow similar questions and problems: #558378: Make workflows exportable with Features (D6)
There is good ideas here and there, so I think you should give it a read.

subscribe

subscribing

subscribing

Worked great for me with 6.x-1.10 (I've only changed the button settings for the two default input formats).
Thanks!

Using the patch at #85 gives me these errors: http://drupal.org/node/890412 untill exportables is enabled.

StatusFileSize
new19.9 KB

@that0n3guy exportables 2.0-beta1 is required to use this patch.

Now that features is stable and properly integrated with CTools 1.7, I'm uploading a new version of this patch. With less lines of code.

Please remember clear your caches.

Title:ctools exportables and features supportCTools exportables and Features support
Version:6.x-2.x-dev» 7.x-2.x-dev
Status:Needs review» Postponed

Sorry for coming late to the show. Thanks for all the work and reviews that have happened so far. However, I've re-opened #643566: Text formats (Features, D7), as I'm pretty sure that all of this cannot properly work without an enforced machine_name for text formats. My very basic proof-of-concept patch over there should solve the underlying problem of a missing machine_name by simply adding it at the right locations and at the right time, i.e., focusing on CUD operations. (which already works cleanly based on manual testing) Of course, the Filter API situation is heavily improved in D7, as we've completely revamped it. Among many other changes, filter settings are now stored with each filter in {filter} instead of variables, which definitely solves or simplifies a large part of the puzzle (it's not that we couldn't make that identical machine_name enhancement concept work for D6, too - actually, the idea of a machine_name module crossed my mind already).

Additionally, I've created an almost RTBC patch for Drupal 7 core that massively simplifies the implementation of machine names: #902644: Machine names are too hard to implement. Date types and menu names are not validated Please help to get that in.

+1 sub

Following

FYI: Patch in #100 is working like a charm, thanks

I'll second that. No problems so far.

Since almost no one of you stepped up in #902644: Machine names are too hard to implement. Date types and menu names are not validated and #903730: Missing drupal_alter() for text formats and filters yet, it is becoming less likely that we will be able to export anything (until D8).

This issue could really use a recap. What are the recommended patches and what are their requirements? (Including core version.)

None of the patches submitted so far is recommended.

Rather, sun has led an effort, with help from several others, to get the bases for input format export into core for D7. This was done in #902644: Machine names are too hard to implement. Date types and menu names are not validated and #934050: Change format into string. The third piece, #903730: Missing drupal_alter() for text formats and filters, is not yet committed.

With these pieces in place, it should be possible to add input format export to Features in #643566: Text formats for D7 at least, with possibly a D6 backport.

When that is done, it will be feasible to write a new patch for WYSIWYG.

Patch #100 breaks with CTools 1.8 FYI. Hopefully we can circle back around with a patch. Downgraded to 1.7 in the meantime.

You need to input_formats and Wysiwyg_Copy to make this useful.

function wysiwyg_features_export($data, &$export, $module_name = '') {
  $return = ctools_component_features_export('wysiwyg', $data, $export, $module_name);
  $export['dependencies']['exportables'] = 'exportables';

I know submit a patch. I am crunched.

subscribing

subscribing.

Also subscribing.

Issue tags:+Features integration

subscribe

subscribe

StatusFileSize
new5.82 KB

Subscribe.

In addition, I've attached the patch for HEAD that I've been using. It loads wysiwyg profiles using CTools exclusively, allowing Features' CTools integration to work its magic. The simplicity of making CTools a dependency appeals to me, but if this is undesirable, it should be straightforward to modify this patch to make it optional.

Thanks for the updated patch!

With ctools now a dependency of Views, we can expect that more than two-thirds of Drupal sites will have it, so it's much less of a barrier than it was.

Status:Postponed» Active

Wysiwyg has reached a distribution level at which we cannot add a dependency on CTools for the sole sake of exportables — you neither need CTools nor Views for simple brochure and blogging sites, so the dependency is not guaranteed to exist; especially in D7. To clarify in advance, the commented out dependency line in the .info file dates back to an old idea of leveraging CTools' content type plugin framework for editor plugins; while this might still happen in the future, it's unlikely that it is going to happen for 2.x. Therefore, support for exportables needs to be optional.

That said, what's the difference to Entity module's support for exportables? Technically, wysiwyg_profile should be registered via hook_entity_info() anyway, so without knowing details, my current assumption is that by merely switching the entity controller class dynamically (based on whether Entity module is enabled or not), Entity module's controller class would automatically take over handling of exportables...? In other words: Let's make sure to use the latest and greatest implementation strategy here, not the (possibly dated?) D6 approaches.

Couldn't ctools be made an optional dependency pretty easily - wrapping in module_enabled/function_exists? I think requiring ctools to use exportables is very reasonable - I can't imagine anyone wanting to use exportables, but not needing ctools already installed.

Indeed in D7 Entity is an alternative to Ctools for providing exportables support and should be considered alongside Ctools.

Documentation on using Entity API for exportables is here: http://drupal.org/node/1021526. Easy!

Compared to Ctools, though, it looks a bit harder to make Entity API an optional dependency, since there's the need to add custom fields in hook_schema(). If we wrap those fields in a module_exists() test, how do we trigger creation of the needed fields if Entity API is subsequently enabled?

Also, can we get exportable support without triggering other, possibly not desired, Entity API handlers?

I can definitely see the issue with adding a ctools dependency to an established module as sun pointed out in #119. It was in large part the reason I did not post my patch in June. It has been some time since I looked at the code, but would it not be possible to have a separate wysiwyg_exportables module which requires ctools?

If I'm thinking and remembering correctly, it would probably require a bit of glue in wysiwyg proper to work though, so perhaps it would be best as an optional sub-module? That way the sub-module could have the dependency, but if you weren't interested in exportables there would be no change.

Edit: Also, to change to Entity API would just make it a different dependency would it not?

@nedjo: Worse yet, what happens if Entity API is subsequently disabled?

The use of extra database columns for exportables is a relatively recent addition to Entity API; see #1008810: Store all exportables in the db.

there's the need to add custom fields in hook_schema()

The actual required columns seem to be defined in entity_exportable_schema_fields(), and default to status and module. We can add those columns permanently to Wysiwyg's schema.

More investigation is required for the status column, as it seems like the values are totally not related to common "status" column values; they're describing the "origin" of the entity (code, custom, etc), but not its status (enabled, disabled, etc).

EDIT: I.e., we likely want to use a custom column name of "origin" (or similar) instead of "status".

what happens if Entity API is subsequently disabled?

In case Entity API is not available, we merely need to make sure that configurations, which normally cannot be edited ("locked") when Entity API is enabled, can't be edited either when it is disabled. Aside from that, I don't see any problems.

Version:7.x-2.x-dev» 6.x-2.2
Assigned:Unassigned» Andoni G
Category:feature» support
Priority:Normal» Major

Hi,

Someone knows if wysiwyg-624018-ctools-export-input-formats-2.patch runs well with Wysiwyg 6.x-2.2? I'm trying to do this but do not show me the Wysiwyg profile option when I create a new feature.

Thanks.

I forgot clear cache, now appears. ;)

Version:6.x-2.2» 7.x-2.x-dev
Assigned:Andoni G» Unassigned
Category:support» feature
Priority:Major» Normal
Status:Active» Needs work

No.

Status:Needs work» Postponed

Once #1034476: Load Wysiwyg profiles as entities is complete we should be able to add exportables via Entity API.

Title:CTools exportables and Features supportExportables and Features support
Issue tags:-CTools exportables+exportables

Entities API-based exportables is not something that can be backported to D6. Should D6 exportability really wait on that?

Status:Postponed» Needs work
Issue tags:-Features integration+features

#1034476: Load Wysiwyg profiles as entities has been committed. Let's move on here. :)

Many sites are still using Drupal 6, and I think the D6 versions of this patch are valuable.

More later.

Here are some first steps towards a patch.

* In hook_schema() and hook_update_x(), add the two fields used by Entity API for exportables. I've retained the field name "status". "origin" seemed inappropriate, since a particular setting might originate in code but have an override. "state" is probably better, but in any case this should be fixed if anywhere in entity.

* Add an 'exportable' key to hook_entity_info(). Also designate a 'label' entity key (since this is needed for Features to label the component being exported).

* Pull the controller class into an include that can be loaded conditionally.

With this in place the "WYSIWYG feature" type shows up under Features "Add components" and existing configurations can be selected and exported to a feature, but on my testing so far the exported code isn't loaded.

As well as fixing this issue, need to test for status (state) and offer a button label (revert) as appropriate. Part of that's in the patch in #117.

More generally, this is already a long issue. Let's reserve it for actual work on the D7 patch. When the fix is in for D7 anyone interested can return to consider if/how a clean D6 solution can be accomplished.

We'll probably need to change the way configuration is saved. Currently, when the main wysiwyg form is submitted, a record is created for each text format, even if the values are entirely empty. So e.g. if the form is submitted and later a module is enabled that provides a default WYSIWYG configuration for one of the existing text formats, the new default will already be overridden. To fix this we'll probably need to ensure we create records only if they are non-empty and different from the default. I included similar code in a draft patch for Captcha, #825088: Exportables support for CAPTCHA points.

#1047068: Redirect to profile edit form after assigning editor (and do not save upfront)

+++ wysiwyg.controller.inc 31 Jan 2011 23:13:02 -0000
@@ -0,0 +1,18 @@
+class WysiwygProfileController extends DrupalDefaultEntityController {
+++ wysiwyg.entity-controller.inc 31 Jan 2011 23:13:02 -0000
@@ -0,0 +1,18 @@
+class WysiwygProfileController extends EntityAPIController {
+++ wysiwyg.module 31 Jan 2011 23:13:02 -0000
@@ -19,28 +19,22 @@ function wysiwyg_entity_info() {
+if (module_exists('entity')) {
+  include_once('wysiwyg.entity-controller.inc');
+}
+else {
+  include_once('wysiwyg.controller.inc');
}

The manual inclusion of the include files needs to go. Instead, we need to rename the second controller to WysiwygProfileEntityController and dynamically assign either that or the other one as 'controller class' in hook_entity_info().

+++ wysiwyg.install 31 Jan 2011 23:13:02 -0000
@@ -32,6 +32,22 @@ function wysiwyg_schema() {
+      // The following two fields are used in exportables functionality via the Entity API module.
+      // See http://drupal.org/node/1021526.

Exceeds 80 chars, but let's remove the comment entirely.

+++ wysiwyg.install 31 Jan 2011 23:13:02 -0000
@@ -32,6 +32,22 @@ function wysiwyg_schema() {
+      'status' => array(
...
+        'description' => 'The exportable status of the entity.',

Hm. One bit item on the current roadmap is to allow to disable wysiwyg profiles (in correlation with allowing to attach more than one profile to a text format)... Will this status property allow us to mark a profile as disabled (regardless of Entity module or exported code)?

+++ wysiwyg.install 31 Jan 2011 23:13:02 -0000
@@ -32,6 +32,22 @@ function wysiwyg_schema() {
+        // Hard code the value of ENTITY_CUSTOM in case entity.module is not present.
+        'default' => defined('ENTITY_CUSTOM') ? ENTITY_CUSTOM : 0x01,

Let's always and unconditionally use the raw/internal value.

+++ wysiwyg.install 31 Jan 2011 23:13:02 -0000
@@ -32,6 +32,22 @@ function wysiwyg_schema() {
+      'module' => array(
...
+        'type' => 'varchar',
+        'length' => 255,
+        'not null' => FALSE,

Missing default, always required for 'not null' => FALSE

Powered by Dreditor.

+    foreach ($queried_entities as $key => $record) {
+      $queried_entities[$key]->settings = unserialize($record->settings);

If you have marked 'settings' as serialized in your schema, the entity API will take care of that for you.

@sun: fixes done. "Will this status property allow us to mark a profile as disabled (regardless of Entity module or exported code)?" By default, no. Possibly if we extend the bit flag defines? And yes, #1047068: Redirect to profile edit form after assigning editor (and do not save upfront) sounds exactly right.

@fago: thx, that cleans it up nicely.

#1047032: Imports fail for entity types without custom entity class--is_new flag not set is now committed (thx fago) so the dev version of Entity API is needed for testing. Opened another related issue on Entity API, #1047704: Add Entity API as a dependency when generating features. Until that's fixed it's necessary to manually add Entity API as a dependency when adding wysiwyg_profile items to a feature.

Version:7.x-2.x-dev» 6.x-2.3
Status:Needs work» Needs review
StatusFileSize
new20.61 KB

Hi,

I adapted the patch version http://drupal.org/files/issues/wysiwyg-624018-with-ui-4.patch (comment #100) to be compatible to the new Wysiwyg version 6.x-2.3.

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

@nedjo: Thanks for re-rolling!

@fago: Do you have any recommendation for us regarding (ab)use of the 'status' column/property? The goal is to allow wysiwyg profiles to be disabled (like many other exportables). It looks like Entity API's 'status' property bit flags only denote the "origin" and behavior of an entity (custom|database|in-code|default), but not really the common sense of an entity 'status' (i.e., "publishing status").

Hence, we'd either use a different column name than 'status' (perhaps something along 'export'), or we'd add and use a custom bit flag (e.g., ENTITY_ENABLED = 0x1).

>Hence, we'd either use a different column name than 'status' (perhaps something along 'export'), or we'd add and use a custom bit flag (e.g., ENTITY_ENABLED = 0x1).

I thought about doing that too already, however I don't see any benefit in adding it to the bit flag except for making querying for it *much* harder. That said, I think it would even break existing queries in the entity API required for managing entities in code upon module (un)installation (mark entities overridden, etc.). So best, just add a separate column. In Rules, I've used $rule->active = TRUE|FALSE.

Status:Needs review» Needs work

@fago

Thx for clarification. Further question. I added the 'module' key to wysiwyg_entity_info().

+++ wysiwyg.module 1 Feb 2011 17:33:38 -0000
@@ -13,14 +13,19 @@ function wysiwyg_entity_info() {
+    'module' => 'wysiwyg',

Is it needed for exportable support or can we drop it?

Also, note to self for next reroll:

+++ wysiwyg.install 1 Feb 2011 17:33:37 -0000
@@ -311,3 +327,26 @@ function wysiwyg_update_7200() {
+  // Create a created column.

Extraneous comment cut/pasted from elsewhere.

Powered by Dreditor.

>Is it needed for exportable support or can we drop it?
No, that's needed, e.g. for the features integration to properly populate the dependency.

ok, was equally concerned about query performance. Let's use a separate column then. However, we want to use 'status' for the actual 'status' (enabled/disabled) like all other core entities, so this Entity API "export status" needs to use a different and custom name. AFAICS, Entity API supports a custom name (perhaps untested? ;), so this leaves us with the question of a proper name. ;)

Options: export, export_status, export_state, state, entity_status, ...

In case there won't be better suggestions, I'd suggest to use either "export" or "entity_status". Actually I hope someone steps up with a better suggestion ;)

>However, we want to use 'status' for the actual 'status' (enabled/disabled) like all other core entities, so this Entity API "export status" needs to use a different and custom name.

Hm, I'd not say node.status or user.status is really the same, as they are not about enabling/disabling configuration.

Anyway for renaming, just set 'entity keys - status' to your key and adjust the schema accordingly. "export_status" would make sense to me.

Title:Exportables and Features supportExportables and Features support for 7.x

FYI, I just posted #1060846: Exportables and Features support for 6.x, since this issue is focused on a 7.x solution that can't be backported to 6.x.

Ideally we'd maintain consistency with other exportable component types.

Ctools exportables, and hence much of what's currently in features, uses a "disabled" property. E.g. for Strongarm variables:

<?php
  $strongarm
= new stdClass;
 
$strongarm->disabled = FALSE; /* Edit this to true to make a default strongarm disabled initially */
?>

Opened a related issue on Ctools and Entity API: #1072838: Harmonize exportables support and UI between Ctools and Entity API.

subscribing

StatusFileSize
new2.58 KB

Re-rolled #138 against the latest git, the only difference being a -1 offset.

Just to mention it, patch #138 appears to work rather well, perhaps someone could post a summary of the remaining issues so we can try to get it finished & committed ASAP? Thanks all.

subscribing

both the patch in #150 and #148 failed to apply properly for me. I'm no pro at applying patches though, so perhaps I did something wrong. 2 hunks applied, 1 failed. here is the contents of wysiwyg.module.rej for the patch in #150

*************** function wysiwyg_entity_info() {
*** 12,25 ****
    $types['wysiwyg_profile'] = array(
      'label' => t('Wysiwyg profile'),
      'base table' => 'wysiwyg',
-     'controller class' => 'WysiwygProfileController',
      'fieldable' => FALSE,
      // When loading all entities, DrupalDefaultEntityController::load() ignor$
      // its static cache. Therefore, wysiwyg_profile_load_all() implements a
      // custom static cache.
      'static cache' => FALSE,
      'entity keys' => array(
        'id' => 'format',
      ),
    );
    return $types;
--- 12,30 ----
    $types['wysiwyg_profile'] = array(
      'label' => t('Wysiwyg profile'),
      'base table' => 'wysiwyg',
+     'controller class' => module_exists('entity') ? 'EntityAPIController' : '$
      'fieldable' => FALSE,
      // When loading all entities, DrupalDefaultEntityController::load() ignor$
      // its static cache. Therefore, wysiwyg_profile_load_all() implements a
      // custom static cache.
      'static cache' => FALSE,
+     // Make exportable if the Entity API module is enabled.
+     'exportable' => TRUE,
+     'module' => 'wysiwyg',
      'entity keys' => array(
        'id' => 'format',
+       // Provides the label in e.g. features export.
+       'label' => 'format',
      ),
    );
    return $types;

Hi Gregory,

Make sure you're applying the patch to the dev version--it requires a patch that was applied but has not yet reached stable.

If you want to try out the results without patching, you could try Open Outreach, which includes a feature (debut_wysiwyg) based on this patch.

@GregoryHeller Yesterday I cleanly applied patch #150 against 7.x-2.x-dev. Something to note is that new patches from git require -p1 rather than -p0 like CVS patches. If you are using Drush Make you need to apply the patch at http://drupal.org/node/745224#comment-4302660 to support both formats. Hope this helps!

Aside from this the patch at #150 is working great so far. If I run into any issues I'll be sure to post. Very happy to see this long standing problem essentially solved!

Status:Needs work» Needs review

I will also say that the patch looks good. Code style looks good, the schema change is there on both install and via an update hook. I'm not familiar with Entity API but the changes are simple enough and it certainly work. Anyone else for a review and to mark RTBC?

subscribing

subscribing

subscribing

Title:Exportables and Features support for 7.xExportables and Features support for WYIWYG 7.x

Updating title...

Title:Exportables and Features support for WYIWYG 7.xExportables and Features support for WYSIWYG 7.x

Fixing typo...

Patch in #150 applies cleanly to latest dev, and everything seems to work with debut_wysiwyg.

I second #162, patch in #150 applies well and works.

#150 working fine for me, nice work guys!

Status:Needs review» Reviewed & tested by the community

Looks like RTBC. Change if incorrect.

trying to set a wysiwyg profile returned an error, which seems to come from Entity API, i think this is related

Fatal error: Class name must be a valid object or a string in MYSITE/includes/common.inc on line 7394

I was trying to set latest CKEditor for Filtered HTML

Issue tags:-wysiwyg

This patch is not finished. Try, for example:

  1. Enable a feature with a wysiwyg profile component
  2. Observe that the feature is not overridden
  3. Delete the profile in admin/config/content/wysiwyg
  4. Observe that the feature is now overridden
  5. Revert the feature
  6. Observe that the feature is still overridden, and your profile is not back
  7. Disable the feature
  8. Reenable the feature
  9. Observe that the profile is back

I will edit later with a better explanation [EDIT: posted #173 instead], and perhaps later a patch, but this issue should not be RTBC.

Status:Reviewed & tested by the community» Needs work

@quartsize: Thanks for the testing and feedback, very valuable.

It's not immediately clear whether the bug you're seeing indicates a problem in the patch or instead in Entity API's features support. On the face of it it sounds more like the latter.

@quartsize: *cough* or a bug in Features *cough* =)

I think I'm doing it wrong... I've applied the patch from #150 on the latest HEAD from the Features git repo (it applies nicely and I see the changes) but when I install the module in a clean version of D7 I'm not seeing the option for "Wysiwyg profile" when I create a new feature. Any ideas?

subscribing

Issue tags:+wysiwyg
StatusFileSize
new1.43 KB

Because we did not want to patch Wysiwyg and depend on Entity API we tried to implement the approach ZenDoodles proposed in #122: We implemented a small module with the sole purpose of exposing Wysiwyg profiles to features. You can find the module attached to this post. (Same approach as fe_block of features_extra, based on the code of features.filter.inc from features.)

Note however: Unfortunately a revert of a feature (i.e. “drush fr [wyswig_feature_name]”) does not get applied as long as you do not clear all caches (i.e. “drush cc all”). We are not sure about that, but as far as we can tell this seams like an issue of the features module.

@cossovich: have you enabled entity API module?

@nedjo, DamienMcKenna: (this is not a rhetorical question) is it a third-party's responsibility to clear wysiwyg's extra cache? Whoever's responsibility it is, I believe it must be taken care of before wysiwyg entity exportables can be called done.

Here's an updated patch; it uses entity_delete when entity API module is enabled. So entity API module's defaults get rebuilt, and the (feature-provided) profile does not go away. Revert also works, although wysiwyg's cache needs to be cleared in order to see the results.

But as I suggested above, that's not my only issue with this patch:

  • Because of how entity API module implements exportables, it appears that using its CRUD is mandatory, but this patch would leave Wysiwyg module updating it's own table with db_merge, bypassing entity API module's need to update its columns and keep the table consistent.
  • There are no UI changes in the patch. The actions of creating, deleting, overriding, and reverting (and enabling/disabling, if available) must be exposed to avoid confusion in the UI.
  • Were sun's concerns in #144, and nedjo's in #135, addressed?

Issue tags:+wysiwyg

Okay, so what's left to do before this can be closed?

subscribe

Updated patch: resets the columns Entity API module manages to their defaults (ENTITY_CUSTOM, no module) if Entity API module is disabled, ensuring that they remain valid if the rows are changed, or other modules enabled/disabled, before Entity API module is reenabled.

@cweagans: To do, in my opinion:

  • Refrain from updating the wysiwyg table without going through Entity API module (in particular, use entity_save()) when Entity API module is enabled, so that its expectations are met and it can manage its columns properly.
  • Decide on and implement changes to the UI when Entity API module is enabled, to make it less confusing. For example, currently, you can click to delete a code-only profile, the message tells you it was deleted, and it remains in the list. It's been reverted, but I'm not sure this is clear. Here's a probably incomplete list of ideas for changes:
    • Edit becomes override when profile is only in code.
    • Delete becomes revert when profile is overridden from code.
    • Show the name of the module that is providing a profile.
    • Don't offer to delete a profile that is only in code.
    • Reflect ENTITY_FIXED in whatever the appropriate way is.

    Though before doing this independently, it may be worth seeing if there's standard code in Entity API module for exposing a UI. In addition, the UI work should also address nedjo's concern in #135 (overriding all profiles when the main form is submitted).

  • Satisfy sun's concern in #144 (the naming of the status column).
  • Fix caching behavior. Currently, custom static and persistent caches are implemented in wysiwyg_profile_load_all, and are cleared by wysiwyg_profile_cache_clear. EntityAPIController does not (and cannot be expected to) know to call wysiwyg_profile_cache_clear when updating the wysiwyg table. This is okay if the wysiwyg table is only ever updated via Wysiwyg module. But by implementing Entity API module, we imply that entity_save('wysiwyg_profile', ...) should work just as well. EntityAPIController does call resetCache(), which could be overridden to clear the static and persistent caches, but it is not properly the controller's responsibility to clear a cache implemented externally. Moreover, entity_load('wysiwyg_profile', ...) would still ignore the cache. It makes the most sense to me to move the caching into the controller, but it looks like one would have to duplicate most of DrupalDefaultEntityController, which makes me a little hesitant.

Thanks for your work on this @IsaMic (comment #139), will this work with CTools 1.8 too?

ad #176:
While the entity API assumes you are using for full CRUD, I don't think you'll necessarily have to use it. However, yes then you'll have to care about setting the right exportable status yourself + calling entity_defaults_rebuild() when an entity got reverted/deleted.

>Though before doing this independently, it may be worth seeing if there's standard code in Entity API module for exposing a UI.
Beside the whole re-usable admin UI it provides theme functions for the exportable entity status and one for the label that also shows the machine name.

Subscribe

I tried the patch in #176, but I'm getting a PDO exception when I go to the WYSIWYG configuration:

PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'base.status' in 'field list': SELECT base.format AS format, base.editor AS editor, base.settings AS settings, base.status AS status, base.module AS module FROM {wysiwyg} base

WYSIWYG 7.x-2.x
Entity API 7.x-1.0-beta8

@FilmKnurd: I'm having trouble reproducing this error; did you run the database update?

@quartsize: Ahhhhh, rookie error. Yeah, that's what I failed to do :-)

Same patch as #176, but manually (sigh) edited to be applied using drush make.

Patch in #176 seems to be working ok in my pretty limited testing.

Subbing

As per the beta9 release of Entity module;

"There are important API changes for all modules that implement exportable entities with the help of the entity API module. Short, entity_load() now always returns numeric ids (instead of names as it was the case previously) and exportables have to use the now separated EntityAPIControllerExportable class."

Patch #183 has been edited to include the above change.

Tested the patch in #186. It fixes the problem with Entity API and works as good as past versions (I believe I was still using #150). I'm able to use "drush features update" to push WYSIWYG back into the feature. Reverting (tested through the UI) doesn't work properly. The feature will say that differences no longer exist but upon checking WYSIWYG settings show that nothing has actually been reverted.

Patch in #186 did the trick.

Sorry, i tried to apply the patch #186 but had no luck... probably i missed something previously in discussion?

i am using wysiwyg-6.x-2.3. Downloaded the patch file in my sites/all/modules/wysiwyg/ folder and launched the command
patch -p0 <wysiwyg-entity-exportables-624018-176_1.patch

this is what i got:

patching file wysiwyg.admin.inc
Hunk #1 FAILED at 549.
1 out of 1 hunk FAILED -- saving rejects to file wysiwyg.admin.inc.rej
patching file wysiwyg.install
Hunk #1 succeeded at 26 (offset -4 lines).
Hunk #2 succeeded at 302 with fuzz 2 (offset -24 lines).
patching file wysiwyg.module
Hunk #1 FAILED at 12.
Hunk #2 FAILED at 140.
Hunk #3 FAILED at 666.
3 out of 3 hunks FAILED -- saving rejects to file wysiwyg.module.rej

thanks for helping me out :)

subscribing

@joeysantiago are you using beta9 release of entity module? http://drupal.org/node/1190594

@joeysantiago The patch is only for Drupal 7 and will not be backported as Drupal 6 does not have Entity API or input formats with machine names. #1060846: Exportables and Features support for 6.x is the issue you want.

subscribing

subscribe

+1

+1

subscribe

subscribing

about to test patch in #186, but it didn't apply for me the way patches usually do, so I edited that (changes to file headers using the a/ b/ format)

so here's a new version:

reporting that #199 patch is working fine for me.

What to do with the Entity API dependency? how can a user know he has to enable Entity AP to make the wysiwyg module settings exportable?

Patch #199 is working for me too. Using entity-7.x-1.0-beta10 and wysiwyg-7.x-2.1. Is necessary to run update.php after patch to work.

sub

Just arriving in this thread.. Its massive. edit - just read above post

sub.

On Drupal 7.7, using the patch from #199, I was greeted by this error message when I went to recreate an existing feature:

PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'base.status' in 'field list': SELECT base.format AS format, base.editor AS editor, base.settings AS settings, base.status AS status, base.module AS module FROM {wysiwyg} base; Array ( ) in EntityAPIController->query() (line 152 of /home/sfyn/www/D7/dev-drupal-7.7-build-1/sites/all/modules/contrib/entity/includes/entity.controller.inc).

Status:Needs work» Needs review

patch in #199 works fine.

@sfyn, you have to run update.php as the database table changes for that patch.

Added some lines to the README, so the weak dependency on entity is documented. Patch atttached.

You can export wysiwyg profiles by using Features. Therefore you also have to install the Entity module.

This wording implies that Entity is a (non-weak) dependency.

How does the attached wording look?

Status:Needs review» Needs work

derhasi had changed the status; I believe the to-do list in #176 still stands.

subscribe

Status:Needs work» Needs review
StatusFileSize
new1016 bytes

Sorry for being late to the party, but after we had the entity based approach in use for a couple of weeks, I've come to the conclusion that entities are NOT the right way to export WYSIWYG profiles. First, they're 100% configuration, and not something you'll want to work with e.g. inside Views (where they're currently appearing).
Another issue were stalled configurations (probably a bug in the Entity API), where I had changed WYSIWYG a profile, but those changes were not reflected in the re-exported feature.
Yet another issue was dependency management: something that Features support, Entitiy API doesn't, so you have to be careful to also select the text format ("filter format") whenever you want to export a WYSIWYG profile through entities.

So I've taken the initial code from @moritzz posted in #172 and turned it into a simple features drop-in – no need for a separate module.

Here's the patch from #208 formatted for drush make's "-p0" method.

ad #211:

>First, they're 100% configuration, and not something you'll want to work with e.g. inside Views (where they're currently appearing).

A wysiwyg profile is already an entity, not? Also, I don't see why it shouldn't. Views integration for exportables might be useful in some cases and as comes for free, why not have it? Still, you can easily disable it if you prefer.
Also see the related issue #1239062: determine entity types being configuration.

>Another issue were stalled configurations (probably a bug in the Entity API), where I had changed WYSIWYG a profile, but those changes were not reflected in the re-exported feature.

I never ran into that. Feel to create an entity api issue for tracking the bug down though!

>Yet another issue was dependency management: something that Features support, Entitiy API doesn't, so you have to be careful to also select the text format ("filter format") whenever you want to export a WYSIWYG profile through entities.

Why should the entity API handle dependency management? You mean its Features integration? Currently, you'll have to care about piping through your dependencies yourself.

Status:Needs review» Needs work

I've been using the patch at #208.
it works fine for exporting a profile, but once its exported, that it.
If you to update the feature via Drush, all customisations to the profile are lost.

Subbing.

What's the chances of this getting finished and into the module soon?

Subscribing

If you to update the feature via Drush, all customisations to the profile are lost.

This actually seems to work fine for me, using the patch in #212 and these module versions: WYSIWYG 7.x-2.1, Entity 7.x-1.0-beta10 and Features 7.x-1.0-beta3

I tested the following:

  • I created a wysiwyg profile and exported it into a feature
  • Then I changed something in the profile; the Features module correctly reported the change (status overridden) and drush fu featurename brought the change into feature code as expected.
  • Then I changed something in the feature code (removed a button); the Features module again correctly reported the difference (status overridden), and a click on "Revert Component" updated the Settings stored in DB to what I have stored in-code.

So, assuming that entities are still the right approach (#211,#213), is quartsize's todo in #176 still the current status for this issue?

----------

Edit: scap that, it doesn't quite work for me either... not sure what was different when I tested before. Now everything I change in the UI (i.e. have in the DB) gets wiped everytime I clear the cache or try a drush feature-update.

Subscribe

Subscribing

Patch in #212 does not work with WYSIWYG 7.x-2.1

subsrcibe

subscribing

StatusFileSize
new3.51 KB

attached is #211 as a patch instead of zip.

subscribing

Subscribing...thanks for everyone's hard work on this.

@njcheng Please use the new Follow button at the top of an issue to subscribe to it.

Using patch in #223 I am not seeing wysiwyg settings applied upon turning on the feature. No error messages in watchdog.

I've tried the patch in #223 and it's working well for me.

Patch #223 works for me too with wysiwyg-7.x-2.1, all settings to wysiwyg profile were applied.

Status:Needs work» Reviewed & tested by the community

Besides that the .module part needs to be applied manually the patch works just fine with the current dev version.

Status:Reviewed & tested by the community» Fixed

#211 convinces through simplicity.

Added undocumented include 'file' info to the API hook to facilitate auto-loading, adjusted to account for the edge-case of profiles mapping to no format, and committed and pushed to 7.x-2.x.

@sun: Hm, can't seem to find what repo it got committed to. Looking at http://drupalcode.org/project/wysiwyg.git nothing has been committed to 7.x-2.x for 3 months.

Ahem, forgot to push - thanks ;)

Status:Fixed» Closed (fixed)

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

Here's a drush_make patch that should apply cleanly to 7.x-2.1 for anyone who needs it

And that's a re-roll of #223/#211, for the record (the non-entity approach)

This seems like a fairly important/exciting feature that might be worth another release? Personally, I will be pulling the repos at this commit just for this feature.

Status:Closed (fixed)» Needs review

Reopening per #237.

Status:Needs review» Closed (fixed)

This patch - http://drupal.org/node/624018#comment-5278644 - worked excellently for me - thanks very much.

Reroll of patch in comment #183 to be applied to latest dev (commit 83476bf)

And here is the patch :-)

@MiSc: this issue is marked as closed and fixed, it's therefore very unlikely that someone will look at your patch. Besides, support for exportables has been a part of of wysiwyg since almost 10 months. If there's a bug or something missing in the current implementation you should open up a new issue!

@bforchhammer, interesting, when using the latest dev, exportables did not work with my setup, but with the patch it does, so maybe I have a problem somewhere else. Anyhow, this solves it for me :-). But I do not need a review of this, just somewhere to place my patch so I could use it in my make file.

@MiSc: Strange, I just updated wysiwyg from 2.1 to current 2.x-dev on my local test machine and got it working. It adding a new item "Wysiwyg profiles: wysiwyg" in the "Edit components" drop down menu on the admin/structure/features/create tab.

features export of wysiwyg profiles in dev works for me too.

Duh. It was a problem in my make file, I thought I checked out revison 83476bf, but I also after that checked out a older version. So do not bother about my patch. Sorry.