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.
Comment | File | Size | Author |
---|---|---|---|
#2 | 1097958-comment-forbidden-class-2.patch | 1.53 KB | dixon_ |
#1 | 1097958-use-dash-instead-of-underscore-in-class-name.patch | 1.24 KB | Devin Carlson |
Comments
Comment #1
Devin Carlson CreditAttribution: Devin Carlson commentedI'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.
Comment #2
dixon_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.
Comment #3
Devin Carlson CreditAttribution: Devin Carlson commentedAs 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.
Comment #4
Dries CreditAttribution: Dries commentedNice 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.