Using Features v7.x-2.3 and WYSIWYG v7.x-2.2+54-dev (2014-10-18), when you modify a WSYWIYG profile it does not export properly when doing "features-update".

The full scenario:

  • Have an existing site with CKEditor v3.5.x and the above releases of Features and WYSWYG, with the current configuration exported to a feature which is also enabled..
  • Update to the latest CKEditor v4.
  • Go to the profile page for the text format that CKEditor is available for.
  • Make adjustments, hit save.
  • Use "drush features-update [featurename]" to update the wysiwyg configuration.
  • Deploy the changes to another instance of the site and revert the feature.
  • Load up a node/add/* form, the WYSIWYG will not be correct, there will be differences versus the original instance where the changes were applied.
  • On the second site instance, load up the same WYSIWYG profile edit page, hit save.
  • The changes will now properly take effect on the node/add/* form.

I've seen this happen where a button for the Footnotes module was not being disabled until after going back into the profile edit form and hitting save again.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

DamienMcKenna’s picture

Here's a test scenario:

$x = 'a:21:{s:7:"default";i:1;s:11:"user_choose";i:0;s:11:"show_toggle";i:1;s:5:"theme";s:8:"advanced";s:8:"language";s:2:"en";s:7:"buttons";a:4:{s:7:"default";a:36:{s:4:"Bold";i:1;s:6:"Italic";i:1;s:9:"Underline";i:1;s:6:"Strike";i:1;s:13:"JustifyCenter";i:1;s:12:"JustifyRight";i:1;s:12:"JustifyBlock";i:1;s:12:"BulletedList";i:1;s:12:"NumberedList";i:1;s:7:"Outdent";i:1;s:6:"Indent";i:1;s:4:"Undo";i:1;s:4:"Link";i:1;s:6:"Unlink";i:1;s:6:"Anchor";i:1;s:11:"Superscript";i:1;s:9:"Subscript";i:1;s:10:"Blockquote";i:1;s:6:"Source";i:1;s:14:"HorizontalRule";i:1;s:3:"Cut";i:1;s:4:"Copy";i:1;s:5:"Paste";i:1;s:9:"PasteText";i:1;s:13:"PasteFromWord";i:1;s:10:"ShowBlocks";i:1;s:11:"SpecialChar";i:1;s:6:"Format";i:1;s:4:"Font";i:1;s:8:"FontSize";i:1;s:6:"Styles";i:1;s:5:"Table";i:1;s:9:"SelectAll";i:1;s:5:"Flash";i:1;s:9:"CreateDiv";i:1;s:6:"Iframe";i:1;}s:9:"footnotes";a:1:{s:9:"footnotes";i:0;}s:3:"swf";a:1:{s:5:"Flash";i:1;}s:6:"drupal";a:1:{s:5:"media";i:1;}}s:11:"toolbar_loc";s:3:"top";s:13:"toolbar_align";s:4:"left";s:8:"path_loc";s:6:"bottom";s:8:"resizing";i:1;s:11:"verify_html";i:1;s:12:"preformatted";i:0;s:22:"convert_fonts_to_spans";i:1;s:17:"remove_linebreaks";i:0;s:23:"apply_source_formatting";i:1;s:27:"paste_auto_cleanup_on_paste";i:0;s:13:"block_formats";s:13:"p,h3,h4,h5,h6";s:11:"css_setting";s:4:"self";s:8:"css_path";s:86:"%bsites/all/themes/catalyst/css/reset.css, %bsites/all/themes/catalyst/css/wysiwyg.css";s:11:"css_classes";s:0:"";s:15:"ckeditor_styles";a:1:{s:16:"stylesheetparser";i:0;}}';
$x = unserialize($x);
ksort($x);
kpr($x, FALSE, 'Scenario A');

$y = 'a:20:{s:7:"default";i:1;s:11:"user_choose";i:0;s:11:"show_toggle";i:1;s:16:"add_to_summaries";i:1;s:5:"theme";s:8:"advanced";s:8:"language";s:2:"en";s:7:"buttons";a:3:{s:7:"default";a:36:{s:4:"Bold";i:1;s:6:"Italic";i:1;s:9:"Underline";i:1;s:6:"Strike";i:1;s:13:"JustifyCenter";i:1;s:12:"JustifyRight";i:1;s:12:"JustifyBlock";i:1;s:12:"BulletedList";i:1;s:12:"NumberedList";i:1;s:7:"Outdent";i:1;s:6:"Indent";i:1;s:4:"Undo";i:1;s:4:"Link";i:1;s:6:"Unlink";i:1;s:6:"Anchor";i:1;s:11:"Superscript";i:1;s:9:"Subscript";i:1;s:10:"Blockquote";i:1;s:6:"Source";i:1;s:14:"HorizontalRule";i:1;s:3:"Cut";i:1;s:4:"Copy";i:1;s:5:"Paste";i:1;s:9:"PasteText";i:1;s:13:"PasteFromWord";i:1;s:10:"ShowBlocks";i:1;s:11:"SpecialChar";i:1;s:6:"Format";i:1;s:4:"Font";i:1;s:8:"FontSize";i:1;s:6:"Styles";i:1;s:5:"Table";i:1;s:9:"SelectAll";i:1;s:5:"Flash";i:1;s:9:"CreateDiv";i:1;s:6:"Iframe";i:1;}s:3:"swf";a:1:{s:5:"Flash";i:1;}s:6:"drupal";a:1:{s:5:"media";i:1;}}s:15:"toolbarLocation";s:3:"top";s:14:"resize_enabled";i:1;s:24:"default_toolbar_grouping";i:0;s:24:"simple_source_formatting";i:0;s:8:"acf_mode";s:1:"0";s:19:"acf_allowed_content";s:0:"";s:11:"css_setting";s:4:"self";s:8:"css_path";s:86:"%bsites/all/themes/catalyst/css/reset.css, %bsites/all/themes/catalyst/css/wysiwyg.css";s:9:"stylesSet";s:0:"";s:13:"block_formats";s:13:"p,h3,h4,h5,h6";s:20:"advanced__active_tab";s:10:"edit-basic";s:21:"forcePasteAsPlainText";i:0;s:15:"ckeditor_styles";a:1:{s:16:"stylesheetparser";i:0;}}';
$y = unserialize($y);
ksort($y);
kpr($y, FALSE, 'Scenario B');

Scenario A is the {wysiwyg}.settings value from the second instance after deploying the changes and then using the command "drush fr -y [featurename]" to load them onto the server. Scenario B is the same database record after manually re-saving the same form. Note that there are a bunch of different values in the array.

DamienMcKenna’s picture

Priority: Normal » Major

Bumping this to major as it indicates that exportables just are not working correctly.

TwoD’s picture

The dumps you posted are valid profiles, but it looks to me like the feature wasn't updated at all.

Your scenario A is missing the 'add_to_summaries' and 'default_toolbar_grouping' keys introduced a while ago in Wysiwyg 7.x-2.x-dev (the only version supporting CKEditor 4), suggesting it's from Wysiwyg 7.x-2.2.

Those keys are present in the second scenario, with the exception of the footnotes plugin (likely because it wasn't detected).

Do you have the feature code itself, as it was created from the original site after the profile was saved (to apply profile changes) and 'drush features-update' was run?

TwoD’s picture

I just created a new feature, noted its state was "default" after being enabled, modified the profile, state changed to "overridden", recreated the feature, state changed to "default".

I didn't have a second site to test with, and I recreated the feature using the Features UI instead of Drush, but the result should be the same since Wysiwyg won't know the difference.

kalinchernev’s picture

+1 for having issues with features exports from wysiwyg.
Digging quite deep, I am prone to propose an update in either wysiwyg_features_revert() or wysiwyg_features_rebuild() hooks, which are responsible for applying the exported configurations to the target staging/prod environments.

Reasoning
- wysiwyg utilizes features hooks to export *.features.wysiwyg.inc file. Exported profile is (object)
- wysiwyg_features_revert() is directly calling wysiwyg_features_rebuild(). Both hooks are used to apply (object) to database. These are features module hooks, which are not alterable by other modules.
- wysiwyg_features_rebuild() is assuming that $profile is an array, not an (object). Thus, rebuild and revert on target instance can never happen.

I tried:
- finding alter hooks on hook_features_revert() and hook_features_rebuild(), didn't find any. I cannot intercept in between to turn the $profile into array.
- re-export via different methods to do the stupidity test. Both drush and browser are using came way to export $profile.

My idea about a quick-fix is the casting just before the query at wysiwyg_features_rebuild(). I believe it relates to the issue as the rebuild and revert features are in the base of the configurations migration.

kalinchernev’s picture

Status: Active » Needs review
TwoD’s picture

Status: Needs review » Postponed (maintainer needs more info)

I forgot to mention I also reverted and had the feature rebuilt. Everything works well in my tests.

@kalinchernev, Wysiwyg profiles are not exported as objects. I'm not sure how you arrived to that result.

Here is a sample created with Features 2.x and the current Wysiwyg 7.x-2.x (code in wysiwyg.features.inc is identical to 7.x-2.2):

/**
 * @file
 * foo.features.wysiwyg.inc
 */

