No CSS classes are passed to the Font style dropdown of the CKeditor. Patch in attachment fixes this.

1 gotcha (should be changed in the help text):
in the css classes field (in the profile) you need to define the classes, the format is:
[title]=[element].[class]
eg.
Subtitle=div.subtitle

when the css classes field is left empty, no font styles are parsed
(as CKeditor expects a name and element to link to the class, some regexp voodoo will be needed)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chellman’s picture

This mostly worked for me. I'd modify the helper function so it skips empty lines (if there are trailing newline in the css classes field).

There's coding standards cleanup to be done here too in a few places. I can reroll this, but I'd rather know what you mean about the regexp voodoo you're thinking of if the field is empty. What's expected to happen in that case?

function _wysiwyg_ckeditor_get_stylesCss($css_classes) {
  $lines = explode("\n", $css_classes);
  foreach ($lines as $line) {
    // skip any empty lines
    if (empty($line)) continue;
    
  	$l = explode("=", $line);
  	$lc = explode(".", $l[1]);
  	
    $style['name'] = $l[0];
    $style['element'] = $lc[0];
    $style['attributes']['class'] = $lc[1];
   
    $css_styles[] = $style;
  }
  
  return $css_styles;     	
}
g10’s picture

to clarify the 'regexp voodoo' :
which this line hints at:
// @todo also parse the css when no classes are defined

if the css classes field is empty, but a css file is selected, the css file could be read and parsed, checking which classes are in the css file… here comes the 'regexp voodoo' into play for parsing/finding [element].[class] eg. div.title…

but maybe this is too far fetched? simply require to fill in the css classes field if one selects a custom css file could/should be enough? (at least it is better opposed to no fontStyles)

sdelbosc’s picture

Just to track the issue...

echoz’s picture

subscribe

pacproduct’s picture

Hi,

g10's patch works fine for me too, but only for the "Define CSS" option.
I've added some lines to his patch to make it work for the "Use theme CSS" option.

Note: These changes only apply for CKEditor version >= 3.0.1.4391, as I'm not sure how it should be implemented for prior versions.

The attached patch contains g10's changes.

TwoD’s picture

re #2, CKEditor already parses all the stylesheets added via the contentsCSS setting, no need for us to duplicate that work.

This patch leaves the combo box empty for me, throws the error "CKEDITOR.stylesSet is undefined" and it tries to load a JS file called "custom_styles.js".

