I can't say for sure that it's a 'best practice' but I've never seen classes or IDs in Drupal user underscores for css. Since an underscore is used everywhere else, I propose that the class comment_forbidden be changed to use a dash (comment-forbidden).

Some time ago, underscores were not supported in css. I have tested it in IE6 to 9, Chome 8 to 10, Firefox 2 to 4 and Safari 3 to 5 (on both Win and osx where appropriate) and can confirm that underscores in CSS selectors do work. But really, unless there is a particular reason why an underscore was used it would be most intuitive and consistent to use a dash.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Devin Carlson’s picture

Version: 7.0 » 8.x-dev
Category: feature » bug
Priority: Minor » Normal
Status: Active » Needs review
FileSize
1.24 KB

I'm going to mark this as a bug because you're right; Drupal uses a dash everywhere else and themers are going to wonder why they have to use an underscore for only this specific case (they might think something is going wrong). Also moving to Drupal 8 as fixes are done there and back-ported to prevent regressions.

I think the attached patch should do the trick.

dixon_’s picture

Bartik uses that class in print.css. I'd say that this is not a subject to backport to D7. It's not major or critical in any way, and themes might very well depend on this class.

Devin Carlson’s picture

Status: Needs review » Reviewed & tested by the community

As dixon_ mentioned, this is a pretty small issue. His patch applies cleanly and covers the rest of the occurrences of comment_forbidden that I missed, so I'm marking this RTBC.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Nice clean-up. I committed this to 8.x. Marking this 'fixed' as I don't think we want to backport this to 7.x. Let us know if you believe otherwise though.

Status: Fixed » Closed (fixed)

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