/**
 * Implements hook_wysiwyg_default_profiles().
 */
function foo_wysiwyg_default_profiles() {
  $profiles = array();

  // Exported profile: cke
  $profiles['cke'] = array(
    'format' => 'cke',
    'editor' => 'ckeditor',
    'settings' => array(
      'default' => 1,
      'user_choose' => 0,
      'show_toggle' => 1,
      'add_to_summaries' => 1,
      'theme' => '',
      'language' => 'en',
      'ss' => 2,
      'buttons' => array(
        'default' => array(
          'Bold' => 1,
          'Italic' => 1,
          'Underline' => 1,
          'Strike' => 1,
          'JustifyLeft' => 1,
          'JustifyCenter' => 1,
          'JustifyRight' => 1,
          'JustifyBlock' => 1,
          'BidiLtr' => 1,
          'BidiRtl' => 1,
          'BulletedList' => 1,
        ),
      ),
      'toolbarLocation' => 'top',
      'resize_enabled' => 1,
      'default_toolbar_grouping' => 0,
      'simple_source_formatting' => 0,
      'acf_mode' => 0,
      'acf_allowed_content' => '',
      'css_setting' => 'theme',
      'css_path' => '',
      'stylesSet' => '',
      'block_formats' => 'p,address,pre,h2,h3,h4,h5,h6,div',
      'advanced__active_tab' => 'edit-security',
      'forcePasteAsPlainText' => 0,
    ),
    'rdf_mapping' => array(),
  );

  return $profiles;
}