+++ ./ckeditor.inc	2010-04-19 09:30:52.348360100 +0200
@@ -169,9 +169,21 @@ function wysiwyg_ckeditor_settings($edit
+
+        // @todo also parse the css when no classes are defined
+        if($config['css_classes']) {
+          $settings['customStylesName'] = "custom_styles";
+          $settings['stylesCss'] = _wysiwyg_ckeditor_get_stylesCss($config['css_classes']);
+        }
       }
       elseif ($config['css_setting'] == 'self' && isset($config['css_path'])) {
         $settings['contentsCss'] = explode(',', strtr($config['css_path'], array('%b' => base_path(), '%t' => path_to_theme())));
+
+        // @todo also parse the css when no classes are defined
+        if($config['css_classes']) {
+          $settings['customStylesName'] = "custom_styles";
+	        $settings['stylesCss'] = _wysiwyg_ckeditor_get_stylesCss($config['css_classes']);
+        }

This always needs to run, so it could be placed just before the check for the language setting instead.

+++ ./ckeditor.inc	2010-04-19 09:30:52.348360100 +0200
@@ -323,3 +335,24 @@ function wysiwyg_ckeditor_plugins($edito
+  	 $l = explode("=", $line);
+  	 $lc = explode(".", $l[1]);

Tab indents.

+++ ./js/ckeditor-3.0.js	2010-04-16 16:43:35.191297500 +0200
@@ -24,7 +24,12 @@ Drupal.wysiwyg.editor.init.ckeditor = fu
+    CKEDITOR.stylesSet.add(wysiwygSettings.customStylesName, wysiwygSettings.stylesCss);

This method doesn't exist.

CKEditor.addStylesSet() does exist, but the key/first parameter needs to be different for each format or they would all use the same settings (if not crashing).

+++ ./js/ckeditor-3.0.js	2010-04-16 16:43:35.191297500 +0200
@@ -24,7 +24,12 @@ Drupal.wysiwyg.editor.init.ckeditor = fu
+    CKEDITOR.stylesSet.add(wysiwygSettings.customStylesName, wysiwygSettings.stylesCss);

This method does not exist.

+++ ./js/ckeditor-3.0.js	2010-04-16 16:43:35.191297500 +0200
@@ -110,7 +115,10 @@ Drupal.wysiwyg.editor.attach.ckeditor = 
+  if(settings.customStylesName)	CKEDITOR.config.stylesCombo_stylesSet = settings.customStylesName;

The value of stylesCombo_stylesSet needs to be a filename (without .js), or a full url to a file containing the definition.

Setting this on CKEDITOR.config makes it global for all editor instances. It needs to be set on an instance level in attach().

Powered by Dreditor.

TwoD’s picture

Status: Needs review » Needs work

Status...

pacproduct’s picture

TwoD, when you say This patch leaves the combo box empty for me, throws the error "CKEDITOR.stylesSet is undefined"..., where exactly do you have these issues? In my case I've no error when creating a node, and my styles are well listed in the Font Style combo box of CKEditor.

TwoD’s picture

The Font style combo box in the CKEditor toolbar is completely empty on every editor instance, if I apply this patch. Which is what I would expect considering there's no 'custom_styles.js' file to parse classes from.
The missing method error also appears wherever there's an editor instance.

fletchgqc’s picture

The patch didn't work for me. I tried the manual way:

CKEditor doco tells you how to define styles:
http://docs.cksource.com/CKEditor_3.x/Developers_Guide/Styles

The doco was out of date with new function names and didn't work.

So I "hacked" sites/all/libraries/ckeditor/plugins/styles/styles/default.js and put my styles in there.

I agree that this needs fixing in Wysiwyg to end hacks.

lee20’s picture

Subscribe

Andrew_Mallis’s picture

subscribe

Stephen Scholtz’s picture

subscribing

rockitdev’s picture

subscribing. for now, i'm using tinymce.

sandwormblues’s picture

Subscribing. font styles and colors are broken. Smileys don't work either! :.-(

Ckeditor is gonna be awesome when it works! i'm excited.

jlyon’s picture

subscribing

rootwork’s picture

sigh. subscribing.

stefan81’s picture

Hi,
I placed config.stylesCombo_DrupalFiltered = 'my_styles'; inside ckeditor.config.js and
CKEDITOR.addStylesSet( 'my_styles', inside ckeditor.styles.js but no custom styles are loaded effect.
Any clues?

upupax’s picture

subscribe

TwoD’s picture

If you need a workaround, here's how to do it using an hook_wysiwyg_editor_settings_alter() implementation in a custom module.
sites/all/MYMODULE/MYMODULE.module

function MYMODULE_wysiwyg_editor_settings_alter(&$settings, $context) {
  if ($context['profile']->editor == 'ckeditor') {
       $settings['stylesCombo_stylesSet'] = 'my_styles:' . base_path() . drupal_get_path('module', 'MYMODULE').'/wysiwyg_styles.js';
  }
}

EDIT: Added base_path() as per requests below, should fix some URL problems.

sites/all/MYMODULE/wysiwyg_styles.js

CKEDITOR.addStylesSet('my_styles',
    [
      {
        name: 'Spaced UL list',
        element: 'ul',
        attributes: {'class' : 'spaced'} 
      },
      {
        name: 'Spaced OL list',
        element: 'ol',
        attributes: {'class' : 'spaced'}
      }
    ]
    );

EDIT: Please note that the above style definitions are just a partial sample of what list styles I used on a site. The element keys above are set to "ul" and "ol", meaning these styles will only show up if an unordered or ordered list is selected in the editor. Check out the link to the official CKEditor documentation in a post below to find out how to write your own definitions.

sites/all/MYMODULE/MYMODULE.info

name = My custom module
description = A small module to override some Wysiwyg settings.
package = Custom
core = 6.x
Peter Törnstrand’s picture

The solution in #20 works fine. You should probably modify the function to this (added base_path()):

<?php
function MYMODULE_wysiwyg_editor_settings_alter(&$settings, $context) {
  if ($context['profile']->editor == 'ckeditor') {
       $settings['stylesCombo_stylesSet'] = 'my_styles:'.base_path().drupal_get_path('module', 'MYMODULE').'/wysiwyg_styles.js';
  }
}
?>
mawosch’s picture

Here a problem appeared with CKEditor and the "YAML for Drupal" Theme.
On different nodes the Firefox, and only the Firefox, scrolls down to different points.

In the script area of the HTML sourcecode the value for "contentsCss": [ ] is empty.
If I switch to Garland theme. There is a link to the style.css
"contentsCss": [ "/themes/garland/style.css" ]

If I use TinyMCE the value in brackets is empty too, but there is no problem with scrolling in Firefox.

goron’s picture

I attempted to use the solution in #20, but I now just get a blank Styles box. Any idea why this could be?

goron’s picture

What file is the patch in #1 for? I can't figure out how to use it. Thanks!

UPDATE:

I realized it should be executed from the wysiwyg/editors/ folder and applies to the files automatically.

TwoD’s picture

@goron, Make sure the name before the colon in the "stylesCombo_stylesSet" value ("my_styles") is the same as the name entered in CKEDITOR.addStylesSet('my_styles',. Also make sure your browser can load wysiwyg_styles.js. (The Net tab in Firebug for Firefox should be able to tell you if there's a 404 error and what path was used.)

danielnolde’s picture

nothing essential, just a minor type in the js code for style definition (sites/all/MYMODULE/wysiwyg_styles.js):

namme: 'Spaced OL list',

should be written with just one "m":

name: 'Spaced OL list',

upupax’s picture

I just tried the workaround.
The Firefox result is an empty selection box.
If I try it with Safari, I get the two js styles.

sittard’s picture

I also got an empty selection box, but after reading: http://docs.cksource.com/CKEditor_3.x/Developers_Guide/Configuration/Styles

I changed sites/all/MYMODULE/wysiwyg_styles.js to:

CKEDITOR.addStylesSet( 'my_styles',
[
    // Block Styles
    { name : 'Blue Title', element : 'h2', styles : { 'color' : 'Blue' } },
    { name : 'Red Title' , element : 'h3', styles : { 'color' : 'Red' } },

    // Inline Styles
    { name : 'CSS Style', element : 'span', attributes : { 'class' : 'my_style' } },
    { name : 'Marker: Yellow', element : 'span', styles : { 'background-color' : 'Yellow' } }
]);

Cleared my cache and it seems to be working now.

Thanks to @TwoD for the original posting #20.

TwoD’s picture

Looks like my sample styles in #20 were a bit too specific, they only showed up in the Styles dropdown if a list element was selected in the editor.
Edited #20 to clarify this, and fixed the typo danielnolde found.

entrigan’s picture

i am getting a blank styles box with #20 as well. Are you guys using ckeditor 3.4?

TwoD’s picture

entrigan, read #20 and #29 again. The style definitions from the sample wysiwyg_styles.js only apply to <ul> and <ol> elements, so the two styles will only be visible if one of them are selected in the editor. Read what's linked to in #28 to find out the syntax for adding more styles.

entrigan’s picture

I am using a nonstandard url structure, so the problem was I needed to specify the base path like so:
$settings['stylesCombo_stylesSet'] = 'my_styles:'.base_path().drupal_get_path('module', 'wc_display').'/wysiwyg_styles.js';

I do not understand why for the ol/ul definitions from #20 you need to select ol text, but for the h2/h3 defitions from #28 you do not. Explanation?

Thanks TwoD/Sittard!

TwoD’s picture

Ah, right. I'll add that change to #20 as well.

The CKEditor style plugin treats elements differently depending on what type they are. It uses a couple of constants and two element lists to determine how to treat some styles. (With styles, I mean instances of CKEDITOR.style, created from the list of "definitions" passed to CKEDITOR.addStylesSet()). The snippets below are from ckeditor/_source/plugins/styles/plugin.js.
Style types:

CKEDITOR.STYLE_BLOCK = 1;
CKEDITOR.STYLE_INLINE = 2;
CKEDITOR.STYLE_OBJECT = 3;

Style groups:

var blockElements = { address:1,div:1,h1:1,h2:1,h3:1,h4:1,h5:1,h6:1,p:1,pre:1 };
var objectElements = { a:1,embed:1,hr:1,img:1,li:1,object:1,ol:1,table:1,td:1,tr:1,th:1,ul:1,dl:1,dt:1,dd:1,form:1};

Styles for "object" elements can only be applied to existing obeject elements (which is why their dropdown items are hidden if you don't have a corresponding block element selected).

Styles for block elements can override other block elements (if another whole block element is selected), so their styles will show up in the list when any block is selected. If no block element is selected but the selected elements are allowed in certain block elements, the corresponding block styles will be visible in the list. I think the reverse is true as well, if the selection matches an element which can contain a block element, the corresponding styles will also be visible in the list but the element mentioned in the style definition will then be placed inside the selected element.

Inline styles (like simple color changes) can be applied to a currently selected block element (not sure about object elements). But if the selection is say a partial text element, that element will be split into at most three ranges. One range before the selection, one range containing the selection itself (now wrapped in a span which gets the actual styles applied to it), and finally there can be a remaining range after the selection. If this all happened inside a single p element, not much else happens, but it gets way more complicated if the selection [partially] spans several elements of varying types.

The above "rules" are a huge simplification as the style plugin does its best at following what the DTD says about which elements go where, how they can be split etc. The CKEditor docs seem to be incomplete so far, but this is what I deduced from the code itself.

Confused yet? I know I am hehe.

entrigan’s picture

Haha, excellent explanation. One note from #20, I think you need to remove the / in 'my_styles:/'

GrahamO’s picture

Subscribing

rootwork’s picture

Component: Editor - CKeditor » Code
FileSize
1.22 KB

I've created all of the files specified in #20 and given it the name ckeditor_styles.module. It's attached here if anyone wants to download it and use it as a starting point for their own styles.

Sample styles are included, but commented out. You need to edit the .js file and uncomment and/or define your styles.

Hope this is helpful.

scarvajal’s picture

@rootwork: Thanks for this!

I have installed it and for me it works, styles are shown on the drop down list.

However the styles are not visible inside the editor (are there in html source), even if I'm usind my theme CSS in ckeditor profile config.
Do you guys have this working with complete wysisyg for styles?

Thanks.

scarvajal’s picture

Answering my own post in case is of help to anybody.

For Ckeditor to be a real WYSIWYG editor it should show me the styles on the editor when I apply them.

The problem is that in my theme I need to set the body ID and class tags for the editor as is commented in this page (its for the standalone CKEditor).

So I created a config file for the ckeditor:

CKEDITOR.editorConfig = function( config )
{
	// Define changes to default configuration here. For example:
	// config.language = 'fr';
	// config.uiColor = '#AADC6E';
  config.extraCss = '';	
  config.bodyClass = 'page-area';
  config.bodyId = 'content-area';
  config.extraCss += "body{background:#FFF;text-align:left;font-size:0.8em;}";	
};

But this config file was no loaded at all, then I found a post from TwoD on other issue that leads me to solve the problem adding some code to what rootwork posted:

<?php
function ckeditor_styles_wysiwyg_editor_settings_alter(&$settings, $context) {
  if ($context['profile']->editor == 'ckeditor') {
    $settings['stylesCombo_stylesSet'] = 'my_styles:' . base_path() . drupal_get_path('module', 'ckeditor_styles').'/ckeditor_styles.js';
     // This is needed to load the config file:
     $settings['customConfig'] = wysiwyg_get_path('ckeditor', TRUE) . '/ckeditor.config.js';
     // To load it from the CKEditor folder, no matter if it's in sites/all/libraries or sites/site-name/libraries.
     // $settings['customConfig'] = wysiwyg_get_path('ckeditor', TRUE) . '/config.js';
  }
}
?>
John Pitcairn’s picture

subscribe

TwoD’s picture

Wohoo! People are searching the issue queue! =D
Just a tip: If you want to make sure your custom config file stays intact between upgrades, place it directly under sites/[default/sitename] or another place where there's less risk someone just deletes a module/library and uploads a newer version.

sun’s picture

Component: Code » Editor - CKEditor
dddbbb’s picture

Subscribing.

michaellander’s picture

Great job rootwork ! One quick change to your ckeditor_styles.js though.

Change the 'styles :' to 'attributes :' like so:

Before:

CKEDITOR.addStylesSet('my_styles',
    [
    // Block Styles
      { name : 'Introductory Text' , element : 'p', styles : { 'class' : 'introtext' } },
      { name : 'Pull Quote' , element : 'p', styles : { 'class' : 'pullquote' } },

    // Inline Styles
    ]
    );

After:

CKEDITOR.addStylesSet('my_styles',
    [
    // Block Styles
      { name : 'Introductory Texts' , element : 'p', attributes : { 'class' : 'introtext' } },
      { name : 'Pull Quotes' , element : 'p', attributes : { 'class' : 'pullquote' } },
    // Inline Styles
    ]
    );
fietserwin’s picture

subscribe

dddbbb’s picture

Any chance we can roll the features of ckeditor_styles into WYSIWYG?

pixelwhip’s picture

The module from #36 via #20 worked as a workaround for me in D7 as well. I did have to edit the core values in ckeditor_styles.info file so D7 would recognize it as compatible.

Thanks!

Sborsody’s picture

subscribing

Sborsody’s picture

According to http://docs.cksource.com/ckeditor_api/symbols/CKEDITOR.config.html#.styl...

CKEDITOR.config.stylesSet replaces CKEDITOR.config.stylesCombo_stylesSet since version 3.3. This may be the reason for the missing methods?

Sborsody’s picture

Version: 6.x-2.1 » 6.x-2.x-dev
Status: Needs work » Needs review
FileSize
1.48 KB

Here's my first shot at this...

I basically modified g10's code according to the review and followed the convention of using "title=element.class" (CKEditor ties classes to elements so trying to stick with title=class paradigm won't be good imho). I made this patch from the 2011-Jan-14 6.x-2.x-dev version. The version of CKEditor I have installed is 3.5.0.6260.

To test format-specific styles, I added a formatted textarea field to the Story content type. I set Filtered HTML and Full HTML to both use ckeditor but with different settings in CSS Classes. The body field was using one format and the custom field was using another format. If there's no CSS Classes set in the format profile, it should fallback to default.

I stuck the js code in editors/js/ckeditor-3.0.js but it should rightfully belong in a new file since CKEDITOR.stylesSet.add was only added since 3.3. I'll have to work on that later. Looking for suggestions... Either create a new file for the newer version or 3.5 seems to also support CKEDITOR.addStylesSet. I'd have to test using the older method with my test case.

Sborsody’s picture

This patch replaces the one I last posted. I tracked down that the code does not work in ckeditor versions below 3.2.1. Those older versions seem to require either the style set to be inside an externally defined js file (like the workaround module) or the style set to be defined right in the page (which I have yet to discover what exactly that means). So I've copied the ckeditor-3.0.js file to ckeditor-3.2.1.js and put the code in there.

While it works to have two different style sets defined for two different formats on the same page, there is a js error generated because of the namespace collision. I have yet to work on naming the custom style sets based upon their format. I think this can be done in the ckeditor-3.2.1.js file.

slashrsm’s picture

Fixes this issue for me.

hadsie’s picture

The patch in #51 works for me with no changes on D7. Thanks @Sborsody!!

Sborsody’s picture

Last and smallest change. This uses the format name for the name of the stylesSet instead of the hard-coded name "custom". This results in no javascript errors due to namespace collisions.

That's cool if it is also working in D7. I haven't tried it yet. Maybe we can get more community reviews and get this patch applied.

TwoD’s picture

I'm not sure this is the best way to solve it yet, as this feature has a bit more power than what can be set via a simple textarea, but I don't have time to write up something better... would also need the "Advanced settings"-issue in first...

Anyway, a quick comment:

@@ -181,6 +184,11 @@ function wysiwyg_ckeditor_settings($editor, $config, $
     }
   }
+  // Get font styles, supported in ckeditor versions 3.2.1 and above
+  if (!empty($config['css_classes'])) {
+    $settings['stylesSet'] = wysiwyg_ckeditor_get_styles_set($config['css_classes']);
+  }
+

If $config['css_classes'] is empty, won't that make $settings['stylesSet'] and finally wysiwygSettings.stylesSet undefined - causing an error?

Sborsody’s picture

It looks like there's no error generated, only an empty object created. But you're right and checking for existence is something I just forgot. I'll blame it on my aversion to JavaScript. :) I've cleaned that code up some in this next patch file.

I'm not sure I understand the rest of your response. Could you please expand upon your thoughts?

fietserwin’s picture

FileSize
10.15 KB

I tested the patch from #56 on D7 with wysiwyg version7 .x-2.x-dev from 2011-Jan-23 and it basically works. However some checks should be added to the wysiwyg_ckeditor_get_styles_set() function as it is parsing user input. So it cannot be assumed that each and every line (especially a trailing empty line is often seen) is in the correct format leading to warnings like:

# Notice: Undefined offset: 1 in wysiwyg_ckeditor_get_styles_set() (regel 350 van D:\Projecten\Drupal7\www\sites\all\modules\wysiwyg\editors\ckeditor.inc).
# Notice: Undefined offset: 1 in wysiwyg_ckeditor_get_styles_set() (regel 353 van D:\Projecten\Drupal7\www\sites\all\modules\wysiwyg\editors\ckeditor.inc).

Note: the output as send to the browser is escaped (by Drupal core) thus only checking on the correct format (name=element.class) should suffice. No need to check for special characters, script injection or other malicious things.

I arrived at something like:

/**
 * Return an array for JS of custom Font styles
 * 
 * note: only checking for correct format of each line, no error logging/hinting, should be done on the settings form
 */
function wysiwyg_ckeditor_get_styles_set($css_classes) {
  $css_styles = array();
  $lines = explode("\n", $css_classes);
  foreach ($lines as $line) {
    if (!empty($line)) {
      $l = explode("=", $line);
      if (count($l) === 2) {
        $lc = explode(".", $l[1]);
        if (count($lc) === 2) {
          $style['name'] = trim($l[0]);
          $style['element'] = trim($lc[0]);
          $style['attributes']['class'] = trim($lc[1]);
          $css_styles[] = $style;
        }
      }
    }
  }
  return $css_styles;
}

this means that an empty array may be returned for a non-empty $config['css_classes'], thus some checking at the calling code might also be needed:

  // Get font styles, supported in ckeditor versions 3.2.1 and above
  if (!empty($config['css_classes'])) {
    $styles = wysiwyg_ckeditor_get_styles_set($config['css_classes']);
    if (!empty($styles)) {
      $settings['stylesSet'] = $styles;
    }
  }

One final note: the help for the css classes on the admin/config/content/wysiwyg/profile/
/edit form should be made dependent on the used editor. current help text for TinyMce, changed text for CKEditor > 3.2. Something like (and probably with a link to the CKEditor help explaining it):

Optionally define CSS classes for the "Font style" dropdown list.
Enter one class on each line in the format: [title]=[element][class]. Example: My heading=h1.header1
If left blank, CSS classes are automatically imported from all loaded stylesheet(s).

slashrsm’s picture

Patch at #57 fails when trying to use it. I thik it is trying to remove new code instead of adding it.

fietserwin’s picture

FileSize
10.15 KB

This one should be better.

mraichelson’s picture

subscribe

afeldinger’s picture

subscribe

patrickharris’s picture

subscribe

cctvsysadmin’s picture

subscribe

endless_wander’s picture

I'm using the custom module download from #36

The first time I added new styles to my custom .js, they appeared in my ckeditor to use.

Since then, though, any changes I make to the custom .js are not reflected in the ckeditor. Adding a new style or removing a style doesn't change the list of styles available in ckeditor at all.

If I load my custom .js straight to the browser, I can see my new styles, but they're just not being reflected in the ckeditor.

What's weird is that even if I delete the custom .js file entirely, it doesn't change anything in the ckeditor. I can still select my custom styles even though the .js file doesn't exist.

I've tried uninstalling my custom module and then re-installing it with the custom styles removed. But even then, the ckeditor styles don't change. They use the same settings from before the module was uninstalled.

I've tried flushing Drupal caches, browser caches, etc. Anyone have any ideas or encountered something similar?

TwoD’s picture

If you're using the module from #36 you should be modifying sites/all/modules/ckeditor_styles/ckeditor_styles.js, unless you changed the path in sites/all/modules/ckeditor_styles/ckeditor_styles.module.

endless_wander’s picture

Thanks for replying TwoD... the ckeditor_styles.js is what I've been modifying, but these changes don't show up in CKEditor. If I load ckeditor_styles.js directly in the browser, it shows my changes, but these changes aren't reflected in CKEditor.

TwoD’s picture

Cache can be a pain sometimes, but completely flushing both Drupal's and the browser's caches should have taken care of it. Can you use Firebug for Firefox or a similar tool to really make sure that's the file being loaded, and that it has the updated contents when parsed as part of the page? The file's contents aren't stored by Wysiwyg or the editor, nor the styles it creates, so something else must be holding onto the old version.

Johan den Hollander’s picture

Today I had the same experience as #64. The problem was browser cache...
After deleting the browsers cache the updated styles list appeared.

endless_wander’s picture

So, clearing all caches in Drupal and browser doesn't solve for me.

Further investigation shows that the problem comes from my custom.js file being tagged with a timestamp variable by CKEditor. This variable is supposed to stop things from being cached (see http://docs.cksource.com/CKEditor_3.x/Developers_Guide/Editor_Core_URLs_...), but it's being duplicated on my page and messing things up.

When my js file loads on the page it comes up as:
/modules/ck_custom/ck_custom.js?t=A5AB4B6&t=A5AB4B6

That "t=A5AB4B6" bit is the CKEditor timestamp variable. But it's only supposed to be there once.

If I go to ck_custom.js?t=A5AB4B6&t=A5AB4B6 in my browser, I see old, out-of-date styles. If I go to ck_custom.js?t=A5AB4B6, I see the correct, up-to-date styles.

Weirdly, anything I put into the module is outputted twice. So if I print $settings['stylesSet'], it outputs twice. If I stick in a print "Hello"; line it will print Hello twice.

I'm guessing this is the problem, but have no idea why it's doing this. I'm going to try rebuilding a new module from scratch with a new name and seeing if problem happens still.

** UPDATE - problem still occurs with brand new module with new name

endless_wander’s picture

Please let me know if I should move my issue to its own thread...

I've uninstalled any custom module and noticed that when CKEditor loads the default.js file, it has the same problem with the duplicated timestamp.

It loads it with this path: /sites/all/libraries/ckeditor/plugins/styles/styles/default.js?t=A5AB4B6&t=A5AB4B6

If I make a change straight to the default.js file, it doesn't appear in the editor.

If I go to /sites/all/libraries/ckeditor/plugins/styles/styles/default.js?t=A5AB4B6&t=A5AB4B6 my change doesn't appear.

But if I go to /sites/all/libraries/ckeditor/plugins/styles/styles/default.js?t=A5AB4B6, the changes appear.

kevinquillen’s picture

Subscribe. Experiencing the same issues.

fietserwin’s picture

Can we progress on this issue?

@all subscribers: please tests and review the most recent patch (#59) and share your results so we can get to RTBC. It looks like you can test the patch in both D6 and D7 (though the patch was generated in a D7 install)
@TwoD: You had some hesitations (#55). Will that prevent you from committing this patch if its reaches status RTBC?
@TwoD: must a changed help text be in the patch to commit it?

kevinquillen’s picture

I was able to get around this by writing a module and hooking in, creating the necessary javascript, as others noted. But would be nice for the admin to work.

mjs2020’s picture

subscribe

I tried patching wysiwyg 6.x-2.3 with patch from #59 but got no change even after clearing Drupal caches and browser caches. I'm just going to manually edit the default.js file in /sites/all/libraries/ckeditor/plugins/styles/styles/default.js for the site I'm working on, but it would be nice to have a clean fix for this.

fietserwin’s picture

@mjs2020:
- Did the patch apply correctly?
- Are you running ckeditor 3.2.1.5372 or newer?
- Does ckeditor-3.2.1.js get included (script tag in the head tag of your pages) and loaded?
- Are you using the correct syntax in the "CSS classes" field on the admin/config/content/wysiwyg/profile/%/edit pages? Example from one of my configurations (this differs from tinymce syntax as is currently displayed below this textarea):
No space below = p.no-space
Indent = p.indent
Image at left = img.img-left
Image at right = img.img-right

gease’s picture

Patch from #59 works great, thanks!

Stephen Scholtz’s picture

Just confirming that patch in #59 worked for me as well, even though I had a new version of CK Editor (3.5.2...)

Note to anyone else who wants to apply this patch, you'll need to specify your styles as "element.classname", and not .classname like you might normally do, as mentioned in #50.

Running:
Drupal 6.19
WYSIWYG 2.3
CK Editor 3.5.2.6450

wuf31’s picture

Confirming that this works on drupal 7 as well, using patch #59, config #50 as above comment.
WYSIWYG 7.2
CK Editor 3.5.3.6655

fietserwin’s picture

Status: Needs review » Reviewed & tested by the community

IMO enough positive feedback to change it to RTBC.

gwideman’s picture

Subscribe

osopolar’s picture

patch #59 does the job (d6, wysiwyg 6.x-2.3, CKEditor 3.5.3.6655).

rootwork’s picture

Let's commit this puppy!

TripX’s picture

Is it commited?

rootwork’s picture

No. That was my enthusiastic request to the maintainers, now that it is RTBC.

acrollet’s picture

The patch in #59 applies cleanly to 7.x-2.0 and works.

sun’s picture

Status: Reviewed & tested by the community » Needs work
+++ ./editors/ckeditor.inc.new	Fri Jan 28 23:16:14 2011
@@ -44,6 +44,9 @@
+      '3.2.1.5372' => array(
+        'js files' => array('ckeditor-3.2.1.js'),
+      ),

@@ -181,6 +184,14 @@
+  // Get font styles, supported in ckeditor versions 3.2.1 and above
+  if (!empty($config['css_classes'])) {
+    $styles = wysiwyg_ckeditor_get_styles_set($config['css_classes']);
+    if (!empty($styles)) {
+      $settings['stylesSet'] = $styles;
+    }
+  }

Do we really need to branch into an entirely new integration file? It looks like we have a unique condition (a new settings/config parameter) to execute code on?

Powered by Dreditor.

jkaine’s picture

Patch from #59 works well in D7 + WYSIWYG 7x-2.0 + CKEditor 3.5.2.6450

Only difficulty I had was that the portion of the patch for ckeditor.inc didn't load. The file dates didn't match in line 2. I was able to mod the file by hand and all went well.

Thanks to all of you for your work in getting this sorted out.

osopolar’s picture

Status: Needs work » Needs review
FileSize
2.45 KB

I hope this patch will find it's way into wysiwyg soon, therefore I applied the changes suggested by sun in #86 to #59.

Instead of a new integration file we now check if object CKEDITOR.stylesSet is set in wysiwyg/editors/js/ckeditor-3.0.js

    // Register Font styles (versions 3.2.1 and above).
    if (CKEDITOR.stylesSet && Drupal.settings.wysiwyg.configs.ckeditor[format].stylesSet) {
      CKEDITOR.stylesSet.add(format, Drupal.settings.wysiwyg.configs.ckeditor[format].stylesSet);
    }

I tested this for ckeditor Versions 3.0.1 (where the style set won't apply, as expected) and 3.6 where the style set apply as expected (as in #59).

Put the patch into the corresponding modules directory and call the command:
patch -p1 --dry-run < ../wysiwyg-font-styles-746524-88.patch

fietserwin’s picture

Issue tags: +needs forward port to Drupal 7
FileSize
3.42 KB

I tested #88 on D7.2 with Wysiwyg 7.x-2.x-dev and CKEditor 3.5.3: still works fine. I added a changed help text to the patch, to describe what goes into the css text area for CKEditor 3.2.1 and above. No other changes.

So, AFAIC, RTBC regarding the existing part of the patch. Can someone else confirm the correctness of my change and RTBC this issue?

TwoD’s picture

Status: Needs review » Needs work
+++ editors/ckeditor.inc	2011-05-31 14:58:51.000000000 +0200
@@ -180,6 +180,14 @@
+  // Get font styles, supported in ckeditor versions 3.2.1 and above

Missing punctuation.

+++ editors/ckeditor.inc	2011-05-31 14:58:51.000000000 +0200
@@ -331,3 +339,26 @@
+ * note: only checking for correct format of each line, no error logging/hinting, should be done on the settings form

Max width should be 80 chars, leave a blank space between summary and the detailed description and please use punctuation etc. Please add info on parameters and return value.

+++ editors/js/ckeditor-3.0.js	2011-05-31 15:18:27.000000000 +0200
@@ -110,9 +114,8 @@
     selectionChange: function (event) {
-      var pluginSettings = Drupal.settings.wysiwyg.plugins[params.format];
-      if (pluginSettings && pluginSettings.drupal) {
-        $.each(pluginSettings.drupal, function (name) {
+      if (Drupal.settings.wysiwyg.plugins[params.format]) {
+        $.each(Drupal.settings.wysiwyg.plugins[params.format].drupal, function (name) {

Why this change?

+++ wysiwyg.admin.inc	Wed Jun 15 14:55:29 2011
@@ -286,7 +286,11 @@
-    '#description' => t('Optionally define CSS classes for the "Font style" dropdown list.<br />Enter one class on each line in the format: !format. Example: !example<br />If left blank, CSS classes are automatically imported from all loaded stylesheet(s).', array('!format' => '<-code>[title]=[class]<-/code>', '!example' => 'My heading=header1')),
+    '#description' => t('Optionally define CSS classes for the "Font style" dropdown list.<br />Enter one class on each line in the format: !format. Example: !example<br />If left blank, CSS classes are automatically imported from all loaded stylesheet(s).',
+         $editor['title'] == 'CKEditor' && $editor['installed version'] >= '3.2.1'
+           ? array('!format' => '<-code>[title]=[element].[class]<-/code>', '!example' => 'My heading=h1.header1')
+           : array('!format' => '<-code>[title]=[class]<-/code>', '!example' => 'My heading=header1')

If this is to be kept, it should use version_compare($editor['installed version'], '3.2.1', '=>'), see ckeditor.inc line 1325.
I'm not really fond of introducing editor-specific code into Wysiwyg's core. Better to let the admin form "reach out" to the editors/plugins via a callback. Got something like that planned for #313497: Allow configuration of advanced editor settings and I hope to be able to post code there really soon.

Powered by Dreditor.

fietserwin’s picture

Status: Needs work » Needs review
FileSize
3.11 KB

- Punctuation. line length and documentation corrected.
- Why this change?

I don't know, but I guess we are preventing runtime errors about undefined properties. Unfortunately, unlike isset() in PHP, javascript (at least in firefox) will complain about parents not existing when testing for something like if (typeof a.b.c !== 'undefined'). Thus in this case, if a.b does not exist you will get a runtime error.

Concluding: the old code looks more robust.So I removed this part.

- If this is to be kept
I leave that up to you. I can agree with your wish to keep wysiwyg core editor agnostic, but the current help text is an IMCE specific text without this becoming apparent.

I would advice to accept this change and add an @todo to it referring to that other issue, but it's your decision.

jastraat’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
FileSize
3.87 KB

Re-rolled and tested a patch against the lastest dev of WYSIWYG for Drupal 7. Please test so we can get this committed!

jkaine’s picture

Jastraat,

Thanks for the patch in #92. Did a clean install of WYSIWYG 7.x-2.x-dev, and then applied the patch.

Running NetBeans in Windows 7, the patch didn't apply completely-- patch for wysiwyg.admin.inc did apply, but then had to edit by hand the changes for editors/ckeditor.inc and editors/js/ckeditor-3.0.js

Once implemented, the changes worked perfect in Windows running Drupal 7.0

I'm migrating out of Windows and into a Linux environment here in a bit. If the patch doesn't work once the site is moved to a Linux server, I'll post an update. Otherwise, consider this a successful test of the patch.

Thanks,
JK

fietserwin’s picture

Status: Needs review » Needs work

Patch seems to be in wrong line ending and contains tabs.

amanaplan’s picture

subscribing

jastraat’s picture

@fietserwin - I'm afraid this is a limitation of the way I'm rolling the patches. Someone else may need to update the patch to match your requirements. I'm afraid I couldn't get your patch to apply either.

sun’s picture

@jastraat:

git config --global core.autocrlf false
git config --global core.safecrlf false
jastraat’s picture

I'm actually using Git through Eclipse, not at the command line. Would you happen to know where I could configure that in Eclipse?

jastraat’s picture

FileSize
3.36 KB

I may have figured out where those settings are within Eclipse. Could someone take a look at this patch and let me know if the line ending and tabs are fixed in this version?

TwoD’s picture

+++ b/editors/ckeditor.incundefined
@@ -180,6 +180,12 @@
+  	if (!empty($styles)) {
+      $settings['stylesSet'] = $styles;
+  	}

Tabs still there (last white-space before the code except on the $settings row). Don't know about line endings, Dreditordoesn't check those AFAIK.

Powered by Dreditor.

amanaplan’s picture

FileSize
3.36 KB

Re-roll of #99 with two tabs removed and proper indenting.

fietserwin’s picture

Status: Needs work » Reviewed & tested by the community

I updated all modules on a site, So I had to reroll this issue. Nice opportunity to test the latest patch: applies correctly (on D7) and still works as supposed.

stred’s picture

I can confirm: patch #101 works as expected

amanaplan’s picture

Patch to port to 7.x-2.x-dev is already in issue--removing tag.

patmacs’s picture

I can also confirm that patch #101 works without problems

cangeceiro’s picture

confirming patch #101 works for me as well

joelcollinsdc’s picture

reroll of #101 with -p0 so i can use this with drush make

anou’s picture

Hello,

Don't know if patch #101 is supposed to work if "CSS classes" is empty but in my case it only works (but that's what I wanted !!!) when you specify classes in the textarea (e.g.: My heading=h1.header1).

If "CSS classes" is empty, I still got the default listing from Ckeditor (begin with RED, BLUE, ...etc...)

Thanks.

For info, my config :

  • Drupal core: 7.4
  • Wysiwyg: 7.x-2.1
  • Ckeditor: 3.6.1

and I have define a path to CSS (/sites/all/themes/my-theme/css/commons.css)

fietserwin’s picture

@#108: The help below the field says:

Optionally define CSS classes for the "Font style" dropdown list.
Enter one class on each line in the format: [title]=[element].[class]. Example: My heading=h1.header1
If left blank, CSS classes are automatically imported from all loaded stylesheet(s).

So my first guess is that this works as designed. However, I'm not sure if the red. blue, and yellow thingies come from the "loaded stylesheets". I guess not, so this might indeed be an error, but if so it would be a separate issue:
- Parsing current style sheets for classes and prefixing them with the (type of) elements they can/should be applied to: looks like a mission impossible to me?
- This issue is about parsing a non-empty textarea in a way that CKEditor can handle.
- I think this patch does what it should do, and should in no way be expected to include even more features. It is eagerly awaited by many people for a long time.

Note that for me specifying "Use theme CSS" works fine as well, so I don't have to specify the CSS path.

alanburke’s picture

#107 does the trick.

giorgio79’s picture

I am experiencing in TinyMce as well. Nothing in the Styles dropdown, no headings and other block formats. Would this patch help tinymce as well?
Posted here as a separate issue:#1244742: No block formats in Styles dropdown

fietserwin’s picture

No, this patch is pure CKEditor only.

eric.toupin’s picture

#107 does it for me w/ core 7.7 & CKEditor 3.6.1.7072. Thanks!

tchopshop’s picture

works for me!

fletchgqc’s picture

#107 works for me - perfect. Great that you got the proper element.class syntax in there - that's crucial for CKEditor.

mariomaric’s picture

Hi to everybody.

#107 is working also for wysiwyg-6.x-2.x-dev (2011-Jun-23) & ckeditor-3.6.1.7072.

Thx for fixing this issue.

rootwork’s picture

Works for me too. Would be great to see this committed!

michaelfavia’s picture

@sun and @twod #107 works here as well, anything else we can provide to get this committed?

Ankabout’s picture

#107 worked for me too.

cangeceiro’s picture

I've been running this patch since july and still have had no issues, +1 to have this committed please.

gagarine’s picture

#107 works for me too.

paulmckibben’s picture

#107 works for me as well. Subscribing so I know if/when this patch gets committed.

missWilder’s picture

subscribing

Ankabout’s picture

Worked perfectly on another install.

rootwork’s picture

RTBC! Let's get this committed! Any maintainers want to comment?

roderik’s picture

FileSize
3.43 KB

+1 for RTBC.

FYI: while looking if there were changes between the D6-D7 patches, I discovered there are none. #107 ends up being exactly like #91 was, except some spacing in wysiwyg.admin.inc. (For which there are no hard rules, it's just personal preference.)

...and some comments had mysteriously disappeared between #91 and #92. I reinstated them.

So, I verified this patch is for D6 too. Still essentially equal to #91 (except a few spaces in a multi-line command), still equally RTBC.

rootwork’s picture

More than two months of positive reviews of the patch have gone by, and it's been marked as RTBC even longer than that (based on earlier patches). Can we PLEASE get a maintainer to patch the D6 and D7 versions now? Pretty please?

dazz’s picture

works perfect

erykmynn’s picture

This worked for me in D7. Commit?

AdamGerthel’s picture

The patch works for me as well (D7)

selinav’s picture

Thanks for this patch, it works for me.

jrattanpal’s picture

Hi

I am new to patches. Can you please help on how I can take this patch and apply to my ckeditor.inc file? do I have to do this line by line?

And how/when will this patch make into the existing code in case I need to update to new version when its out? Sorry, if those are basic questions but I am new to this process.

Thanks

rootwork’s picture

@jsingh The easiest tutorials on drupal.org about patches are:

http://drupal.org/node/367392
http://drupal.org/node/1054616 (scroll down to "applying patches")

You'll need command-line access in order to apply the patches most easily. There are some graphical programs that would allow you to patch the files if you don't have access to the command line, but they vary by OS (and would require you to download some of your site locally).

jrattanpal’s picture

Thanks. I applied the patch successfully. It works when I have CSS Classes in the text area. When it loads classes from textarea, it removes all other default classes. Is it intended?

However, it still doesn't load the classes from my custom theme CSS. I have selected "Define CSS" in the drop down and provided with CSS path.

EDIT: I also checked with FireBug and I am getting 200 on my CSS files (and I see those in JQuery query call in HTML using View Source). So, I have entered the correct path and its working. Its just not loading it in Styles drop down.

jrattanpal’s picture

To add more, I noticed that when I use the classes manually from my CSS files (or even preview it) then it works as expected. It uses styles from my CSS files.

However, it doesn't populate Styles Drop Down and I'd need that so I can just let users know (when adding/editing content) which styles to use in certain areas.

fietserwin’s picture

@jsingh: this is intended, ckeditor cannot know for which type of elements the styles in your CSS are meant. So you have to (make a selection and) add them to the textarea in the syntax as ckeditor expects.

abelb’s picture

Category: bug » support

i am using
CKEditor 3.5.1.6398
Drupal 7.7

I have gotten the patch applied, defined my own css and the font styles. All of this is working except the wysiwyg is not recognizing other styles i have defined such as body and link styles.

ie I want the body of the wysiwyg textarea to be Arial but It is not recognizing that style. It is showing up Times New Roman like it is not seeing any body style.

any ideas?

and thanks for the patch!

steinmb’s picture

Category: support » bug

Pls. do not change the category of this issue.

Peter Swietoslawski’s picture

This is still an issue for Drupal 6. The latest patch provided here in #101 seems to do the job for D6. From what I can see the latest dev version of module is dated on 2011-Jun-23 which makes me think there is not a lot activity going on there.
Is there any plan to incorporate above fix in D6 version?

TwoD’s picture

Status: Reviewed & tested by the community » Needs work
+++ wysiwyg.admin.incundefined
@@ -286,7 +286,11 @@ function wysiwyg_profile_form($form, &$form_state, $profile) {
     '#title' => t('CSS classes'),
     '#default_value' => $profile->settings['css_classes'],
-    '#description' => t('Optionally define CSS classes for the "Font style" dropdown list.<br />Enter one class on each line in the format: !format. Example: !example<br />If left blank, CSS classes are automatically imported from all loaded stylesheet(s).', array('!format' => '<//code>[title]=[class]<//code>', '!example' => 'My heading=header1')),
+    '#description' => t('Optionally define CSS classes for the "Font style" dropdown list.<br />Enter one class on each line in the format: !format. Example: !example<br />If left blank, CSS classes are automatically imported from all loaded stylesheet(s).',
+      $editor['title'] == 'CKEditor' && version_compare($editor['installed version'], '3.2.1', '>=')
+      ? array('!format' => '<//code>[title]=[element].[class]<//code>', '!example' => 'My heading=h1.header1')
+      : array('!format' => '<//code>[title]=[class]<//code>', '!example' => 'My heading=header1')
+    ),

I'm not too happy about introducing editor-specific code here, that should really be fixed with #313497: Allow configuration of advanced editor settings. But since that has stalled, I'm willing to commit this if Sun agrees it's a necessary compromise.

(That $editor['title'] == 'CKEditor' check should probably be $profile->editor == 'ckeditor' so it doesn't rely on the "display title" rather than our internal name for the editor.)

This patch would go into both D7 and D6.

fietserwin’s picture

Status: Needs work » Needs review
FileSize
3.1 KB

Patch from #126 with the proposed change. Tested only in D7, but I assume that the $profile->editor property also exists in D6 as the suggestion came from one of the module maintainers.

Please, can someone else test once more and RTBC it, so it can finally be committed .........

bluehut’s picture

Tested patch 746524-141-D7.patch with:

  • Drupal 7.9
  • CKEditor 3.6.2
  • wysiwyg dev (2011-Nov-06)

I have not read the whole thread so may not have tested everything.

One possible issue found with the content of field 'CSS classes'

Heading 2=h2.title
Heading 3=h3.title
Heading 4=h4.
Heading 5=h5
Highlight=span.highlight

Heading 5 does not show up in the editor where I would have expected it to. Whereas Heading 4 (with the trailing period) does.

cheers
Nigel

fietserwin’s picture

FileSize
3.22 KB

Both your h4 and h5 lines are incorrect according to CKEditor syntax. So I tightened the syntax checking by testing for non-emptiness of all 3 components. Note: looking at the basic handling of the content of this textarea I don't see any syntax checking at all, but let's do it "correctly" for CKEditor.

Needs review again.

bluehut’s picture

OK. 746524-143-D7.patch looks good to me.

squarecandy’s picture

746524-143-D7.patch working great for me.
Thanks!

squarecandy’s picture

PS - I'm experiencing the issue in WYSIWYG 6.x-2.4
As noted above, this patch works for both 6 and 7 - just wanted to confirm the version I'm testing.

TwoD’s picture

Ignoring incorrectly formatted lines is really good, thanks for adding that.

The latest Wysiwyg versions (including -devs) for D6 and D7 are pretty much identical except for the obvious Drupal Core API differences. As long as patches only affect the editor implementations and not when/where Core API integration happens, patches should apply cleanly to both versions.editors

scottrigby’s picture

#143 working here

fietserwin’s picture

Status: Needs review » Reviewed & tested by the community

Should be enough to RTBC again, especially as the difference with the previous (RTBC'ed) patch is very small.

adel-by’s picture

#143 is working
you can also use hook_wysiwyg_editor_settings_alter

function MYMODULE_wysiwyg_editor_settings_alter(&$settings, $context) {
  if (!empty($context['profile']->settings['css_classes'])) {
    $styles = wysiwyg_ckeditor_get_styles_set($context['profile']->settings['css_classes']);
    if (!empty($styles)) {
      $settings['stylesSet'] = $styles;
    }
  }
}

function wysiwyg_ckeditor_get_styles_set($css_classes) {
  $css_styles = array();
  $lines = explode("\n", $css_classes);
  foreach ($lines as $line) {
    if (!empty($line)) {
      $l = explode('=', $line);
      if (count($l) === 2) {
        $lc = explode('.', $l[1]);
        if (count($lc) === 2) {
          $style['name'] = trim($l[0]);
          $style['element'] = trim($lc[0]);
          $style['attributes']['class'] = trim($lc[1]);
          if (!empty($style['name']) && !empty($style['element']) && !empty($style['attributes']['class'])) {
            $css_styles[] = $style;
          }
        }
      }
    }
  }
  return $css_styles;
}
GaryWong’s picture

Yes.. The D7 patch works on my D6 install. Thanks muchly!

  • Drupal 6.22
  • WYSIWYG 6.x-2.4
  • CKEditor 3.6.2.7275
superd2’s picture

746524-143-D7.patch works like a charm!
Thank you.

AaronBauman’s picture

Patch #143 applied.
Cache cleared.
"Font style" checkbox clicked.
Examples inputted on ckeditor styles settings for "CSS Classes".

Drupal.settings reflects my "CSS Classes" settings, but the Font Style menu doesn't show up in ckeditor.

Further, if i change to "Define CSS" and input the path to a CSS file, the css file is loaded properly but the menu still doesn't show up.

What am I doing wrong?
How can I test this properly?
What is it supposed to look like?

fietserwin’s picture

#153: is your syntax correct? Do you have a block/p/img/a selected (or as current element) , because the styles only appear if they can be used in the current element.

AaronBauman’s picture

comment deleted

OK, I was unaware of an implementation of hook_wysiwyg_editor_settings_alter() that was manually setting the toolbar order and button set.
Sorry for the WOB, carry on then.

mariomaric’s picture

Patch 746524-143-D7.patch from #143 works really good! :)

I'm using:

Drupal 7.10
WYSIWYG 7.x-2.x-dev - 2011-Dec-13
CKEditor 3.6.2.7275

Patched successfully from wysiwyg module directory with patch -p0 < 746524-143-D7.patch command.

But, as commented at #154 it is really important to select in CKEditor area HTML element that you have configured at Wysiwyg profile if you want to see anything in Styles drop down menu..

eikes’s picture

#143 works wonderfully!

David Vespoli’s picture

#143 is perfect. Thank you!

Peter Törnstrand’s picture

#143 works, great!

DuaelFr’s picture

#143 is perfect but why it has not been pushed to dev version yet ?

patrickharris’s picture

Hey - this thread is not even two years old. It's a baby in comparison to some will-not-commit-the-patch threads. It's passive resistance, and it works.

LeoDoms’s picture

FileSize
16.82 KB

The solution in #150 worked perfectly for me. As you parse the CSS-styles input yourself, it gives some added flexibility, like adding some classless tags among the classed ones. Our client seemed to find that an interesting option to unclutter the toolbar somewhat.
To that end, I tweaked the #150 code as follows. I'm a beginning drupaler, so I suppose it won't be up to the top quality coding standards, but it seems to work fine.

/*
 * implementation of hook_wysiwyg_editor_settings_alter
 */

function wysiwyg_custom_wysiwyg_editor_settings_alter(&$settings, $context) {

  if ($context['editor']['title'] == 'CKEditor') {
    if (!empty($context['profile']->settings['css_classes'])) {
      $styles = wysiwyg_custom_get_styles_set($context['profile']->settings['css_classes']);
      if (!empty($styles)) {
        $settings['stylesSet'] = $styles;
      }
    }
  }
}

/*
 * Creates a stylesSet array from the list of css classes for font styles
 * This implementation supports :
 *  - css-entity (e.g. em )
 *  - css-entity.class (e.g. span.instructions)
 * @param css_classes : array of  wysiwyg font styles
 * 
 * return stylesSet for the settings of the wysiwyg editor
 */

function wysiwyg_custom_get_styles_set($css_classes) {
  $css_styles = array();
  $lines = explode("\n", $css_classes);
  foreach ($lines as $line) {
    if (!empty($line)) {
      $l = explode('=', $line);
      if (count($l) === 2) {
        $lc = explode('.', $l[1]);
        $style['name'] = trim($l[0]);
        $style['element'] = trim($lc[0]);
        if (count($lc) === 2) {
          $style['attributes']['class'] = trim($lc[1]);
        }
        elseif (count($lc) === 1) {
          $style['attributes'] = null;
        }
        if (!empty($style['name']) && !empty($style['element'])) {
          $css_styles[] = $style;
        }
      }
    }
  }
  return $css_styles;
}

Tried it with CSS classes input

instructie=span.instructie
vet=strong
voorbeeld=tt
vloeiend=em.vloeiend
div=div
Groene Hoofding=h1.ecolo

and css

span.instructie { color: red; font-weight: bold; font-style: italic; }
span.instructie:before { content:url(../images/smiley.jpg); }
em.vloeiend { font-family: lucida; }
h1.ecolo { color: green; }

Gives perfect results

annikaC’s picture

Patched and working great so far! Thanks!

sun’s picture

Status: Reviewed & tested by the community » Needs work

Sorry for not reviewing this earlier. People should be safe to apply the patch in #143 for the time being, but we should implement this more cleanly:

  1. I'd like to see a 'settings form callback' for hook_editor(). The callback gets $form and $form_state passed from wysiwyg_profile_form(). Thus, wysiwyg_profile_form() is no longer hacked for this editor (and even version) specific adjustment, but instead, the editor plugin alters/overrides the profile settings form.
  2. The adjusted form element in the settings form callback should use an #element_validate form element validation callback to properly validate the new format of the user input.
  3. Before adding the font styles to the JS settings, the settings callback should check whether it is supported by the editor version. This check no longer needs to happen in the frontend/client-side JavaScript then (so the JS only checks whether the setting exists and applies it).

Furthermore:

+++ editors/ckeditor.inc	Tue Nov 08 09:36:01 2011
@@ -331,3 +339,37 @@
+  $lines = explode("\n", $css_classes);

This also needs to account for Windows line endings (CRLF) - essentially stripping all CRs before processing.

+++ editors/ckeditor.inc	Tue Nov 08 09:36:01 2011
@@ -331,3 +339,37 @@
+      $l = explode('=', $line);
...
+        $lc = explode('.', $l[1]);

The variable names are very cryptic; let's find and use explicit names here. Looks like $l could replace $line.

A typically useful coding pattern in this situation is list():

list($label, $selector) = explode('=', $line, 2);
list($element, $class) = explode('=', $selector, 2);
+++ editors/js/ckeditor-3.0.js	Tue Nov 08 09:36:22 2011
@@ -23,6 +23,10 @@
+    // Register Font styles (versions 3.2.1 and above).
+    if (CKEDITOR.stylesSet && Drupal.settings.wysiwyg.configs.ckeditor[format].stylesSet) {
+      CKEDITOR.stylesSet.add(format, Drupal.settings.wysiwyg.configs.ckeditor[format].stylesSet);
+    }

Why do we need to register stylesSet manually? Isn't there a way that makes CKEditor automatically pick up the stylesSet option in the config?

fietserwin’s picture

+++ editors/ckeditor.inc Tue Nov 08 09:36:01 2011
@@ -331,3 +339,37 @@
+ $lines = explode("\n", $css_classes);

To remove other line-ending characters one can use:

  $lines = array_filter(array_map('trim', explode("\n", $css_classes)));

(this also removes empty lines, thus that if in the for loop becomes obsolete)

Currently, I'm too busy to do some patch writing. So even though the recent patches may be from mine hand, I would like to ask someone else to give this a try.

plach’s picture

FileSize
3.75 KB

Rerolled the patch against the current head and tried to implement the code-style suggestions from #165. No time for further work.
Some notes:

$lines = array_filter(array_map('trim', explode("\n", $css_classes)));

This would perform and additional unneeded iteration over the $lines array.

list($label, $selector) = explode('=', $line, 2);
list($element, $class) = explode('=', $selector, 2);

These cannot be performed until proper validation is in place, no time to implement it atm. I tried to make variable names more explicit, though.

Why do we need to register stylesSet manually? Isn't there a way that makes CKEditor automatically pick up the stylesSet option in the config?

Unless I'm missing something (which is likely, given that I'm not familiar with the wysiwyg code at all), http://docs.cksource.com/CKEditor_3.x/Howto/Styles_List_Customization seems to suggest this is the proper way to do it.

robinvdvleuten’s picture

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

When define your stylesheet's path at admin/config/content/wysiwyg/profile/filtered_html/edit, they aren't visible to the CKEditor. This is because the CKEditor has to load the stylesheetparser to parse them.
With this small snippet, you are able to do so. Please incorporate this into the module.

function ckeditor_styles_wysiwyg_editor_settings_alter(&$settings, &$context)
{
	if ($context['profile']->editor == 'ckeditor') {
		$settings['extraPlugins'] .= ',stylesheetparser';
		$settings['stylesSet'] = array();
	}
}
steinmb’s picture

Version: 7.x-2.1 » 7.x-2.x-dev
kardave’s picture

FileSize
4.07 KB

Joining code #167 and patch #166
Update: please see patch in #171 instead of this.

amaria’s picture

I'm getting errors trying to apply this patch:

wysiwyg-746524-169.patch:10: trailing whitespace.
'#description' => t('Optionally define CSS classes for the "Font style" dropdown list.
Enter one class on each line in the format: !format. Example: !example
If left blank, CSS classes are automatically imported from all loaded stylesheet(s).',
wysiwyg-746524-169.patch:11: trailing whitespace.
$profile->editor == 'ckeditor' && version_compare($editor['installed version'], '3.2.1', '>=')
wysiwyg-746524-169.patch:12: trailing whitespace.
? array('!format' => '[title]=[element].[class]', '!example' => 'My heading=h1.header1')
wysiwyg-746524-169.patch:13: trailing whitespace.
: array('!format' => '[title]=[class]', '!example' => 'My heading=header1')
wysiwyg-746524-169.patch:14: trailing whitespace.
),
fatal: corrupt patch at line 103

kardave’s picture

FileSize
3.99 KB

Try this one, I tested it:

patch -p0 < wysiwyg-746524-171.patch
(Stripping trailing CRs from patch.)
patching file editors/ckeditor.inc
(Stripping trailing CRs from patch.)
patching file editors/js/ckeditor-3.0.js
(Stripping trailing CRs from patch.)
patching file wysiwyg.admin.inc
amaria’s picture

That did it and the patch works for me. Thanks!

amaria’s picture

Spoke too soon. Although the patch allows me to see my custom styles in the Font Styles drop-down, it no longer recognizes my css file. Using wysiwyg from HEAD without any patches, if I add the class via source mode in the editor, it applies the style from my css file which makes it format correctly in the editor.

amaria’s picture

Okay, let me start over. It works! It's just that I'm using LESS and the css file was being generated on every request.

grantlucas’s picture

Status: Needs work » Needs review

Tested and works fine for me.

TwoD’s picture

Status: Needs review » Needs work
+++ editors/ckeditor.inc	(working copy)
@@ -344,3 +352,51 @@
+/**
+ *  load the stylesheet parser to parse the user defined stylesheets
+ */
+function ckeditor_styles_wysiwyg_editor_settings_alter(&$settings, &$context) {
+  if ($context['profile']->editor == 'ckeditor') {
+    $settings['extraPlugins'] .= ',stylesheetparser';
+    $settings['stylesSet'] = array();
+  }
+}

This code will never be executed as there's no module called 'ckeditor_styles', so Drupal won't find this function when invoking hook_wysiwyg_editor_settings_alter().

If it did run however, it would undo all the work done in the rest of the patch since it clears the 'stylesSet' setting.

It would be a lot better to put meta-data for this plugin/extension in wyswiyg_ckeditor_plugins(), after the default "plugin".

$plugins = array(
  'default' => array(
    'buttons' => array(
      ...
    ),
  ),
);
if (version_compare($editor['installed version'], '3.6', '>=') {
  $plugins['stylesheetparser'] = array(
    'url' => 'http://docs.cksource.com/CKEditor_3.x/Developers_Guide/Styles#Stylesheet_Parser_Plugin',
    'extensions' => array('stylesheetparser' => t('Stylesheet parser')),
    'internal' => TRUE,
    'load' => TRUE,
  );
}

(Untested.)

That would present a new checkbox called "Stylesheet parser" on the "Buttons and plugins" list so users can choose whether they want to use this new plugin. (The contentsCss setting gets set by the widgets controlling which stylesheets get loaded into the editor.)

Btw, I'm not ignoring this issue. I have a s*tload of other things to have done before yesterday and since there are actually other devs in here actively testing and working on the patches - HUGE thanks to everyone involved! - I feel I can focus on other things while this matures.

I'm still not very into the idea of introducing editor-specific code into Wysiwyg's "core", so I'm still been spending most of my Wysiwyg-related coding time on allowing editors and plugins to properly customize the admin GUI. (Basically what sun talks about in #164.)
I've realized that problem could become a bit of a flame-bait if not solid enough so I'm careful with which code I actually want to release before I know it'll work in the long run. That has lead to issues like this one being left open - waiting for that generic form-alteration solution - way too long and I am sorry for that.

This is currently a pretty small change in the CKEditor GUI, perhaps one many will not even notice, so I will go ahead and commit this change once we have a final patch. When the patch with that generic form-alteration solution eventually gets out, I'll move/adapt this code as part of implementing editor-specific settings forms.

Ankabout’s picture

Can anyone explain in short why this is such a big problem if there have been patches early in this thread that worked fine? I'm not criticising anyone, I just want to understand why it's taking so long to get this committed to the released versions.

fietserwin’s picture

As #176 wrote:

I'm still not very into the idea of introducing editor-specific code into Wysiwyg's "core".

As a software engineer I fully agree with this standpoint. But one could also say that the existing code is TinyMCE specific, as it allows to apply user defined styles to all kinds of elements which is absolutely not how styles classes normally are used... So are we talking over engineering or is this a valid code smell? I don't know and leave that decision to the module maintainers.

davidseth’s picture

Patch at #169 worked a treat! Thanks. I am now able to define my own custom styles and they show up and work within Ckeditor :)

Thanks @kardave!

mu5a5hi’s picture

Anyone else have issues with this patch?

wysiwyg-746524-169.patch

I tried git apply.. patch.. kept getting errors seemingly related to trailing whitespace..
I finally just carefully applied the changes by hand and it works.

The only reason I ask is if the issue is not isolated to just me I want the next person to not have to go through this.

steinmb’s picture

It is not only you. If you can, pls reroll from your manually applied patch.

curl http://drupal.org/files/wysiwyg-746524-169.patch | git apply -
<stdin>:10: trailing whitespace.
    '#description' => t('Optionally define CSS classes for the "Font style" dropdown list.<br />Enter one class on each line in the format: !format. Example: !example<br />If left blank, CSS classes are automatically imported from all loaded stylesheet(s).',
<stdin>:11: trailing whitespace.
      $profile->editor == 'ckeditor' && version_compare($editor['installed version'], '3.2.1', '>=')
<stdin>:12: trailing whitespace.
      ? array('!format' => '[title]=[element].[class]', '!example' => 'My heading=h1.header1')
<stdin>:13: trailing whitespace.
      : array('!format' => '[title]=[class]', '!example' => 'My heading=header1')
<stdin>:14: trailing whitespace.
    ),
fatal: corrupt patch at line 103
DuaelFr’s picture

#171 worked for me

Bevan’s picture

#171 worked for me. I had to disable and enable the module before it would take effect. Unfortunately however it breaks the FID relationship of images embedded via media module.

sylus’s picture

FileSize
3.97 KB

Patch #171 was not working in drush make... Rerolled the patch and verified as below:

$ git apply -v wysiwyg-746524-184.patch
Checking patch editors/ckeditor.inc...
Checking patch editors/js/ckeditor-3.0.js...
Checking patch wysiwyg.admin.inc...
Applied patch editors/ckeditor.inc cleanly.
Applied patch editors/js/ckeditor-3.0.js cleanly.
Applied patch wysiwyg.admin.inc cleanly.

Marty2081’s picture

Patch from #184 works for me (patched on 7.x-2.x-dev).

DuaelFr’s picture

#184 working as well

When this will be commited please ? I have to patch this thrice a month... it is a bit boring...

TwoD’s picture

#184 still has the same problems mentioned in #176.

Ankabout’s picture

#171 has another problem it seems...

If I enter only an element, and not a style for it to have, it does not appear in the Font Styles dropdown.

So:

Heading=h2.myheading works fine, but
Heading=h2 does not work.

Looks like a bug to me.

fietserwin’s picture

Well, it is a styles dropdown, not the format dropdown. So without specifying a class it would be quite useless.

Ankabout’s picture

That doesn't mean it shouldn't work right?

Question is just, is it as intended, or a bug?

Reason I use it like that is because this way I can give the "html block elements" my own names, which makes it more user-friendly for my users. Not everyone knows what "Heading 2" means, and in my personal situation it might be different.

fietserwin’s picture

It is intended, you can't change the tag with this feature, you can just set a class on it.

Ankabout’s picture

But if you select the content and you change the style, the tag does change automatically as well.

In previous versions of the module/patch it did work with tags without styles. That's why I'm trying to figure out what happened, since it used to work, and doesn't now.

tlangston’s picture

171 works here.

RedTop’s picture

Patches did not work for me on 7.x-2.x-dev 2012-Apr-30.

$ git apply -v wysiwyg-746524-184.patch
Checking patch editors/ckeditor.inc...
Checking patch editors/js/ckeditor-3.0.js...
Checking patch wysiwyg.admin.inc...
Applied patch editors/ckeditor.inc cleanly.
Applied patch editors/js/ckeditor-3.0.js cleanly.
Applied patch wysiwyg.admin.inc cleanly.

No difference, shows default font styles. even after completely uninstalling module and re-installing patched module.

$ git apply --check wysiwyg-746524-171.patch
error: ckeditor.inc: No such file or directory
error: js/ckeditor-3.0.js: No such file or directory

Am I doing something incorrectly?

Apologies, it did work. After a day of working on this website and a couple of failed attempts the last one was actually successful... except for me not defining the classes.

I did put in element + class though, unlike previous attempts which were class only, after I saw this in the code:
// [name]=[element].[class] pattern expected.

sagannotcarl’s picture

I had to apply the patch manually because I'm using 7.x-2.1 but the functionality is working great.

AaronBauman’s picture

Status: Needs work » Needs review
FileSize
4.04 KB

#184 no longer applies cleanly.
rerolled @ latest dev.

this is a 2-year old issue with almost 200 comments.
can we get this committed already so we can stop rerolling this?

escoles’s picture

What's the current status of this bug -- should it be possible to define style classes in WYSIWYG when using CKEditor? The bug description is ambiguous -- it seems to be saying that yes, it's possible, if you understand the format in which the styles must be specified. (Incidentally, that documentation still doesn't seem to have been changed.)

I ask because it does not seem possible to do that on Drupalgardens, which uses CKEditor.

DuaelFr’s picture

I can confirm it is not possible to define styles without patching the module.

I had to patch this module for every single project I have to build for a year ! (~20 projects)

fietserwin’s picture

Status: Needs review » Postponed

Breaking this issue up into a general and a specific part,
General: #1650416: Allow editor specific changes to be made to the profile settings form
Specific: this issue

In doing so, I'm hoping to get this issue moved instead of hanging on acceptance by the module maintainers. Step 1 is to get the general part accepted, after which step 2 should be to rewrite the current patch using the mechanism introduced by step 1. Step 1 is as per sun's suggestions in #164. So it would be nice if sun (or TwoD) would join the discussion in the separate issue as well. to other followers: please review the other issue.

Marking this as postponed and waiting for #1650416: Allow editor specific changes to be made to the profile settings form

lukus’s picture

When I try to apply the latest patch I receive a white space error:

wysiwyg-font-styles_746524-196.patch:52: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
fietserwin’s picture

Assigned: Unassigned » fietserwin
Status: Postponed » Active

#1650416: Allow editor specific changes to be made to the profile settings form has been committed. We now have the option to define an editor specific form alteration function. i will rewrite the above patch shortly.

fietserwin’s picture

A new patch for this problem, now based on #1650416: Allow editor specific changes to be made to the profile settings form. So, to install and test it, you will need a dev version from 2012-jun-26 or later.

Remarks:

  • re #162 (LeoDoms): this patch will not allow the syntax you use. I propose to defer this to a follow up issue, where we can also look at offering the more advanced options by e.g. allowing to specify a js file (that can contain the more advanced style settings).
  • re #164 (Sun): points 1, 2, and 3, Windows line endings: done.
  • re #164 (Sun): variable naming: done as per #166 (plach).
  • re #164 (Sun): Why do we need to register stylesSet manually? This is how the ckeditor docs explains it, see http://docs.cksource.com/CKEditor_3.x/Developers_Guide/Styles.
  • re #167 (robinvdvleuten): I don't see a need for this. You can enable the 'Font style' under 'Buttons and plugins' t get the styles you define in the 'CSS Classes'. So #176 (TwoD) does no longer apply.

More documentation (regarding 1st and 4th bullet): http://docs.cksource.com/ckeditor_api/symbols/CKEDITOR.config.html#.styl...

Please test, review and post your findings.

fietserwin’s picture

Status: Active » Needs review
sun’s picture

Status: Needs review » Fixed
FileSize
7.91 KB
5.32 KB

Thanks for reporting, reviewing, and testing! Committed attached patch to 7.x-2.x.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

DuaelFr’s picture

Cheers !

webadpro’s picture

Great :)

gagarine’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
Status: Patch (to be ported) » Fixed

You made my (work) day!

AaronBauman’s picture

Version: 7.x-2.x-dev » 6.x-2.x-dev
Status: Fixed » Patch (to be ported)

Same updates needed for 6.x branch.

gagarine’s picture

Version: 7.x-2.x-dev » 6.x-2.x-dev
Status: Fixed » Patch (to be ported)

Perhaps it will be good to open a new issue to port this patch.

TwoD’s picture

Status: Patch (to be ported) » Fixed

I've pushed this to the 6.x-2.x branch as well.
Just noticed that we don't have a changelog entry for this. Any particular reason for that, sun? Just overlooked it?

sun’s picture

heh - apparently you only pushed #1650416: Allow editor specific changes to be made to the profile settings form ;)

I've cherry-picked also this commit into 6.x-2.x.

oh, and yeah, on changelog, I pretty much stopped using/updating it in all of my projects after having learned some git fu, so...:
#1671030: Remove CHANGELOG.txt

TwoD’s picture

Doh! Forgot it was supposed to be two patches. Thanks, sun!

Status: Fixed » Closed (fixed)

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

Bernsch’s picture

Thanx! It works in 7.x-2.x-dev from 2012-Jul-22

The result I can not see life in the CKeditor.
When I have the style blue (in CSS: blue=span.blue), I see the results-only when I save the node.

Bernsch’s picture

@developers
Is this project/module known to you? --> CKEditor Sytle

I think it is time for a new release #1340052: New Wysiwyg 7.x-2.2 Release?
Otherwise be even more additional modules... this is not useful.

romaingar’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
Assigned: fietserwin » Unassigned
Status: Closed (fixed) » Active

@Bernsch : I've got the same problem, i don't see the style applied in the editor, only if i save the node. Did you find a way to achieve this ?

@developers : thanks

harcher’s picture

Big confusing issue... All I want to know is how to center an image from within the wysiwyg. In D6 i created a module that adds style wrappers that get added in the dropdown. Remeber using ckeditor.styles.js.

This thread/issue is too long, confusing, and doesn't help.

Why isn't there an area (at the very start maybe?) within an issue that summerises and updates two things:

1. The problem
2. The latest stable solution/patch (if one is available)?

This will eliminate having to read page after page to see what is the "best" solution/fix/patch...

Anonymous’s picture

If you basically only use CKeditor and need styles to be working without having to resort to workarounds and patching: Use the "normal" CKeditor module and create a custom configuration (ckeditor.config.js / ckeditor.styles.js - google for it, it's easy to get started) for the editor. I switched from WYSIWYG to the "native" module because of this whole style-issue. It also allows for more CKeditor specific configuration options. WYSIWYG specific plugins like "Insert" and "LinkIt" et al work perfectly fine.

Link: CKeditor module

fietserwin’s picture

Status: Active » Closed (fixed)

@218: Note that Linkit is not wysiwyg specific, but has been adopted to also work with the "native" CKE module (and tinymce via wysiwyg and ...)

@216: The issue in this report has long been solved, if you only see it in node view you have to assure that your CSS is loaded in ckeditor and not only on node view. But that is not part of this issue.