Filter options based on CSS attributes, not just HTML tags

douggreen - December 31, 2007 - 18:05
Project:Drupal
Version:7.x-dev
Component:filter.module
Category:feature request
Priority:normal
Assigned:Unassigned
Status:active
Description

Drupal input filters are used to control what HTML tags a user can use. For example, the default "filtered input" filter allows users to use <a> <em> <strong> <cite> <code> <ul> <ol> <li> <dl> <dt> <dd>. This means that the node author can use tags such as <strong> to make some portion of the text bold as I've done here.

The problem is that accessible HTML should use CSS for formatting. Most WYSIWYG editors do this properly by writing CSS instead of the (deprecated) HTML tags. But our input filter system strips CSS classes and style attributes, leaving HTML only. And pages don't display as intended by the WYSIWYG editor. The only way to fix this is to replace the CSS style directives with HTML. However, many of the HTML formatting tags have been deprecated (for example <center> <font> <u> <i> are all deprecated). You can still use them, but it's not advisable. And there's just no way of accomplishing certain basic text formatting without CSS (for example, the CSS attribute "text-align: right" doesn't have an equivalent HTML tag).

So, we need to rethink the filter.module.

  • Class attributes should not filtered. For example, <p class="right"> seems like reasonable HTML to allow. Is there any security risk here? I can't think of one.
  • <span> and <div> should not be filtered. These two HTML tags are the building blocks for adding most CSS. By themselves they do very little. But they are containers for holding many CSS directives. Is there any security risk here? I can't think of any harm in allowing these.
  • We need a way of specifying what CSS attributes can or can not be used with the filter, rather than just what HTML tags can or can not be used. This is the tricky part, and will involve an entirely new way of defining filters. Do we restrict to classes? Do we define the permitted attributes? and/or the permitted attribute/value pairs? For Example, do we allow all "font-weight" attributes, "font-weight: bold", or just "class='bold'"?

IMO, these are necessary precursors to adding a WYSIWYG editor to core.

#1

catch - February 2, 2008 - 17:55

I've run into this a few times. Can think of a couple of issues:

If you have some positional css in your theme - say stuff with floats or position: absolute; attached to classes, then potentially someone could re-use that to stick their html element somewhere crazy. Most of the time these are used with IDs, so might just come under 'don't do that', but still potential weirdness could ensue.

It'd be possible to close an existing div by putting in. Does the htmlcorrector filter in core deal with unopened tags now? Might not be worth worrying about if so.

Personally I'd be against allowing inline style declarations - or at least make this configurable same as the html tags.

#2

Oceria - February 6, 2008 - 21:53

I second this. I use css formatting in my nodes as well, but the html filter changes the layout in limited html filter and rss feeds. Adding tags like does not work.

#3

sun - February 13, 2008 - 03:16

Interesting. It seems, I somehow managed to circumvent the removal of 'class' attributes, see project page of Image Assist. While I think that this kind of abuse is okay, evil users could certainly apply classes that will completely break a site's layout.

However, +1 on this in general. How about a module-controlled tag and attribute register? Instead of manually typing a list of allowed tags and attributes, we should /force/ the required (for certain modules like Image Assist or Wysiwyg editors) and allow to /select/ the optional.

#4

beginner - June 9, 2008 - 23:13

Yes, allowing style attributes seems logical.

 
 

Drupal is a registered trademark of Dries Buytaert.