Hi sun,

I got your message on IRC and wanted to talk to you about the isset($element['format']['guidelines']) issue that you mentioned from filtersbynodetype.

I've tested my module and because my module uses an almost exact copy of filter form it works fine with one format.

However, I do have one setting that conflicts, "show format tips". This setting allows a user to hide the tips for the formats. This is fine if there are multiple formats to choose from but as you know if only 1 format is showing wysiwyg is looking for the guidelines array which is no longer there because the admin has decided to hide the tips which is what the guidelines are. Wysiwyg also looks for the fieldset array at times as well I noticed which better formats also lets you change and/or hide.

I have filed this task to discuss with you on the options we have because I believe these to be valid and useful features of better formats for Drupal. I have not fully read through all of the the wysiwyg module and do not have a complete understanding of you reasoning for using certain conditions so please correct me if I am wrong or if your modules needs something I am missing.

According to filter_form(), the format value is the only required part of the format form and should be all wysiwyg needs to determine if the field is formatted. Here is the line from filter_form():

 $form[$format->format] = array('#type' => 'value', '#value' => $format->format, '#parents' => $parents);

If that is not sufficient for wysiwyg to opperate, I would like to hear your thoughts on how I may be able to keep these features and still work with wysiwyg.

Again, thank you for bringing this to my attention.

Comments

sun’s picture

Tried to ping you in IRC.

The problem with the form element you mentioned is:

$form[$format->format]

I.e. there is no fixed array key Wysiwyg could check for. The only known key in this array that is the same for one and multiple formats is the last one, containing a link to the formatting guidelines help page.

So Wysiwyg needs to explicitly check for the containing elements - and alter them accordingly. If there are multiple, we can attach our classes to the radio buttons. Otherwise, we must add a new DIV container that holds our classes.

Please also note that the new #input_format FAPI property in D7 will simplify this a lot. So this is just a - admittedly nasty - workaround for D5 and D6.

dragonwize’s picture

Status: Active » Fixed

Thanks for the reply.

Ok, I changed my filter_form() tips code, as shown below, and the issue seems to be fixed. I just kept the guidelines array but set its value to an empty string. Not the most ideal solution, like you said, but if we need it, it will have to work.

if ($show_tips) {
  $extra = theme('filter_tips_more_info');
  $tips  = _filter_tips($format->format, FALSE);
  $form['format']['guidelines'] = array(
     '#title' => t('Formatting guidelines'),
     '#value' => theme('filter_tips', $tips, FALSE, $extra),
   );
   $form[] = array('#value' => $extra);
} else {
    // this is to get around an issue with Wysiwyg API looking for guidelines
    // should not be needed in D7
    // @see drupal.org/node/344169
    $form['format']['guidelines'] = array(
       '#title' => t('Formatting guidelines'),
       '#value' => '',
    );
}
dragonwize’s picture

Title: isset format guidelines discussion » Format guidelines field or more info field are required

Making an additional note here for record purposes.

This was also causing the same issue to happen when tips were hidden even when multiple formats were available to choose from. This is because Wysiwyg API pops off the last element that it expects to be the extra value field that contains the link to the long filter descriptions. The same method of placing an empty value for the field solved it as well.

sun’s picture

Hm. You are hiding the entire format selector and predefine the format? Wouldn't it be better to hide the filter tips for each format only, so the user is still able to change the editor?

dragonwize’s picture

There are 5 display options that can be set with Better Formats. One of them is to hide the format selector but it does not cause any issues because in hiding the selection it just acts as if the default format(which is highly customizable) was the only format available to the user. The is useful to force a format to be used even though there are multiple formats available and can be set per role.

The only option that has caused me issues with integration with Wysiwyg API is the option to hide the format tips. The first issue is caused when there is only one format for the user wysiwyg looks for the guidelines array key which I was removing if tips were set to hidden.

The second issue was cause by when there is a format selector with multiple formats, wysiwyg doesn't look for guidelines but it does array_pop() the last format element which in core is always the "More information about formatting options" link. I consider that link to be part of the tips and so hide or show it with that option. So with that removed it is no longer the last element in the children array and the array pops off the last format instead. While that doesn't affect what format options are available to the user it did cause wysiwyg to not process that format so that a editor would not be shown on that format.

sun’s picture

ok, in any way you should make sure that the resulting form structure looks like the structure generated by filter_form(), according to your alterations:

1) If you limit or default to one input format, the structure should look like as if there was one format only from the beginning.

2) If you hide the input format selector, you are defaulting to a certain input format; thus, the resulting structure should look like as if there was one format only from the beginning, but without contents.

3) If you hide the default input format, you should probably remove the contents (input filter tips) only; leaving the rest of the structure intact.

4) In any case, you should probably append a fake element that is expected to get popped out.

Status: Fixed » Closed (fixed)

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