Enable users to add a CSS class name to a view

shouldn't you escape this
$vars['css_class'] = $view->display_handler->get_option('css_class');
like this
$vars['css_class'] = views_css_safe($view->display_handler->get_option('css_class'));
or not?

Oh, ok
My bad :)

This would be a really. really helpfull addition!
Is the patch allready integrated in the latest dev-version of Views?
Or will it be?

It will be integrated if

• Users had tested it.
The maintainer thinks, its a valuable and good implemented feature

You can review the patch by using http://drupal.org/patch/apply

I totally agree with the idea of the patch. Adding a template each time for every view is hard.

My review:
The patch itself was not easy to apply because the patch was created out of root of drupal anyway. i updated this.
I moved the css configuration unders style settings and before theme, i think this is a good place, any opinions about this, see screenshot.

Before we go this route (and I am not against going this route) I would like to do some Big Picture Thinking on the whole process. If we add classes here, will we need to add classes everywhere? How does this interact with the semantic views module, which I believe lets you add classes per field?

Should we automatically let all fields and all styles have a class variable? Should we then integrate that similarly to how we integrate grouping?

If we do classes, is that opening the door for something else?

As a sidenote, and would we then want to extend this to use something like CTools' CSS caching to allow users to add custom CSS on a per view/display/style basis?

subscribing

Agree on that.

... Agree with #10, we only need to add this on the view level. Once that is done, then the styling becomes much more clear because of the natural hierarchies used in CSS.

.view-name-of-my-view-block-1 .field-lead-image-fid{ ...}

is MUCH more confusing (especially to a team of themers) than

.descriptive-class .field-lead-image-fid{ ...}

@merlinofchaos: How does this interact with the semantic views module, which I believe lets you add classes per field?
Yes, semantic views lets you add classes per field (awsome) but not for the view's outermost wrapper

After re-reading my comment in #12 .. an equally good solution would be to add this functionality directly to semantic views... as I see it, this would be just as good and may assuage some of the hesitancy on merlinofchaos' part.

I'm happy either way.

@bangpond ... thanks for clarifying.

In that case I think this is definitely needed so we should move forward with it here. As a side note, seutje and I were debating this earlier on IRC and it is worth noting that in teh last 2 days at least three people in #drupal-support have asked for this exact functionality...

I have applied this patch and used it on my sandbox site and it works great except my screen looks a bit different than the screenshot in #7... see attached.

I like #7 much better, but my options are not categorized like that... thoughts?

For what my pennyworth is worth, I would vote for being able to add ID and/or class to a View, whether that goes in the semantic Views module or not. At the moment I find the number of different classes incredibly difficult to understand when it comes to theming, plus I have to manage a whole series of sites/views with identical look and feel, it would be really nice to be able to do this in the CSS without having to specify each View individually.

@bleen18: dereine used 3.x version of views for his patch. I'm changing the version of this issue.

I personally prefer #7 against #18. We cannot define a special section for each new feature added to views.

This patch needs to sanitize the css class or prevent non alphanumeric characters. By now it allows XSS attacks.

views allows entering straight up php code, so I think we can assume that administering views is only handled by trusted ppl

Views only allow to enter PHP code using input formats that are controlled properly. I think this feature should check if the text inserted follow this pattern:

classname  another-class-name anotherclassname

I can still enter arbitrary php code without turning on the php input filter module...

as BarisW mentioned in #2, views barely filters anything and this is an entirely different discussion

also, the spec only mentions that a class attribute shouldn't start with a number and can only contain a-z, A-Z, -, _, 0-9. so a class of "foo_BaR-foo658" should validate fine afaik

@bangpound: mmm, what happens if user type ".class-1 #class-2 #class-3 456class" this keys are not valid css name.

I think that we need a regular expression that check the pattern defined by @seutje that can be repeated zero or more times. We can use drupal_attributes() if you want but we need to check the user input.

@dagmar: I don't agree that we should be using regex to determine valid class name(s). Of course we should use drupal_attributes() because that ensures inputs are filtered for security reasons but if a user chooses to use a no-good-very-bad class name, so be it.

Use case: starting a class with a number is generally a no-no, but FF does support it. Maybe someone is creating an intranet site for just FF users. We should always output proper class names when the module creates them, but if the user has an option to enter a class name it should be up to him/her. We don't run regex on .php.tpl files to make sure they are W3C compliant ...

+1 for lets add this as is

As I said in #18 this patch allows XSS Attacks. We need a new version of this patch implementing drupal_attributes(), check_plain() or something like this.

Until that this cannot be marked as reviewed & tested by the community.

Yes! let's beat around the bush about an XSS vulnerability while that same user can enter any php code he wants

make sense please -_-

seutje: Not necessarily. The PHP code is *all* permission controlled. You can turn every bit of it off and a views administrator can then only view the PHP, not edit it.

I agree that we must scrub the string. I hope this ends the argument.

