escaped html element in html select element of exposed filter

O M G... I did not see the exception (#no_convert) thrown in there.

Patch summary: the code strips html structure, if #type is "select", which later goes to check_plain() in form_select_options().

Might as well add decode_entities() in to the mix. :3

I'm worried about the last comment in stackoverflow though, that strip_tags() would nuke <b I forgot to close the tag. :S

I was grepping strip_tags from Drupal core, there's not many of them, I found:

• check_plain(decode_entities(strip_tags($description))) in includes/common.inc line 1591
• truncate_utf8(trim(decode_entities(strip_tags($comment_text))), 29, TRUE) from modules/comment/comment.module line 2181

and bunch of others, and took the generalization from there.

I think I overlook something else, both _options_prepare_options() and form_select_options() takes into account that the display value could be an array.

    // Recurse for optgroups.
    if (is_array($label)) {
      _options_prepare_options($options[$value], $properties);
    }

    if (is_array($choice)) {
      $options .= '<optgroup label="' . $key . '">';
      $options .= form_select_options($element, $choice);
      $options .= '</optgroup>';
    }

In that case the clean up logic should be in its own function, and do a recursive if the display value is an array, like they did with _options_prepare_options() and form_select_options().

On second thought, _options_prepare_options() looks very similar to form_select_options(), what's new is the strip_tags() and filter_xss() call, a clean up logic that should have been done in form_select_options() instead?

My next logical step is to copy _options_prepare_options(), except the filter_xss() part--I think that was meant for radios and checkboxes--in views; or let views depend of field|module for sake of lazyness. :p

Maybe core form.inc omitted strip_tags() on purpose to gain speed?

Hmm, this issue is now outside my comfort zone, should the clean up logic:

• be a method of views_handler_filter
• be a function inside file views_handler_filter.inc
• be a function inside file views.module
• move the issue to drupal form.inc

The patch was made with the assumption that only plain text is going to be the label on the options in select. We didn't expect an image as its label. Since we started with that assumption what the patch does is if you have this label before:

<img src="icon.png"> option number one

After it goes through the patched codes would turned in to (with html elements nuked out of existence):

 option number one

Also, as you can see in this dabblet mockup, even if you forced it, the browser isn't going to render the image element, natively.

What you want though is the #no_convert option, that I ran into at #3.

The #no_convert option, forced Views to display the checkbox element as is, in that Views does not convert (er, translate?) it to select element.

  /**
   * Make some translations to a form item to make it more suitable to
   * exposing.
   */
  function exposed_translate(&$form, $type) {
...
    // Checkboxes don't work so well in exposed forms due to GET conversions.
    if ($form['#type'] == 'checkboxes') {
      if (empty($form['#no_convert']) || empty($this->options['expose']['multiple'])) {
        $form['#type'] = 'select';
      }
...

The comment on that code about GET conversions is weird though, dunno what it meant.

@adelka doesn't look like strip_tags() made an effort to extract alt, or title attribute out of an img element. :(

Also I think you are using core's Field module, as in you are making a Field with the type List (text) and the widget is Check boxes/radio buttons. So I'm quite sure that talking about writing php codes would complicate things.

Is it possible to do this instead:

Sodexho|<img title="Sodexho" src="sodexho.png"><span class="element-invisible">Sodexho</span>

Also for accessibility sake, if you have an image, make sure you have a textual representation of that image, usually in the form of alt attribute, except in this case strip_tags() would nuked it, not good. Naughty strip_tags(). That special visually hidden span element there served the same purpose I think. The title adds context in case someone is not clear of the icon's meaning. Err, but screen reader would read both the image title and the span next to it. Dunno.

Tagging with needs accessibility review (I wonder if that's a bit much :3).

Googling sodexho I found this. :o

Edit: err, the css rules for element-invisible wasn't available in Drupal 6. If you're using one you might need this in your theme css file.

/**
 * Hide elements visually, but keep them available for screen-readers.
 *
 * Used for information required for screen-reader users to understand and use
 * the site where visual display is undesirable. Information provided in this
 * manner should be kept concise, to avoid unnecessary burden on the user.
 * "!important" is used to prevent unintentional overrides.
 */
.element-invisible {
  position: absolute !important;
  clip: rect(1px 1px 1px 1px); /* IE6, IE7 */
  clip: rect(1px, 1px, 1px, 1px);
}

/**
 * The .element-focusable class extends the .element-invisible class to allow
 * the element to be focusable when navigated to via the keyboard.
 */
.element-invisible.element-focusable:active,
.element-invisible.element-focusable:focus {
  position: static !important;
  clip: auto;
}

Or this one from html5 boilerplate (replace visuallyhidden with element-invisible):

/* Hide only visually, but have it available for screenreaders: h5bp.com/v */
.visuallyhidden { border: 0; clip: rect(0 0 0 0); height: 1px; margin: -1px; overflow: hidden; padding: 0; position: absolute; width: 1px; }

/* Extends the .visuallyhidden class to allow the element to be focusable when navigated to via the keyboard: h5bp.com/p */
.visuallyhidden.focusable:active, .visuallyhidden.focusable:focus { clip: auto; height: auto; margin: 0; overflow: visible; position: static; width: auto; }

LAAAAATEEE REPLYYYYYYY -.-