(This may also expect a foo.features.filter.inc file with the corresponding filter.)

Start with the profile in the same state as the exported on-file profile.

Test 1, reverts:

  1. The feature is now 'Default'.
  2. Disable the button 'Bold' in the profile UI.
  3. The feature is now 'Overridden'.
  4. Revert the feature.
  5. The 'Bold' button is enabled again'.

This calls wysiwyg_features_revert() and then wysiwyg_features_rebuild(). The $profile variable is an array as it's read directly from the file. (Tested using Devel's dd().)

Wysiwyg implements 'faux-exportables' as it always needs database entries for profiles and can't use the on-file versions directly.
'Faux-exportables' use hook_features_rebuild() to be told when the on-file data has changed so the DB entries can be updated.

Test 2, restores:

  1. The feature is now 'Default'.
  2. The 'Strike' button is enabled in the profile UI.
  3. Comment/remove the 'Strike' => 1, line in foo.features.wysiwyg.inc.
  4. Look at the profile UI again (update the page), 'Strike' is still enabled.
  5. Visit the Features overview. It will say 'Checking' and then 'Default' after a short delay.
  6. Look at the profile UI again, 'Strike' is now disabled, without manual changes to the profile.

The act of visiting the Features overview page triggers Features to call hook_features_rebuild() directly, if the on-file data has changed compared to the cached version, and it's not 'Overridden'. (Clearing the cache has the same effect if enabled in Features.)
Again, the $profile variable is an array because it gets the value from the exported file.

anthonylindsay’s picture

Similar to the above comment in #7, I've experienced the following:

  • I create a wysiwyg profile and export it to a Feature.
  • I make a change through the UI to override the Feature.
  • I do a cache clear
  • Boom - the Feature is reverted and the changes made through the UI are gone.