oh, while testing this I didn't manage to give my test user permission to administer views without allowing him to enter php code, so this made no sense at all to me

but I'll take your word for it and then this does make sense

my apologies

[edit]
hmm, after testing it again, it seems that switching user went fine the first pageload, but after the second and so forth, it just jumped back on UID1
unrelated to this issue or views in general (just to be clear)

@seutje: Please calm down. Once you've had a few deep breaths, please stop assuming that only UID 1 has access to administer views.

If you setup a "views administrator" role that only has the "administer views" and "access administration pages" permissions, you can administer views, including accessing this new proposed setting, but you cannot execute PHP code, anywhere. Not in argument validators. Not in header/footer text. Not in field empty text. No where. Try it yourself. And, if you do find somewhere in the views UI you can execute PHP code in this configuration, please email security@drupal.org about it, don't post it here. ;)

So yes, we do actually need to prevent a user who can administer views from being able to execute XSS attacks on the other users of the site by means of this setting. There's no reason to assume that users with "administer views" can be fully trusted. That's the point of having a fine-grained access system in Drupal.

p.s. Sorry, this issue was open in my browser from earlier, and I hadn't reloaded to see the new comments. My comment #35 was actually in reply to comment #30...

Well at least this patch doesn't allow to input < > tags. It doesn't allow underscores either because views usea views_css_safe() to replace it.

There is no reason to restrict underscores... The whole point here is to let the user add their own class name.

+++ plugins/views_plugin_display.inc	3 Dec 2009 14:38:32 -0000
@@ -1598,6 +1624,12 @@ class views_plugin_display extends views
+          form_error($form['css_class'], t('Css classes must be alphanumeric or upperscores only.'));

Is "upperscore" the correct term? I've never heard that. Can we just use "dashes"?

I'm on crack. Are you, too?

Re: #37: "But then again... Why do we have this discussion here? The other fields (try Path settings or Title for example) aren't filtered on XSS vulnerability."

I don't think you understand Drupal's security model. Content in Drupal is filtered on output, not on input. So, yes, you can type this into the title field of a display:

<script>alert('views-title-xss')</script>

And you can save it. And you can see it in the title text field if you try to edit it again.

However, if you preview this display, or it's a page display and you visit the path, etc, the actual HTML for the page looks like this:

    <title>alert('views-title-xss') | local D6</title>
...
<h2>alert('views-title-xss')</h2>
...

See the following for more:
http://drupal.org/writing-secure-code
in particular:
Why does Drupal filter on output?

Re: patch #38:

A) We should filter on output. ;)

+  $vars['css_class']  = $view->display_handler->get_option('css_class');

That's the spot we want to be filtering this on the way out (it's in theme.inc when setting up the template variables), not up in the form handler for the setting like you're doing with that strip_tags() call earlier.

B) Class names can't include spaces, can they? Because this regular expression allows them:

+        if (preg_match('/[^a-zA-Z0-9- ]/', $css_class)) {

@bleen18: yes my english is not perfect, thanks for the review. Please modify all the strings that you consider necessary.

@dww:

A) done
B) yes, you are right class names can't include spaces, however I'm allowing spaces to allow users to define multiple classes per view display. This can be useful in some circumstances when views has a lot of similar displays.

New patch

Note: I'm allowing underscores by now, however I would like to know why views use views_css_safe()

Class names should be restricted to [a-zA-Z0-9-] I can't think of any other legal character in a class name we can think of. On output, convert anything that does not match that pattern to a - via preg_replace -- I'm pretty sure at the worst case there's an example of doing this somewere in the views admin because we already have to do that for classes on the fields when restricting available fields based upon the select in the 'add field/sort/etc' widget.

I'm allowing spaces to allow users to define multiple classes per view display

Agreed ...

can we at least let go of the underscore? or are we assuming ppl are using bad css hacks?

Hm. I guess we can let the underscore through, though I seem to recall that at one point underscores were not allowed in CSS which is why views_css_safe() makes that change.

From W3C (http://www.w3.org/TR/CSS2/syndata.html#characters):

In CSS, identifiers (including element names, classes, and IDs in selectors) can contain only the characters [a-zA-Z0-9] and ISO 10646 characters U+00A1 and higher, plus the hyphen (-) and the underscore (_); they cannot start with a digit, or a hyphen followed by a digit. Identifiers can also contain escaped characters and any ISO 10646 character as a numeric code (see next item). For instance, the identifier "B&W?" may be written as "B\&W\?" or "B\26 W\3F".

CSS1 did not allow underscores but CSS2 does. Also according to this, we should not be limiting characters at all because escaped chars are valid CSS2

I think not all browsers are nice about that. Or at least, weren't. Perhaps modern browsers are.

That said, I would still rather stick to alpha, spaces, dashes and underscores. Change anything else unknown to a dash. We don't really need classes with accents and umlauts in them. =)

"I would still rather stick to alpha, spaces, dashes and underscores" ... Sold to the merlinofchaos!

ur right, for IE6 to pick it up, one would have to escape it in the selector (making this look like this) but I'm just afraid of users getting all confused when they enter an underscore and it doesn't spit out an underscore, turns out not everybody that's dealing with css classes uses firebug, which surprises me as well

For this kind of thing if it would prevent user confusion, we can validate the input as well. It's not difficult to do and it provides better feedback. But we should validate the output even if we also validate the input.

So, to clarify all this stuff:

1) We will not allow underscores because they will be replaced anyway and may confuse users.
2) We need a regular expression that replace all non [a-Z0-9- ] characters replacing by -

