Most settings available for tweaking through the editor profile configuration GUI are currently implemented in wysiwyg.admin.inc even though most editors/integrations do not support them all. The widgets and settings descriptions available in the WysiwygGUI were originally based on the settings offered by TinyMCE and are thus very specific to that editor.
This is becoming a problem since the default state of the profile GUI has lots of widgets not relevant for many editors, confusing users as to which settings are actually possible to change. Editors not supporting some settings will simply not react in any way no matter which value a settings widget in the GUI is changed to, a major UX WTF!

Since #1650416: Allow editor specific changes to be made to the profile settings form was committed (7.x-2.2), it's possible to implement a callback (settings form callback), defined in the editor metadata, for overriding the form generated by wysiwyg.admin.inc.

I intend to move as many settings as possible from wysiwyg.admin.inc out to the individual wysiwyg/editors/editorname.inc files and update the internal names of the settings to actually match those used by the editor (makes for saner code int the editor .inc files). Settings widgets which are not relevant for an editor will simply no longer exist or be visible for that editor.

Existing editor profiles will be updated to use the new names to avoid having to support two names for the same setting. New editor profiles will automatically use the correct names. This will not affect modules implementing hook_wysiwyg_editor_settings_alter() because the editor .inc files are already converting the names of working settings (or the editors wouldn't understand them), and that hook has to expect the native names of settings for each editor to make sense anyway. This means you'll see PHP notices about missing settings if you update the code and forget to run update.php. The notices are harmless in themselves, but if you forget to run update.php before editing an editor profile, some settings will revert to their default values.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TwoD’s picture

Status: Active » Needs review
FileSize
45.34 KB

And here's a first patch for 7.x-2.x-dev.
I've tried hard to move as much as possible without affecting the parts of the GUI which should stay the same for each editor, as in the widgets which do actually work.

Notable things which this does change include:

  • Some editors implementations do not [yet] support controlling the toolbar at all, so the whole "Buttons and plugins" list is disabled for them.
  • Several editors implementations support significantly fewer CSS related settings (if any at all), so several or all widgets have been disabled for them.
  • The same goes for settings controlling the generated output, such as whether to indent the generated markup.
  • Some of the widgets work slightly different for some editors, and I've tried to compensate for that in the form element descriptions.
  • Some paste related TinyMCE/CKEditor/FCKeditor options have been moved to their own vertical tab, to try to clearly indicate these belong to a plugin. My intention is to later let [external] plugins add settings in a similar fashion. They can theoretically already do that that by implementing hook_form_alter(), but that does not give Wysiwyg any control over when/where plugin settings are shown. Ideally, I only want settings for enabled plugins to show.
  • A few [CSS] settings have been disabled for some of the less popular editors even though there is support for them in the *.inc file, but the clientside isn't able to actually use the value of the setting. If you're using these settings, you're already in a world of hurt so this should not disrupt things for the average user. Note that the actual value for the disabled settings isn't being removed from the editor profile during the update, and it'll still be passed to the editor. so editors will keep behaving the same until you save the profile configuration form again - at which point you'll notice there's no widget for that setting any longer.
TwoD’s picture

Status: Needs review » Needs work
+++ b/editors/ckeditor.incundefined
@@ -171,6 +221,15 @@ function wysiwyg_ckeditor_settings_form_validate_css_classes($element, &$form_st
+  form_set_value($form['css']['block_formats'], preg_replace('@\s+@', '', $element['#value']), $form_state);

Forgot to pass $element instead of the $form array in these validation handlers so saving the form throws errors.

I'm doing things a bit different for the next iteration of this patch so it won't look exactly the same anyway.

TwoD’s picture

Status: Needs work » Needs review

Large update, some highlights:

  • Moved out more settings from the default profiles form, including some validation code which was specific to some editors.
  • Added links to most of the editor options used internally by Wysiwyg.
  • The settings widget names now reflect the actual editor setting used internally by Wyiwyg. The update hook translates the old names for existing profiles.

I could use some help testing this and verify all settings changes are correctly forwarded to the editors. But, since there are other issues depending on this one (most notably the TinyMCE 4 support), I will commit this once those need to go in.

TwoD’s picture

FileSize
56.97 KB

And the patch...

TwoD’s picture

+++ b/editors/tinymce.incundefined
@@ -127,6 +128,158 @@ function wysiwyg_tinymce_themes($editor, $profile) {
+        '@url' => url('http://www.tinymce.com/wiki.php/Configuration3x:apply_source_formatting'),

Typo, will remove this.

UPDATE: Also missed a call to url() in tinymce.inc.

TwoD’s picture

FileSize
56.84 KB

Fixed the above notes, otherwise the same patch.

Some default values for new profiles are still missing.

Help with testing the upgrade hooks and various editor versions would also be greatly appreciated.

TwoD’s picture

FileSize
57.45 KB

Added the last default values for some settings in a few editors.
Removed duplicate code from YUI.
Removed a comment from CKEditor's form callback since it no longer reflects the only thing it did earlier. Switched to using the landing URL for the settings reference link in the comment.
Some minor comments removed since they didn't add anything.

rocketeerbkw’s picture

@@ -275,23 +327,28 @@ function wysiwyg_ckeditor_settings($editor, $config, $theme) {
+  $check_if_set = array(
+    'apply_source_formatting',
+    'forcePasteAsPlainText',
+    'language',
+  );

apply_source_formatting isn't used in ckeditor

@@ -275,23 +327,28 @@ function wysiwyg_ckeditor_settings($editor, $config, $theme) {
+  if (isset($config['toolbarLocation'])) {
+    $settings['toolbarLocation'] = $config['toolbarLocation'];
   }

Move this up to $check_if_set?

Patch applied cleanly, and the update function worked. I only tested ckeditor, but looking over the rest I didn't catch anything wrong.

TwoD’s picture

Status: Needs review » Needs work

Good catches! apply_source_formatting is actually used by the JavaScript side of the CKEditor integration to trigger some tweaks to the DataProcessor's Writer class, but I forgot to copy the form element into the CKEditor settings form callback so there's no way to toggle it now. Should probably also rename it to something else while we're at it.

I'd like to expand the source formatting options later, but that's for another issue.

Yes, we should move the toolbarLocation check, thanks. Will re-roll ASAP.

TwoD’s picture

Status: Needs work » Fixed
FileSize
59.01 KB

Fixed the last points above and committed the attached patch to 7.x-2.x.

Commit: 5bf5886

Thanks for your help, this was much needed!

I've tested all of these changes very carefully, but if something was indeed broken, let's deal with that in a follow-up issue.

Status: Fixed » Closed (fixed)

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

TwoD’s picture

Backported to 6.x-2.x.