This was with Features 2 and Wysiwyg 7.x-2.x-dev

TwoD’s picture

@anthonylindsay, that should not happen. Features should not auto-revert unless you have some other module which does it for you. I've seen some sites do it as they control every single change via code pushes (they often don't even have the UI modules enabled) so in case something did change any configuration

Features does auto-rebuild non-overridden features if the exported file has changed on cache-clear, if the setting to do that is enabled. Compared to the above use case, this would happen when devs do push up new features code and does a cache clear, so the changes get applied even if they forget to manually revert changes after the code update. This would theoretically work if you don't have the UI enabled (unless it isn't actually implemented in the Features UI module), only have FTP-access to your server, and could not use drush to revert features after uploading new code.

Either case, Wysiwyg itself does not control when a revert or rebuild happens, it just acts as ordered.

HLopes’s picture

I'm seeing the same behaviour described in #5.

The problem is this cast:

  $profiles['block_html'] = (object) array(

Removing it manually makes the problem go away.

Features version: 7.x-2.9
Wysiwyg version: 7.x-2.2

This is what I obtain after using the Features UI to create a feature.

<?php
/**
 * @file
 * some_random_feature_export.features.wysiwyg.inc
 */

/**
 * Implements hook_wysiwyg_default_profiles().
 */
function some_random_feature_export_wysiwyg_default_profiles() {
  $profiles = array();

  // Exported profile: block_html
  $profiles['block_html'] = (object) array(
    'format' => 'block_html',
    'editor' => 'ckeditor',
    'settings' => array(
      'default' => 1,
      'user_choose' => 0,
      'show_toggle' => 1,
      'theme' => 'advanced',
      'language' => 'en',
      'buttons' => array(
        'default' => array(
          'Bold' => 1,
          'BulletedList' => 1,
          'NumberedList' => 1,
          'Link' => 1,
          'Unlink' => 1,
          'Anchor' => 1,
          'Cut' => 1,
          'Copy' => 1,
          'Paste' => 1,
        ),
      ),
      'toolbar_loc' => 'top',
      'toolbar_align' => 'left',
      'path_loc' => 'bottom',
      'resizing' => 1,
      'verify_html' => 1,
      'preformatted' => 0,
      'convert_fonts_to_spans' => 1,
      'remove_linebreaks' => 1,
      'apply_source_formatting' => 0,
      'paste_auto_cleanup_on_paste' => 0,
      'block_formats' => 'p,address,pre,h2,h3,h4,h5,h6,div',
      'css_setting' => 'theme',
      'css_path' => '',
      'css_classes' => '',
    ),
    'rdf_mapping' => array(),
  );

  return $profiles;
}

HLopes’s picture

First stab at a patch to fix this.

DamienMcKenna’s picture

Status: Postponed (maintainer needs more info) » Needs review
TwoD’s picture

Status: Needs review » Postponed (maintainer needs more info)

I can not reproduce this in the current 7.x-2.x-dev branch. A couple of features related fixes have been committed which I hope should fix this as well.

In my tests so far there has been no need for the explicit array cast. Profiles are exported as arrays and loaded as arrays with Features 7.x-2.10. There is no cast to an object as seen in the sample from #10.

Can anyone still reproduce this? If not, I'll assume it's been fixed.

HLopes’s picture

The problem seems to originate from this patch, as mentioned on the comment after that one.

TwoD’s picture

Status: Postponed (maintainer needs more info) » Needs review

Ok, so once that patch gets into Features we would have a problem in Wysiwyg, which the patches here attempt to fix. That explains why I couldn't reproduce it. Thank you @HLopes.

Will see if I can verify this ASAP.

  • TwoD committed c55e14e on 7.x-2.x authored by HLopes
    - #2414575 by HLopes, kalinchernev, DamienMcKenna, anthonylindsay: Fix...
TwoD’s picture

Status: Needs review » Fixed

This does look like a useful precaution in case that patch is being used (or Features ever commits it), so I have added it to 7.x-2.x.

Thank you all!

Status: Fixed » Closed (fixed)

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