Is this correct? Is something else?

and 3) we should go ahead and validate on input like we do view names so that users are not confused when their classes change from what they entered.

Right. I'm all in favor of validating the input for better UI feedback, but we also need to filter on output to be safe. Anyway, #41 definitely needs work, but it's closer. Thanks, folks.

Implemented 1, 2 and 3 in this patch.

If we are to validate input, consider disallow classes to start with a digit, or a hyphen followed by a digit (coz the spec disallows it)

probably best not to allow it to start with a hyphen to begin with, not sure what happens when you use a class like "-moz-border-radius", but I haven't tested any of that

-> [a-zA-Z]+[a-zA-Z0-9- ] ?

Why do we have to check it both in output and on validation? The user cannot submit the css-class if the validation fails.

Because it's best practice to scrub on output anyway. Views can be imported so they might not necessarily come from the normal user input channel.

@bangpond ... +1

in particular I think this:

It would also allow hook_views_pre_render to add more classes to the array before it's imploded.

will be hugely helpful

While I still think bangpound's idea (#58) is an excellent one I think it needs to be split into its own issue (which I did here: #665888: Add an $attribs array to handle HTML attributes in outermost <div>).

I also think that this should not prevent us from moving forward with the patch in #54 (incorporating suetje's suggestion from #55) ...

I still believe this is a super important patch and I'd give it a go myself, but regex is my achilles heal :(

I'm not testing out Views 3 yet, but I needed this for one of my sites. I tried this out with views 2 and it was successful. Any possibility of a backport to views 2 once this is finished?

Subscribing

I tested this against views 2.8 and it appears to work. I have one question though. If I specify a class name on a block display type, I only get the additional class on the div inside the 'content' div for that block. This leaves the div that has the block title and the surrounding div for the whole block left out.

I came across this thread hoping it would let me easily 'name' and style each block, but the div that surrounds the block still has css names like (in my case) : id="block-views-Books-block_6" and class="block block-views".

Is this by design? Does it only do this against 2.8?

Thanks,
Cliff

[EDIT]

For my case, I think I can just blank out the 'Title' parameter and put my block title as the 'Header' parameter, which will allow me to style it, but I still see value in being able to style (by specified css name) the entire block's surrounding div. This could be useful when adding differing borders,background colors, or margin/padding to different blocktypes in the same view. Plus you don't seem to lose much, because if the custom css name is on the surrounding block instead of the div inside content, you can still 'get at' those elements pretty easily, in the same way that fields are easy to style.

I just confirmed the behavior meecect is seeing and I agree that this class should appear on teh outermost div of *all* views including blocks.

There is no way that Views can specify a class name to go on any div outside of the 'views-view' div that it creates.

Committed to all branches.

:) thank you!

how can I use patch manualy by editing module on notepad to use css for a page i created using view

Hello,
I know that this post is very old, but I really need this patch for my projetct.
I tried to install this patch but it didn't work.
My configuration :
- Mac OSX LION
- Drupal 6
- Views : views-6.x-2.14.

1) I've downloaded this version of the patch : views-582348.patch

2) I downloaded and intalled GIT from this link : http://code.google.com/p/git-osx-installer/downloads/detail?name=git-1.7...

3) I've copied the patch file to /my-drupal-site/modules/views

4) I've launched the command on terminal : git apply -v views-582348.patch
I'm getting this error :

Checking patch views_plugin_display.inc...
error: views_plugin_display.inc: No such file or directory
Checking patch theme.inc...
error: theme.inc: No such file or directory
Checking patch views-view.tpl.php...
error: views-view.tpl.php: No such file or directory

5) I've copied it on the folder "theme", and launched the same command, but here is what I get :

Checking patch views_plugin_display.inc...
error: views_plugin_display.inc: No such file or directory
Checking patch theme.inc...
Checking patch views-view.tpl.php...
error: while searching for:
*
* Variables available:
* - $css_name: A css-safe version of the view name.
* - $header: The view header
* - $footer: The view footer
* - $rows: The results of the view query, if any

error: patch failed: views-view.tpl.php:6
error: views-view.tpl.php: patch does not apply

Can somebody please help me, I really need this patch. Or if a new solution exist can you please tell me.

Thanks.

If somebody still maintaining it I really need help.

Hello All,
I am new drupal 7 , Could i have same solution in drupal 7 .
Thanks in advance